TimL on C#.NET

There is a long-standing debate amongst C#.NET developers: When raising an event, should you copy the event delegate first or not?

public event EventHandler MyEvent;

private void OnRaiseMyEventWithoutCopy()
{
    if (MyEvent != null)
    {
        MyEvent(this, new EventArgs());
    }
}

private void OnRaiseMyEventWithCopy()
{
    EventHandler myEvent = MyEvent;
    if (myEvent != null)
    {
        myEvent(this, new EventArgs());
    }
}

 

In both cases, you must check to see if the event delegate is null; if you attempt to invoke a null delegate you will get a NullReferenceException. Once the last event handler has unregistered, the delegate will be null.

Interestingly, the arguments for doing it one way or another typically involve reasons for not doing it the other way:

The argument for the "with copy" case basically comes down to this: If you do not do the copy, the delegate may change to null after you did the null-check but before the delegate was invoked, and that could be bad.

The argument for the "without copy" case basically comes down to this: If you do the copy, then you might invoke an event handler that has already unregistered, and that would be bad.

Honestly, I have flip-flopped on this several times over the years. But, I currently believe it to be better to do the copy. In the tradition of saying something is better this way because it is not doing that:

1) If you do not do the copy, you still have the potential NullReferenceException issue. If .NET threw a different type of exception, then you could catch it and ignore it because it would be expected that the last handler may have unregistered before you actually invoked the delegate. But, since it throws the NullRefernceException which could easily have been thrown by one of the handlers, you should probably let that bubble out so that the code raising the event knows something bad happened.

2) Whether you do the copy or not, once the framework starts traversing down the invocation list, it's just like doing a copy anyway. If a handler unregisters for an event while the framework is traversing the list, that handler will still be called. Therefore, handlers must always be written with the understanding that it can still get called after it has been unregistered.

Since you cannot prevent issue 2, which is basically what the "without copy" way is trying to prevent, you should at least do the copy because you can prevent issue 1.