How do I fire an event safely

Question

When there are no subscribers to an event how do I ensure that an exception will not be thrown if the event is fired.

 // Delegate declaration
 public delegate void _delDisplayChange(object sender,string option);

 // Event declaration
 public event _delDisplayChange  DisplayChange;

 //throwing the event
 DisplayChange(this, "DISTRIBUTION");

I've posted a question on this topic a while ago and got quite a bit of flame for it too, but it may be a good place to clarify some of your questions: http://stackoverflow.com/questions/840715/the-proper-way-of-raising-events-in-the-net-framework — Mike Dinescu, Dec 08 '09 at 18:31
Note that successfully avoiding the exception is only the START of your worries, not the END. If you are in a multi-threaded situation, you also need to worry about the *entirely orthogonal* problem of whether you are invoking a stale event handler, and if so, whether you deal with that situation correctly. See http://blogs.msdn.com/ericlippert/archive/2009/04/29/events-and-races.aspx for details. — Eric Lippert, Dec 08 '09 at 19:42
Possible duplicate of [C# Events and Thread Safety](https://stackoverflow.com/questions/786383/c-sharp-events-and-thread-safety) — , Mar 29 '18 at 15:15

Brian Genisio · Accepted Answer · 2009-12-08T18:34:36.993

Here is the recommended way to do it:

protected void RaiseDisplayChanged(string message)
{
    var handlers = DisplayChange;
    if(handlers != null)
        handlers(this, message);
}

Copying the event handlers enumeration before checking does two things:

If DisplayChange handlers becomes null between the check and the firing, you don't die
If the listeners modify the DisplayChange list while enumerating through it, you don't run into oddities.

Also, you are not using the standard Event protocol. Your delegate should be:

public delegate void DisplayChangeDelegate(object sender, OptionsEventArgs args);

Where OptionsEventArgs derives from EventArgs. Going a step further, in .Net 3.5, you should never define a delegate like this. Instead, you should just define your event:

public event EventHandler<OptionsEventArgs> DisplayChanged;

I like to take it even one step further by defining this class:

public class EventArgs<T> : EventArgs
{
    public T Payload { get; private set }
    public EventArgs(T payload)
    {
        Payload = payload;
    }
}

Then, you don't need to define OptionsEventArgs:

public event EventHandler<EventArgs<string>> DisplayChanged;

Just some stuff to think about...

Good answer, but it's also worth explaining why the backing field can become `null` (multithreading), and why it's worth making event registration and raising thread-safe to begin with. — Pavel Minaev, Dec 08 '09 at 18:41
Should consider making `RaiseDisplayChanged(string)` virtual, so derived classes can override the event's internal behavior. — walkingTarget, May 06 '14 at 16:57

score 4 · Answer 2 · answered Dec 08 '09 at 18:24

4

Change this:

// Event declaration      
public event _delDisplayChange  DisplayChange;

to this:

// Event declaration      
public event _delDisplayChange  DisplayChange = delegate{};

This will ensure your event will always have at least one subscriber.

answered Dec 08 '09 at 18:24

Jay Riggs

53,046
9
139
151

No need to add a fictitious subscriber that takes memory and does not obey any logic; checking for null is more consistent with the rationale of event/subscriber logic. – CesarGon Dec 08 '09 at 18:27
It is an interesting twist, but like Cesar said, there are better ways. – Pierre-Alain Vigeant Dec 08 '09 at 18:28
What is the "rationale of event/subscriber logic", and why checking for null is consistent with it? – Pavel Minaev Dec 08 '09 at 18:40
The rationale of event/subscriber logic is that subscribers subscribe to events, and an event with no subscribers has a null subscriber list, and therefore fails if invoked (that's how .NET implements it, anyway). Checking whether the subscriber list is empty or not before firing an event is coherent with this. Adding a fake subscriber just to avoid making the check is a quick & dirty trick, which works in the short term, but is not coherent with the underlying semantics of event/subscriber. It's akin to altering the value of a loop variable inside a loop: it can work, but it's not on. – CesarGon Dec 08 '09 at 22:35
+1 adding an empty subscriber is NOT a dirty trick. This is essentially the null object pattern: you replace the special handling of null by an object (in this case a delegate) which has the same effect. The null object pattern is the more object oriented approach. http://en.wikipedia.org/wiki/Null_Object_pattern – Wim Coenen Dec 09 '09 at 22:57
@Wim Coenen: While event subscription/unsubscription usually isn't a huge bottleneck, it's much faster to combine a delegate with null, than to combine two non-null delegates. It's also faster to check whether a delegate is null, than to invoke a non-null do-nothing delegate. Since most events will have zero or one subscribers, adding a dummy subscriber represents a substantial pessimization in the cases. – supercat Aug 27 '11 at 17:28
from the wiki-link: "Care should be taken not to implement this pattern just to avoid null checks and make code more readable" ...it goes on to explain why. – AnorZaken Nov 07 '14 at 14:09

Anton · Answer 3 · 2009-12-09T07:18:36.977

1

As Brian said: many sources suggest making a copy of the event before checking that it is not null:

_delDisplayChange displayChangeCopy = DisplayChange;
if (displayChangeCopy != null)
    displayChangeCopy(this, "DISTRIBUTION");

This helps to make your code more thread-safe because the value of displayChangeCopy will not change between the null check and the invocation.

edited Dec 09 '09 at 07:18

answered Dec 08 '09 at 18:26

Anton

3,170
20
20

2

This does NOT make your code thread-safe. It eliminates ONE race condition. It does not eliminate the more general race condition, which is that between the storage of the value of DisplayChange and the invocation of the local copy, *the contents DisplayChange could have changed*, and therefore *you are now invoking a function which might rely upon state that has been destroyed*. Don't go making claims that code is "thread safe" without clearly describing what thread danger you are mitigating. You haven't mitigated nearly all the dangers here. – Eric Lippert Dec 08 '09 at 19:48

How do I fire an event safely

3 Answers3

Linked