Domanda

Microsoft's tutorial on events shows how to check an event for null before triggering it:

protected virtual void OnChanged(EventArgs e) 
{
    if (Changed != null)
    {   // Potential Race-condition at this point!
        Changed(this, e);
    }
}

But this leaves open a race-condition, as detailed in Eric Lippert's blog, where he writes that events should be triggered via a local event to avoid a race-condition:

protected virtual void OnChanged(EventArgs e) 
{
    ChangedEventHandler temp = Changed;  // Local copy of the EventHandler
    if (temp != null)
    {                                    // Race condition avoided by local variable.
        temp(this, e);
    }
}

While this works, it has confused many developers who get it wrong, and don't put it in the locally-scoped event.


Another solution, from DailyCoding is to always initialize your event to have one empty handler, so a null-check is never needed:

// Set with empty delegate, will never be null
public event ChangedEventHandler Changed = delegate { };

protected virtual void OnChanged(EventArgs e) 
{
    // Neither Null-check nor local variable is needed; just trigger the event.
    Changed(this, e);
}

This one makes a lot of sense, and is pretty simple.
However, since I see this technique so rarely mentioned online, I think there must be a reason why.
Is there a downside to initializing an event with an empty delegate like this?

È stato utile?

Soluzione

You will see an absolutely tiny performance hit, but there are problems in more advanced cases, for example serializing and deserializing the class could lead to you losing the fake event handler, and the lack of a null check then throwing an exception.

Altri suggerimenti

  • It's a slight performance hit if the event would have been empty
  • If you ever write Changed = null in the class, it will break.

In Eric Lippert's blog article you posted, he says:

There are other ways to solve this problem; for example, initializing the handler to have an empty action that is never removed. But doing a null check is the standard pattern.

But before that he also says:

Removing the code around the call site [the null check] does not decrease the number of race conditions in the code [...]. Worse, doing so makes the race condition harder to detect by shrinking the window in which the race can occur without eliminating it.

This is because as he describes, it can still happen

Between the push of the delegate value [onto the stack] and the call to invoke it [...]

So basically, if you use an empty handler, you experience some performance loss (this seems to be the consensus here). So you're what you're gaining is readability, but most importantly: weird behavior will be more apparent. (I deduce this from less performance -> takes longer -> more likely to invoke stale handler) So if you're fully aware that things like these can happen, and the null checking doesn't bother you, go for it. Or don't, if you don't want to.

Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top