views:

152

answers:

6

The following code snippet is from book Effective C#,

public event AddMessageEventHandler Log;

public void AddMsg ( int priority, string msg )

{
    // This idiom discussed below.
    AddMessageEventHandler l = Log;
    if ( l != null )
        l ( null, new LoggerEventArgs( priority, msg ) );
}

The AddMsg method showsthe proper way to raise events. The temporary variable to reference the log event handler is an important safeguard against race conditions in multithreaded programs. Without the copy of the reference, clients could remove event handlers between the if statement check and the execution of the event handler. By copying the reference, that can't happen.

Why temporary variable can stop client from removing event handler? I must be missing something here, thanks

+2  A: 

Delegate chains are immutable. Therefore, if another thread accesses "Log" and removes an eventhandler, Log gets assigned a new delegate chain. Therefore, when l is accessed, even if an eventhandler is removed from Log, it won't effect l as it will no longer be "pointing" to the same delegate chain. So yes, it does protect against race conditions, however you might end up with a scenario where one thread unsubscribes, but the evanthandler will still be called.

BFree
+1  A: 

The client can still remove the event handler.

Bad:

if ( Log != null )
{
  //another thread removes the event handler at this point,
  // Log is now null
  Log ( null, new LoggerEventArgs( priority, msg ) );
}

Good:

AddMessageEventHandler l = Log; 
if ( l != null )
  //another thread removes the event handler at this point,
  // Log is now null
  // l is not null, so we are safe.
l ( null, new LoggerEventArgs( priority, msg ) );
David B
A: 

Because when you assign it you create a local reference to the object, you then check that the reference exists before proceeding. If you just checked if Log was not null then proceeded to the next statement, another thread could null the Log object between the 2 instructions

If(Log != null) // another thread could null the reference to Log here l.DoSomething()

AddMessageEventHandler l = Log; if ( l != null ) // nothing can null the reference here as you’ve created a local copy of that reference l ( null, new LoggerEventArgs( priority, msg ) ); }

Keith
+1  A: 

It doesn't stop the client from removing the event handler - it just means that you'll call that event handler anyway.

The important bit you may be missing is that delegates are immutable - when an event handler is removed, the value of Log will change to be the new delegate or null. That's okay though, because by that stage you're using 1 instead of Log.

Jon Skeet
A: 

It doesn't stop the client removing the event handler - it just means that if they do you don't end up invoking a null delegate... consider:

  • thread A: checks that Log isn't null
  • thread B: unsubscribes, causing Log to become null
  • thread A: invokes Log (which is now null) = boom

Of course, with the above fix you now have a phantom invoke instead - i.e. the subscriber can have the event invoked after they unsubscribed... gotta love threading.

As always, Eric Lippert has a blog on the subject: Events and Races

Marc Gravell
A: 

The text you bolded explains what can go wrong.

Without the copy of the reference, clients could remove event handlers between the if statement check and the execution of the event handler.

A second thread might remove its event handler from Log while your AddMsg method is running. For example:

if (Log != null)
{
    // other thread has just removed its event handler, so Log is now null

    Log(null, new LoggerEventArgs(priority, msg)); // Oops! Throws NullReferenceException
}
Zr40