Why copy an event handler into another variable

Question

In an example, I find this code:

public event EventHandler ThresholdReached;

protected virtual void OnThresholdReached(EventArgs e)
{
  EventHandler handler = ThresholdReached;
  if (handler != null)
            handler(this, e);
}

I would like to understand the reason for the line:

EventHandler handler = ThresholdReached;

Can we not just do it this way:

public event EventHandler ThresholdReached;

protected virtual void OnThresholdReached(EventArgs e)
{
  if (ThresholdReached != null)
            ThresholdReached(this, e);
}

Are there some advantages/disadvantages to either way of doing it?

Answer 1

The problem is that between this line

if (ThresholdReached != null)

and this line

ThresholdReached(this, e);

a different thread could have removed the handlers from the ThresholdReached event. So it would be null and an exception would be thrown.

By storing the value in a local variable, you make the call thread-safe.

Since C# 6 you can shorten the code to this

ThresholdReached?.Invoke(this, e);

Now the compiler takes care of storing the value in a temporary variable.

Answer 2

There's a theoretical risk of a thread race in the second version where someone unsubscribes the event between the check and the invoke, causing a NullReferenceException in the invoke step. Capturing the value into a local and testing/invoking that prevents this. However, perhaps use the third version, possible with C# 6 or above (thanks @Cid):

ThresholdReached?.Invoke(this, e);

This is basically a short-hand version of the first version - all the safety, but now with terseness.