Raise Event Thread Safely - Best Practice

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 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.

c# event handling: best practice to avoid thread contention and threadpool draining

So far, this is the best answer I have found. Does there exist any shorthand equivalent of a non-blocking lock() to shorten this up?

static object _myLock;
void myMethod ()
{
    if ( Monitor.TryEnter(_myLock) )
    {
        try
        {
            // Do stuff
        }
        finally
        {
            Monitor.Exit(_myLock);
        }
    }
    else
    {
        // then I failed to get the lock.  Optionally do stuff.
    }
}

Why does this common idiom for thread-safe event-calling in C# works at all?

The delegate object is immutable, so a copy of the reference to it is fine. A standalone local copy of a reference to an immutable object is pretty much the gold standard for avoiding a thread-race problem.

When you add/remove an event subscription, Delegate.Combine et al create a new delegate instance every time it changes (or null if you unsubscribe the last handler) and assigns a reference (/ null) to that new object to the backing field. That is why the snapshot is helpful.

BTW : in modern C#, you can just use TheEvent?.Invoke(....), which does this for you.

Raising Events in Multi-Threaded Environment

You have to add the only line to make the first snippet correct in multithreaded environment:

public event EventHandler MyEvent;
protected void OnMyEvent(EventArgs e)
{
    EventHandler myEvent = MyEvent;
    Thread.MemoryBarrier();
    if (myEvent != null)
    {
        myEvent(this, e);
    }
}

Memory barrier refuses to reorder read and writes for both compiler and CPU. That's how volatile reads/writes are implemented. You can read more about memory barrier here.

Thread Safe Event Handling

If it were

if (mNotification!=null)
{
    mNotification(this, null);
}

mNotification could be set to null by another thread between if (mNotification!=null) and mNotification(this, null);

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.

---

Related Topics