[英]C# Am i using lock correctly?

I'm currently trying to write a thread-safe logger class. I'm not very familiar with correct design and best practices in this area. Is there a flaw in my code?


public class WriteStuff
    private readonly StreamWriter m_Writer;
    private readonly object m_WriteLock = new object ();

    public WriteStuff(String path)
        m_Writer = File.CreateText (path);

        m_Writer.WriteLine ("x");
        m_Writer.Flush ();

    public void ListenTo(Foo foo)
        foo.SomeEvent += new EventHandler<SomeArgs> (Foo_Update);

    private void Foo_Update(object sender, SomeArgs args)
        lock (m_WriteLock) {
            m_Writer.WriteLine (args);
            m_Writer.Flush ();

3 个解决方案


What you've posted looks fine from a multi-threading perpective. Although I could be wrong, it would appear that any other code that does some multi-threading (even using the foo object) should be safe. Certainly, I can't see any deadlocks in the that section of code.


A few things worth noting anyway (apart from being very careful with deadlocks and testing rigourously to insure they won't occur):


  • It's best to put a lock around the code within the constructor, as I believe it's possible in certain circumstances that methods can be called before the constructor block has finished executing. (Someone please correct me if I'm wrong on this one.)
  • 最好在构造函数中对代码进行锁定,因为我相信在某些情况下可以在构造函数块执行完毕之前调用方法。 (如果我错了,请有人纠正我。)

  • The StreamWriter object in this case is private, which is good. If it were protected or internal you would certainly have to be cautious about how other code utilised the object (in fact I think it would be best to almost always declare such objects as private).
  • 在这种情况下,StreamWriter对象是私有的,这很好。如果它是受保护的或内部的,你肯定必须小心其他代码如何使用该对象(实际上我认为最好几乎总是将这些对象声明为私有)。

  • You've done locking the right way! It's always safest to lock on a separate private instance object because you know that object can't be locked by any other code than your own (which isn't the case if you lock this or the StreamWriter object itself).
  • 你已经完成了正确的锁定!锁定单独的私有实例对象始终是最安全的,因为您知道该对象不能被您自己的任何其他代码锁定(如果您锁定此对象或StreamWriter对象本身则不是这种情况)。

Still, I may be missing something, and there is a small possibility that some other code not shown above might cause problems, but as far as I can see it that code isn't flawed except for a possible missing lock around the constructor code. You're more likely to have to watch out for deadlock situations when you start doing more complex multi-threading, especially across classes/instances.


Anyway, hope that helps.



Well, that looks OK to me; I'd probably implement IDisposable as a means to Close() the file, but...


Of course, you could also use any of the (many) pre-canned logging frameworks.



One thought: you might want to consider what happens if the file already exists; you don't want to stomp on your logs...



The event handler is on the same thread as the event generator which means your app could end up being held up by your log file write.


private void Foo_Update(object sender, SomeArgs args)        { 
    ThreadPool.QueueUserWorkItem(WriteAsync, args);

private void WriteAsync(object state) {  
    SomeArgs args = (SomeArgs)state;    
    lock (m_WriteLock) {                        
       m_Writer.WriteLine (args);                        
       m_Writer.Flush ();                



粤ICP备14056181号  © 2014-2021 ITdaan.com