[英]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