Is this a valid pattern for raising events in C#?
- by Will Vousden
Update: For the benefit of anyone reading this, since .NET 4, the lock is unnecessary due to changes in synchronization of auto-generated events, so I just use this now:
public static void Raise<T>(this EventHandler<T> handler, object sender, T e) where T : EventArgs
{
if (handler != null)
{
handlerCopy(sender, e);
}
}
And to raise it:
SomeEvent.Raise(this, new FooEventArgs());
Having been reading one of Jon Skeet's articles on multithreading, I've tried to encapsulate the approach he advocates to raising an event in an extension method like so (with a similar generic version):
public static void Raise(this EventHandler handler, object @lock, object sender, EventArgs e)
{
EventHandler handlerCopy;
lock (@lock)
{
handlerCopy = handler;
}
if (handlerCopy != null)
{
handlerCopy(sender, e);
}
}
This can then be called like so:
protected virtual void OnSomeEvent(EventArgs e)
{
this.someEvent.Raise(this.eventLock, this, e);
}
Are there any problems with doing this?
Also, I'm a little confused about the necessity of the lock in the first place. As I understand it, the delegate is copied in the example in the article to avoid the possibility of it changing (and becoming null) between the null check and the delegate call. However, I was under the impression that access/assignment of this kind is atomic, so why is the lock necessary?
Update: With regards to Mark Simpson's comment below, I threw together a test:
static class Program
{
private static Action foo;
private static Action bar;
private static Action test;
static void Main(string[] args)
{
foo = () => Console.WriteLine("Foo");
bar = () => Console.WriteLine("Bar");
test += foo;
test += bar;
test.Test();
Console.ReadKey(true);
}
public static void Test(this Action action)
{
action();
test -= foo;
Console.WriteLine();
action();
}
}
This outputs:
Foo
Bar
Foo
Bar
This illustrates that the delegate parameter to the method (action) does not mirror the argument that was passed into it (test), which is kind of expected, I guess. My question is will this affect the validity of the lock in the context of my Raise extension method?
Update: Here is the code I'm now using. It's not quite as elegant as I'd have liked, but it seems to work:
public static void Raise<T>(this object sender, ref EventHandler<T> handler, object eventLock, T e) where T : EventArgs
{
EventHandler<T> copy;
lock (eventLock)
{
copy = handler;
}
if (copy != null)
{
copy(sender, e);
}
}
