C#: Thread-safe events

c#, event-handling, events, multithreading, thread-safety

Solution

There is no need for the private `ProcessCompleted` member to be an `event` - it could just be a field: `private EventHandler<ProcessCompletedEventArgs> ProcessCompleted;` - inside the class it always goes straight to the field, so the `event` stuff is lost anyway.

The approach you've shown with an explicit lock object isn't much more thread-safe than just having a field-like event (i.e. `public event EventHandler<ProcessCompletedEventArgs> ProcessCompleted;` - the only difference is that you aren't locking "this" (which is a good thing - you should ideally avoid locking on `this`).. The "handler variable" approach is the right one, but there are still side-effects you should be aware of.

Problem

Is the implementation below thread-safe? If not what am I missing? Should I have the `volatile` keywords somewhere? Or a lock somewhere in the `OnProcessingCompleted` method? If so, where? ``` public abstract class ProcessBase : IProcess { private readonly object completedEventLock = new object(); private event EventHandler<ProcessCompletedEventArgs> ProcessCompleted; event EventHandler<ProcessCompletedEventArgs> IProcess.ProcessCompleted { add { lock (completedEventLock) ProcessCompleted += value; } remove { lock (completedEventLock) ProcessCompleted -= value; } } protected void OnProcessingCompleted(ProcessCompletedEventArgs e) { EventHandler<ProcessCompletedEventArgs> handler = ProcessCompleted; if (handler != null) handler(this, e); } } ``` Note: The reason why I have private event and explicit interface stuff, is because it is an abstract base class. And the classes that inherit from it shouldn't do anything with that event directly. Added the class wrapper so that it is more clear =)

Original source