Safely Firing an Event

[Bob Johnson]

In order to avoid a race condition a recommendation that makes a lot of
sense has been put forth.... create a local copy of the event handler
delegate, then test and invoke the copy, like this:

MyEventHandler hCopy = MyEvent;
if hCopy != null)
{
hCopy (this, myEventArgs)
}


But that's not good enough, according to the following blog (fairly brief
entry)...

http://blogs.msdn.com/jaybaz_ms/archive/2004/09/16/230681.aspx

.... because - as you can read there - the JITer may under some circumstances
optimize locals out of existance.


My question: How much validity would any of you place in the above claim
(JITer may optimize locals out of existance)? It's hard to believe that such
a thing could happen in the JITer (i.e., it overrides your coded logic) -
but considering the source... is there any validity to the claim?

The guy's recommended solution seems like a total hack.

I'd appreciate your thoughtful feedback on the above link and the issues and
solutions presented.

 

[Nicholas Paldino [.NET/C# MVP]]

Bob,

Oh, it's completely possible that the JITer will optimize the local out
of existence. However, the recommendation isn't the best one, IMO. If you
are owrried about race conditions, then you really have to take it one step
further, placing a lock around where you fire the event, and then accessing
the same lock in the add and remove handlers for the event:

public class MyClass
{
private object eventlock = new object();

public event MyEventHandler MyEvent
{
add
{
// Lock around the event.
lock (eventlock)
{
// Add the event.
MyEvent += value;
}
}
remove
{
// Lock around the event.
lock (eventLock)
{
// Remove the event.
MyEvent -= value;
}
}
}

private void FireMyEvent(MyEventArgs args)
{
// Lock around the event.
lock (eventLock)
{
// Get the event.
if (MyEvent != null)
{
// Fire the event.
MyEvent(this, args);
}
}
}
}

Basically, with the lock in place, it doesn't matter what optimizations
the JITer puts in place, as the lock will prevent any code from running and
touching the delegate while the shared lock is held.

 

[Marc Gravell]

I would recommend releasing the lock before invoking the event -
otherwise if threaded code attempts to unsubscribe when hearing the
event, it will deadlock.

private void FireMyEvent(MyEventArgs args)
{
EventHandler handler;
lock (eventLock)
{
handler = MyEvent;
}
if (handler!= null)
{
// Fire the event.
handler(this, args);
}
}

 

[Jon Skeet [C# MVP]]

In order to avoid a race condition a recommendation that makes a lot of
sense has been put forth.... create a local copy of the event handler
delegate, then test and invoke the copy, like this:

MyEventHandler hCopy = MyEvent;
if hCopy != null)
{
hCopy (this, myEventArgs)
}

But that's not good enough, according to the following blog (fairly brief
entry)...

http://blogs.msdn.com/jaybaz_ms/archive/2004/09/16/230681.aspx

... because - as you can read there - the JITer may under some circumstances
optimize locals out of existance.

I checked about this a while ago:
http://msmvps.com/blogs/jon.skeet/a...memory-model-and-specific-specifications.aspx

The result is basically, "don't worry about it, it's not actually
going to happen".
I'm not entirely sure, but I *think* the ECMA CLI spec does
technically allow it, but it would break so much code that no
commercial CLR would do it.

The guy's recommended solution seems like a total hack.

You mean creating a no-op event handler using delegate {}? No, that's
not a hack - that's a really elegant way of handling it, IMO.
Admittedly the problem it's getting round isn't as bad as might be
feared, but it's still a neat way of avoiding the nullity check in the
first place.

On the other hand, without a lock (in both the read and the add/
remove) you've still got the potential for firing an "old" set of
event handlers. This is only an issue if you want your class to be
thread-safe, however. I've come to the conclusion that most of the
time you really don't need to subscribe to events from threads other
than the one which will raise them.

Jon