35

Something that confuses me, but has never caused any problems... the recommended way to dispatch an event is as follows:

public event EventHandler SomeEvent;
...
{
    ....
    if(SomeEvent!=null)SomeEvent();
}

In a multi-threaded environment, how does this code guarantee that another thread will not alter the invocation list of SomeEvent between the check for null and the invocation of the event?

6 Answers 6

57

As you point out, where multiple threads can access SomeEvent simultaneously, one thread could check whether SomeEventis null and determine that it isn't. Just after doing so, another thread could remove the last registered delegate from SomeEvent. When the first thread attempts to raise SomeEvent, an exception will be thrown. A reasonable way to avoid this scenario is:

protected virtual void OnSomeEvent(EventArgs args) 
{
    EventHandler ev = SomeEvent;
    if (ev != null) ev(this, args);
}

This works because whenever a delegate is added to or removed from an event using the default implementations of the add and remove accessors, the Delegate.Combine and Delegate.Remove static methods are used. Each of these methods returns a new instance of a delegate, rather than modifying the one passed to it.

In addition, assignment of an object reference in .NET is atomic, and the default implementations of the add and remove event accessors are synchronised. So the code above succeeds by first copying the multicast delegate from the event to a temporary variable. Any changes to SomeEvent after this point will not affect the copy you've made and stored. Thus you can now safely test whether any delegates were registered and subsequently invoke them.

Note that this solution solves one race problem, namely that of an event handler being null when it's invoked. It doesn't handle the problem where an event handler is defunct when it's invoked, or an event handler subscribes after the copy is taken.

For example, if an event handler depends on state that's destroyed as soon as the handler is un-subscribed, then this solution might invoke code that cannot run properly. See Eric Lippert's excellent blog entry for more details. Also, see this StackOverflow question and answers.

EDIT: If you're using C# 6.0, then Krzysztof's answer looks like a good way to go.

5
  • I have a problem with the statement "In addition, assignments of object references in .NET are thread-safe". You surely meant atomic? As far as I know, if thread A sets a reference on variable V, nothing guarantees that thread B will set the updated reference in variable V, unless V is volatile or lock statements are used while reading and writing V. Apr 6, 2011 at 13:26
  • That also means that your example is broken. If thread A added event handlers to SomeEvent, and then thread B invokes SomeEvent, it may very well be the case that thread B sees SomeEvent as null, unless SomeEvent was declared as volatile Apr 6, 2011 at 13:31
  • 1
    @Christophe, by "thread-safe", I mean that assignment of an object reference in one thread will never be interrupted or seen as inconsistent by another thread. As mentioned in my update, that's definitely not the same thing as saying that thread A and thread B will always have the same view of the event. All this example does is prevent one specific race condition, not every race condition.
    – HTTP 410
    Apr 8, 2011 at 23:04
  • @RoadWarrior, as this question is still getting traffic, I awarded the top answer to the ?.Invoke answer so that those looking for the quick-hit solution see it first.
    – spender
    Jun 5, 2018 at 13:15
  • @spender, agreed - Krzysztof's answer is now the best approach.
    – HTTP 410
    Jun 5, 2018 at 22:34
26

In C# 6.0 you can use monadic Null-conditional operator ?. to check for null and raise events in easy and thread-safe way.

SomeEvent?.Invoke(this, args);

It’s thread-safe because it evaluates the left-hand side only once, and keeps it in a temporary variable. You can read more here.

2
  • I'll probably come back and mark this as the correct answer when we upgrade to vs2015 and I've had a chance to play with the new language features.
    – spender
    Sep 7, 2015 at 8:54
  • Nice. This now looks like THE de-facto approach going forward.
    – StuartLC
    Jun 16, 2016 at 13:48
22

The simplest way remove this null check is to assign the eventhandler to an anonymous delegate. The penalty incurred in very little and relieves you of all the null checks, race conditions etc.

public event EventHandler SomeEvent = delegate {};

Related question: Is there a downside to adding an anonymous empty delegate on event declaration?

2
  • Why is this not marked as the "official" answer? This is the only good answer.
    – yfeldblum
    Nov 12, 2008 at 6:03
  • 4
    I don't agree. That method seems like a hack and does not make firing events "safe" or reliable, it only solves the null checking issue. Please see my explanation in the related thread.
    – Scott P
    Nov 12, 2008 at 6:22
4

The recommended way is a little different and uses a temporary as follows:

EventHandler tmpEvent = SomeEvent;
if (tmpEvent != null)
{
    tmpEvent();
}
4
  • so does that clone the invocation list then?
    – spender
    Nov 12, 2008 at 0:55
  • Alternatively you could call GetInvocationList into a temporary and execute each in order. Nov 12, 2008 at 1:12
  • dp, you don't need to do this. See my answer for why.
    – HTTP 410
    Nov 12, 2008 at 1:38
  • RW: agreed. I was trying to point out that essentially this does the same thing. Good answer. +1 Nov 12, 2008 at 18:47
3

Safer approach:


public class Test
{
    private EventHandler myEvent;
    private object eventLock = new object();

    private void OnMyEvent()
    {
        EventHandler handler;

        lock(this.eventLock)
        {
            handler = this.myEvent;
        }
        if (handler != null)
        {
            handler(this, EventArgs.Empty);
        }
    }

    public event MyEvent
    {
        add
        {
            lock(this.eventLock)
            {
                this.myEvent += value;
            }
        }
        remove
        {
            lock(this.eventLock)
            {
                this.myEvent -= value;
            }
        }

    }
}

-bill

2
  • 1
    This is the correct approach as given by Jon Skeet in yoda.arachsys.com/csharp/events.html Apr 6, 2011 at 14:03
  • 1
    You have synchronized the assignment of handler with a lock, but won't checking for null after leaving the synchronized block still allow for the same issues? Sep 8, 2011 at 17:15
0

I would like to suggest an slight improvment to RoadWarrior's answer by utilizing an extention function for the EventHandler:

public static class Extensions
{
    public static void Raise(this EventHandler e, object sender, EventArgs args = null)
    {
        var e1 = e;

        if (e1 != null)
        {
            if (args == null)
                args = new EventArgs();

            e1(sender, args);
        }                
    }
  }

With this extension in scope, events can be raised simply by:

class SomeClass { public event EventHandler MyEvent;

void SomeFunction()
{
    // code ...

    //---------------------------
    MyEvent.Raise(this);
    //---------------------------
}

}

Your Answer

Reminder: Answers generated by Artificial Intelligence tools are not allowed on Stack Overflow. Learn more

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.