Raising Events or Exceptions?

This post discusses the issues involved in raising events in a .NET class for a multi threaded application and a problem with the widely accepted solution.

When you write your own class that supports events, you also have to write the code that raises these events. A typical way to structure that code is in a protected virtual On[EventName] method and most implementations look something like this:

public event EventHandler Changed;

protected virtual void OnChanged()
{
if (Changed != null)
Changed(this, EventArgs.Empty);
}

But there is a race condition in this code. In a multi threaded application the last handler could be removed exactly between the null-check and the actual call to raise the event. In that specific case a NullReferenceException will be thrown because Changed is null.

One solution to this problem is to copy the (potential) last reference to the subscribed event handler into a seperate local variable. Something like this:

protected virtual void OnChanged()
{
EventHandler handler = Changed;
if (handler != null)
{
handler(this, EventArgs.Empty);
}
}

You capture that (potential) last event handler and test if it is not null. If that check passes, you raise the event. Your code works and no NullReferenceExceptions will be thrown. This is the typical solution to the problem you see suggested by many as the only right way to do it. But I see some problems with it.

The proposed solution does not take into account how this sequence of events impacts the event subscriber. To the event subscriber it is the worst code you can have! Why? Because to the subscriber an event handler that has just been removed from an event can still be called. You don't expect that as a subscriber and you shouldn't have to. This behavior of the event provider is counter-intuitive and can lead to hard (or imposible) to find bugs.

I'm not sure what the correct solution should be. I just wanted to point out the side effect of the solution that 'everybody' holds for the correct solution. For inspiration you should check out this blog post by Roy Osherove. He discusses defensive techniques for raising events.

Maybe something as simple as putting a try-catch block in the OnChanged method is good enough? Make sure you publish those exceptions, though. You'd still want to know about them.