Event Handlers Not Thread Safe

Event handlers not thread safe?

Events are really syntactic sugar over a list of delegates. When you invoke the event, this is really iterating over that list and invoking each delegate with the parameters you have passed.

The problem with threads is that they could be adding or removing items from this collection by subscribing/unsubscribing. If they do this while you are iterating the collection this will cause problems (I think an exception is thrown)

The intent is to copy the list before iterating it, so you are protected against changes to the list.

Note: It is however now possible for your listener to be invoked even after you unsubscribed, so you should make sure you handle this in your listener code.

C# Is it thread safe to subscribe Same event handler for all Objects

Thread-safety always needs a context - a from what.

If you mean the +=

For the +=, that depends on how the event is implemented. If it is implemented as a field-like event, i.e.

public event SomeEventType EventReceived;

then yes: it is thread-safe. The specification requires that the accessors for field-like events are thread-safe, although the implementation can vary (for example, the MS compiler used to use lock(this) or lock(typeof(DeclaringType)), however it now uses Interlocked instead).

If the event is implemented manually, i.e.

public event SomeEventType EventReceived {
    add { ... }
    remove { ... }
}

Then the thread-safety is defined entirely by the add/remove implementations.

If you mean the invoke

Then that is thread-safe since delegates are immutable, but note that it all happens on the single thread that invokes the thread. However, a common mistake is to introduce a race condition in a null test:

if(EventReceived != null) EventReceived(this, someArgs);

The above is not thread-safe, since technically the value of EventReceived can change after the test. To ensure this does not error, it should be:

var handler = EventReceived;
if(handler != null) handler(this, someArgs);

If you mean the handlers

Then the thread-safety is defined entirely by the individual handlers. For example, in UI applications it is the handlers that must check for, and switch to, the UI thread.

is it thread safe to register for a c# event?

Yes, the += and -= operators on auto implemented events are atomic (if a library used a custom event handler it could very easily not be atomic). From the MSDN Magazine article .NET Matters: Event Accessors

When the C# compiler generates code for MyClass, the output
Microsoft® Intermediate Language (MSIL) is identical in behavior to
what would have been produced using code like that in Figure 1.

Figure 1 Expanded Event Implementation

class MyClass
{
    private EventHandler _myEvent;

    public event EventHandler MyEvent
    {
        [MethodImpl(MethodImplOptions.Synchronized)]
        add 
        { 
            _myEvent = (EventHandler)Delegate.Combine(_myEvent, value);
        }
        [MethodImpl(MethodImplOptions.Synchronized)]
        remove 
        { 
            _myEvent = (EventHandler)Delegate.Remove(_myEvent, value); 
        }
    }
    ...
}

[...]

Another use for explicit event implementation is to provide a custom
synchronization mechanism (or to remove one). You'll notice in Figure
1 that both the add and remove accessors are adorned with a
MethodImplAttribute that specifies that the accessors should be
synchronized. For instance events, this attribute is equivalent to
wrapping the contents of each accessor with a lock on the current
instance:

add { lock(this) _myEvent += value; } 
remove { lock(this) _myEvent -= value; }

C# Events and Thread Safety

The JIT isn't allowed to perform the optimization you're talking about in the first part, because of the condition. I know this was raised as a spectre a while ago, but it's not valid. (I checked it with either Joe Duffy or Vance Morrison a while ago; I can't remember which.)

Without the volatile modifier it's possible that the local copy taken will be out of date, but that's all. It won't cause a NullReferenceException.

And yes, there's certainly a race condition - but there always will be. Suppose we just change the code to:

TheEvent(this, EventArgs.Empty);

Now suppose that the invocation list for that delegate has 1000 entries. It's perfectly possible that the action at the start of the list will have executed before another thread unsubscribes a handler near the end of the list. However, that handler will still be executed because it'll be a new list. (Delegates are immutable.) As far as I can see this is unavoidable.

Using an empty delegate certainly avoids the nullity check, but doesn't fix the race condition. It also doesn't guarantee that you always "see" the latest value of the variable.

Raise event thread safely - best practice

There is a tiny chance that SomethingHappened becomes null after the null check but before the invocation. However, MulticastDelagates are immutable, so if you first assign a variable, null check against the variable and invoke through it, you are safe from that scenario (self plug: I wrote a blog post about this a while ago).

There is a back side of the coin though; if you use the temp variable approach, your code is protected against NullReferenceExceptions, but it could be that the event will invoke event listeners after they have been detached from the event. That is just something to deal with in the most graceful way possible.

In order to get around this I have an extension method that I sometimes use:

public static class EventHandlerExtensions
{
    public static void SafeInvoke<T>(this EventHandler<T> evt, object sender, T e) where T : EventArgs
    {
        if (evt != null)
        {
            evt(sender, e);
        }
    }
}

Using that method, you can invoke the events like this:

protected void OnSomeEvent(EventArgs e)
{
    SomeEvent.SafeInvoke(this, e);
}

c# events execution are thread safe?

C# will not doing any locking for you. If the events can be raised by multiple threads simultaneously, you must write code to handle that (if necessary).

You can use a lock statement to prevent multiple threads from executing it:

private void MyEventHandler(object sender, EventArgs e)
{
    lock (lockingObject)
    {
        // Handle event here.
        // Only one thread at a time can reach this code.
    }
}

Where lockingObject is a field inside your class declared like:

private readonly object lockingObject = new object();

You also have to be careful about threading in a method that raises an event.

Suppose you have an event in your class called MyEvent. You should not do this:

private void RaiseMyEvent()
{
    if (MyEvent != null)                  // {1}
        MyEvent(this, new EventArgs());   // {2}
}

If another thread can detach from MyEvent, then it's possible that it could detach between line {1} and line {2}. If that happens, line {2} will throw a null reference exception because MyEvent will have suddenly become null!

The correct way to do this is:

private void RaiseMyEvent()
{
    var handler = MyEvent;

    if (handler != null) 
        handler (this, new EventArgs()); 
}

Now the null reference exception can't happen.

However, note that when using multiple threads it's possible for an event handler to get called after the thread has detached it!

---

Related Topics