Something that confuses me, but has never caused any problems... the recommended way to dispatch an event is as follows:
public event EventHandler SomeEvent;
...
{
....
if(SomeEvent!=null)SomeEvent();
}
In a multi-threaded environment, how does this code guarantee that another thread will not alter the invocation list of SomeEvent between the check for null and the invocation of the event?
As you point out, where multiple threads can access SomeEvent simultaneously, one thread could check whether SomeEventis null and determine that it isn't. Just after doing so, another thread could remove the last registered delegate from SomeEvent. When the first thread attempts to raise SomeEvent, an exception will be thrown. A reasonable way to avoid this scenario is:
protected virtual void OnSomeEvent(EventArgs args)
{
EventHandler ev = SomeEvent;
if (ev != null) ev(this, args);
}
This works because whenever a delegate is added to or removed from an event using the default implementations of the add and remove accessors, the Delegate.Combine and Delegate.Remove static methods are used. Each of these methods returns a new instance of a delegate, rather than modifying the one passed to it.
In addition, assignment of an object reference in .NET is atomic, and the default implementations of the add and remove event accessors are synchronised. So the code above succeeds by first copying the multicast delegate from the event to a temporary variable. Any changes to SomeEvent after this point will not affect the copy you've made and stored. Thus you can now safely test whether any delegates were registered and subsequently invoke them.
Note that this solution solves one race problem, namely that of an event handler being null when it's invoked. It doesn't handle the problem where an event handler is defunct when it's invoked, or an event handler subscribes after the copy is taken.
For example, if an event handler depends on state that's destroyed as soon as the handler is un-subscribed, then this solution might invoke code that cannot run properly. See Eric Lippert's excellent blog entry for more details. Also, see this StackOverflow question and answers.
EDIT: If you're using C# 6.0, then Krzysztof's answer looks like a good way to go.
In C# 6.0 you can use monadic Null-conditional operator ?. to check for null and raise events in easy and thread-safe way.
SomeEvent?.Invoke(this, args);
It’s thread-safe because it evaluates the left-hand side only once, and keeps it in a temporary variable. You can read more here.
The simplest way remove this null check is to assign the eventhandler to an anonymous delegate. The penalty incurred in very little and relieves you of all the null checks, race conditions etc.
public event EventHandler SomeEvent = delegate {};
Related question: Is there a downside to adding an anonymous empty delegate on event declaration?
The recommended way is a little different and uses a temporary as follows:
EventHandler tmpEvent = SomeEvent;
if (tmpEvent != null)
{
tmpEvent();
}
Safer approach:
public class Test
{
private EventHandler myEvent;
private object eventLock = new object();
private void OnMyEvent()
{
EventHandler handler;
lock(this.eventLock)
{
handler = this.myEvent;
}
if (handler != null)
{
handler(this, EventArgs.Empty);
}
}
public event MyEvent
{
add
{
lock(this.eventLock)
{
this.myEvent += value;
}
}
remove
{
lock(this.eventLock)
{
this.myEvent -= value;
}
}
}
}
-bill
I would like to suggest an slight improvment to RoadWarrior's answer by utilizing an extention function for the EventHandler:
public static class Extensions
{
public static void Raise(this EventHandler e, object sender, EventArgs args = null)
{
var e1 = e;
if (e1 != null)
{
if (args == null)
args = new EventArgs();
e1(sender, args);
}
}
}
With this extension in scope, events can be raised simply by:
class SomeClass { public event EventHandler MyEvent;
void SomeFunction()
{
// code ...
//---------------------------
MyEvent.Raise(this);
//---------------------------
}
}
