C # Я правильно использую блокировку?

Question

Я сейчас пытаюсь написать потокобезопасный класс логгера. Я не очень хорошо знаком с правильным дизайном и лучшими практиками в этой области. В моем коде есть изъян?

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 ();
        }
    }
}

Answer 1

Ну, это выглядит нормально для меня; Я бы, вероятно, реализовал IDisposable как средство для Close() файла, но ...

Конечно, вы также можете использовать любую из (многих) предварительно консервированных каркасов журналирования.

---

Обновление:

Одна мысль: вы можете подумать, что произойдет, если файл уже существует; Вы не хотите топать свои логи ...

Answer 2

То, что вы опубликовали, выглядит хорошо с точки зрения многопоточности. Хотя я могу ошибаться, может показаться, что любой другой код, который выполняет многопоточность (даже используя объект foo), должен быть безопасным. Конечно, я не вижу никаких тупиков в этом разделе кода.

В любом случае стоит отметить несколько вещей (кроме того, что нужно быть очень осторожным с блокировками и тщательно проверять, чтобы убедиться, что они не возникнут):

• Лучше поставить блокировку вокруг кода внутри конструктора, так как я считаю, что возможно при определенных обстоятельствах, что методы могут быть вызваны до завершения выполнения блока конструктора. (Кто-то, пожалуйста, поправьте меня, если я ошибаюсь в этом.)
• Объект StreamWriter в этом случае является приватным, что хорошо. Если бы он был защищенным или внутренним, вам, безусловно, следовало бы быть осторожным с тем, как другой код использует объект (на самом деле, я думаю, что было бы лучше почти всегда объявлять такие объекты как частные).
• Вы сделали правильную блокировку! Всегда безопаснее заблокировать отдельный объект частного экземпляра, потому что вы знаете , что этот объект не может быть заблокирован каким-либо другим кодом, кроме вашего (что не так, если вы блокируете this или StreamWriter сам объект).

Тем не менее, я могу что-то упустить, и есть небольшая вероятность того, что какой-то другой код, не показанный выше, может вызвать проблемы, но, насколько я понимаю, этот код не ошибочен, за исключением возможного отсутствует блокировка вокруг кода конструктора. С большей вероятностью вам придется остерегаться тупиковых ситуаций, когда вы начнете выполнять более сложные многопоточности, особенно между классами / экземплярами.

В любом случае, надеюсь, это поможет.

Answer 3

Обработчик событий находится в том же потоке, что и генератор событий, что означает, что ваше приложение может в конечном итоге быть задержано записью вашего файла журнала.

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 ();                
   }        
}