Is this a valid pattern for raising events in C#?

Posted by Will Vousden on Stack Overflow See other posts from Stack Overflow or by Will Vousden
Published on 2010-01-23T15:17:01Z Indexed on 2010/05/01 15:27 UTC
Read the original article Hit count: 282

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);
    }
}

© Stack Overflow or respective owner

Related posts about c#

Related posts about events