views:

381

answers:

4

So i've read around that instead of calling a event directly with

if (SomeEvent != null)
   SomeEvent(this, null);

i should be doing

SomeEventHandler temp = SomeEvent;
if (temp != null)
    temp(this, null);

Why is this so? How does the second version become thread safe? What is the best practice?

+4  A: 

Best practice is the second form. The reason is that another thread might null or alter SomeEvent between the 'if' test and the invocation.

Mitch Wheat
Why can't that happen in the second statement?
Daniel
The read is of `SomeEvent` is atomic, i.e. it happens all the way or nothing. Thus `temp` cannot be modified outside that thread due to being a local.
sixlettervariables
Ooops `s/Thus/Also/`
sixlettervariables
+1  A: 

Here is a good write up about .NET events and race conditions with threads. It covers some common scenarios and has some good references in it.

Hope this helps.

Brian ONeil
Thanks for the link
Daniel
+7  A: 

Events are really syntactic sugar over a list of delegates. When you invoke the event, this is really iterating over that list and invoking each delegate with the parameters you have passed.

The problem with threads is that they could be adding or removing items from this collection by subscribing/unsubscribing. If they do this while you are iterating the collection this will cause problems (I think an exception is thrown)

The intent is to copy the list before iterating it, so you are protected against changes to the list.

Note: It is however now possible for your listener to be invoked even after you unsubscribed, so you should make sure you handle this in your listener code.

Nigel Thorne
Actually, it will not cause a problem while you iterate over the collection. The delegates in play here are immutable. The only problem is the split second before checking if anyone has subscribed and actually calling the event handlers. Once you've started executing those, no background changes will affect the current invocation.
Lasse V. Karlsen
+9  A: 

IMO, the other answers miss one key detail - that delegates (and therefore events) are immutable. The significance of this is that subscribing or unsubscribing an event handler doesn't simply append/remove to a list - rather, it replaces the list with a new one with an extra (or one less) item on it.

Since references are atomic, this means that at the point you do:

var handler = SomeEvent;

you now have a rigid instance that cannot change, even if in the next picosecond another thread unsubscribes (causing the actual event field to become null).

So you test for null and invoke it, and all is well. Note of course that there is still the confusing scenario of the event being raised on an object that thinks it unsubscribed a picosecond ago!

Marc Gravell