A Truly Thread-Safe Event Handler Collection

For quite a while after I moved into the world of .NET I was slightly bugged by event registration in that I had this nagging suspicion that one day it would all go horribly wrong and blow up in my face because I have this incredibly nasty habit of using background threads for long-running, or potentially long-running tasks. So for example, even if I know that retrieving a particular piece of information from a given SQL Server instance should take a couple of hundred milliseconds at most, I worry about what might happen if that instance is sitting on a machine on the other side of the world, or if it goes down and I have to wait for a connection timeout, so I’m anal and spin of a background thread to prevent the UI from locking up, and display a spinny thing or something like this. I tend to call this good engineering but to be honest most people just mock me for it.

But anyway, moving on. The problem arises when I want to fire one or more events from said background thread. What if, at the same time as I’m firing the event, somebody registers or unregisters an event handler? In most cases it’s pretty unlikely, but as I said, I worry about things like this, and there’s not much uglier than the exception that gets thrown when a collection is modified whilst you’re iterating over it using an enumerator. That dialog box that pops up is really unfriendly.

There are two problems though. The first is that like a lot of people I’m a bit lazy and unless something really gets on my nerves, or somebody presents me with a shrink wrapped solution my default behaviour is to do nothing. And the second is that when you’re under time pressure you sometimes don’t worry so much about coming up with a really nice solution to something that might be considered peripheral to the problem at hand.

Step in Jeff Richter. Anyone who’s read a few of my earlier blog posts will know that I’m quite a fan of his book Applied Microsoft .NET Framework Programming. The version I have is somewhat old now, referring to .NET 1.1, but it’s still pretty useful from time to time. There is also a newer version for .NET 2.0 of which there’s a copy floating around the office, and I’d suggest that this is really one of those must have reference books for .NET developers who want to get that bit more serious about what they’re doing. You don’t need it to write for the .NET platform, but you’ll certainly write better .NET applications and make better use of the framework if you have it (and read it obviously).

The point is that in the Events chapter Jeff provides an implementation of an EventHandlerSet class that can be used as a thread-safe dictionary of event handlers for all the events your type defines. So what other advantages are there apart from thread-safety? Well, probably the biggest advantage is that if you have a type that defines a lot of events the CLR doesn’t have to allocate storage space for each event, you just have one member variable of type EventHandlerSet. If you’re not bothered about, or rather don’t need, thread-safety then you could just use the EventHandlerList class in the System.ComponentModel namespace of the FCL, which provides similar functionality.

Anyway, let’s get down to looking at some code. This is pretty much a verbatim copy of Jeff’s EventHandlerSet class, along with the inner synchronized version – all I’ve done is generify it for use with .NET 2.0 (not that you really need to, but it’s much nicer obviously):

using System;
using System.Collections.Generic;
using System.Reflection;
using System.Runtime.CompilerServices;

namespace RedGate.Util
{
    /// <summary>
    /// Manages event handlers for a type.
    /// </summary>
    /// <remarks>
    /// Based on Jeff Richter's <strong>EventHandlerSet</strong> class described
    /// in chapter 11 (Events) of <strong>Applied Microsoft .NET Framework Programming</strong>.
    /// </remarks>
    public class EventHandlerSet
    {

        internal IDictionary< object, Delegate >    m_Handlers    = new Dictionary< object, Delegate >();

        /// <summary>
        /// Accesses the event handlers for the specified event key.
        /// </summary>
        public virtual Delegate this[ object eventKey ]
        {
            get { return m_Handlers.ContainsKey( eventKey ) ? m_Handlers[ eventKey ] : null; }
            set { m_Handlers[ eventKey ] = value; }
        }

        /// <summary>
        /// Adds a handler for the specified event.
        /// </summary>
        /// <param id="eventKey"">Event key.</param>
        /// <param id="handler"">Event handler.</param>
        public virtual void AddHandler( object eventKey, Delegate handler )
        {
            m_Handlers[ eventKey ]    = Delegate.Combine( m_Handlers.ContainsKey( eventKey ) ? m_Handlers[ eventKey ] : null, handler );
        }

        /// <summary>
        /// Removes the specified event handler.
        /// </summary>
        /// <param id="eventKey"">Event key.</param>
        /// <param id="handler"">Event handler.</param>
        public virtual void RemoveHandler( object eventKey, Delegate handler )
        {
            m_Handlers[ eventKey ]    = Delegate.Remove( m_Handlers[ eventKey ], handler );
        }

        /// <summary>
        /// Fires an event.
        /// </summary>
        /// <param id="eventKey"">Event key.</param>
        /// <param id="sender"">Sending object.</param>
        /// <param id="args"">Event arguments.</param>
        [System.Diagnostics.CodeAnalysis.SuppressMessage( "Microsoft.Design", "CA1030:UseEventsWhereAppropriate" )]
        public virtual void Fire( object eventKey, object sender, EventArgs args )
        {
            Delegate del;
            m_Handlers.TryGetValue( eventKey, out del );
            if ( null != del )
            {
                del.DynamicInvoke( new object[] { sender, args } );
            }
        }

        /// <summary>
        /// Gets a synchronized instance of <see cref="EventHandlerSet"/>.
        /// </summary>
        /// <param id="source"">Source event handler set.</param>
        /// <returns>Synchronized <see cref="EventHandlerSet"/>.</returns>
        /// <exception cref="ArgumentNullException">
        /// <strong>source</strong> is <strong>null</strong>.
        /// </exception>
        public static EventHandlerSet Synchronized( EventHandlerSet source )
        {
            if ( null == source )
            {
                throw new ArgumentNullException( "source" );
            }
            else if ( source is SynchronizedEventHandlerSet )
            {
                return source;
            }
            else
            {
                return new SynchronizedEventHandlerSet( source );
            }
        }

        private class SynchronizedEventHandlerSet : EventHandlerSet
        {
           
            private EventHandlerSet    m_Unsynchronized;

            internal SynchronizedEventHandlerSet( EventHandlerSet unsynchronized )
            {
                m_Unsynchronized    = unsynchronized;
                m_Handlers            = null;
            }

            public override Delegate this[ object eventKey ]
            {
                [ MethodImpl( MethodImplOptions.Synchronized ) ]
                get { return m_Unsynchronized[ eventKey ]; }
                [ MethodImpl( MethodImplOptions.Synchronized ) ]
                set { m_Unsynchronized[ eventKey ] = value; }
            }

            [ MethodImpl( MethodImplOptions.Synchronized ) ]
            public override void AddHandler( object eventKey, Delegate handler )
            {
                m_Unsynchronized.AddHandler( eventKey, handler );
            }

            [ MethodImpl( MethodImplOptions.Synchronized ) ]
            public override void RemoveHandler( object eventKey, Delegate handler )
            {
                m_Unsynchronized.RemoveHandler( eventKey, handler );
            }

            [ MethodImpl( MethodImplOptions.Synchronized ) ]
            public override void Fire( object eventKey, object sender, EventArgs args )
            {
                m_Unsynchronized.Fire( eventKey, sender, args );
            }
        }
    }
}

I’ve used this class in a couple of projects and it’s been great to be honest, however like most things it’s not without its drawbacks. For example, if you like to protect your code with StrongNameIdentityPermissionAttribute then you’re going to run into problems with it because it uses reflection to fire the events. What happens is that the LinkDemand security action gets promoted to a Demand action causing a full stack walk when the event is fired rather than just causing a check at link time, and because reflection calls through mscorlib which doesn’t have the required permission before calling back into your protected code it’ll all blow up in your face with a SecurityException.

Another thing is that because of the reflection any exception thrown by an event handler will be wrapped in a TargetInvocationException, so if you want to do any handling for specific exception types you need to unwrap the exception inside the TargetInvocationException and check its type, and this is obviously a bit ugly as you might need to rethrow either it or the TargetInvocationException if you are unable to deal with the inner exception rather than just not writing a catch block to handle it.

However there is a much more insidious problem lurking in the shadows. The above code is absolutely fine if you’re only ever firing events from a single thread for a given instance of SynchronizedEventHandlerSet, however the moment you start firing events from more than one thread or registering events on the same instance in an event handler at the same time as another thread might be registering an event, you may very well run into a deadlock, and this is exactly what I found several months back with SQL Prompt. Fortunately it didn’t take too long to find and I felt a mixture of disappointment and relief when in a roundabout sort of way it turned out to be the classic deadlock scenario: thread 1 has the lock for A and needs the lock for B, thread 2 has the lock for B and needs the lock for A, and neither can make progress. The call stacks did their best to obfuscate the situation, but that’s basically what it boiled down to.

To explain a little further in the context of this type: say you’re firing a event from thread 1. Thread 1 has the lock for your SynchronizedEventHandlerSet and holds on to it until it’s finished firing the event. Now thread 2 comes along and needs to fire an event, but of course it blocks because thread 1 has the lock for the SynchronizedEventHandlerSet. No big deal so far, but oh, I forgot to mention, thread 2 also has the lock for object O and now thread 1 wants that lock as well, and oops, it’s all gone horribly pear-shaped. Back to the drawing board methinks.

Now it turns out this isn’t too difficult to fix, and in fact all I did was modify the SynchronizedEventHandlerSet class by getting rid of the [ MethodImpl( MethodImplOptions.Synchronized ) ] attributes, and instead using a lock object. I then modified the Fire() method to create a copy of the relevant delegate list whilst holding the lock, and then fire the event to each method individually after releasing the lock. You could just as easily wrap the copying code in a private method and mark that method with a MethodImpl attribute if you want.

Here’s my modified version of the SynchronizedEventHandlerSet inner class:

private class SynchronizedEventHandlerSet : EventHandlerSet
{
           
    private readonly object    m_Lock                = new object();
    private EventHandlerSet    m_Unsynchronized;

    internal SynchronizedEventHandlerSet( EventHandlerSet unsynchronized )
    {
        m_Unsynchronized    = unsynchronized;
        m_Handlers            = null;
    }

    public override Delegate this[ object eventKey ]
    {
        get
        {
            lock ( m_Lock )
            {
                return m_Unsynchronized[ eventKey ];
            }
        }
        set
        {
            lock ( m_Lock )
            {
                m_Unsynchronized[ eventKey ] = value;
            }
        }
    }

    public override void AddHandler( object eventKey, Delegate handler )
    {
        lock ( m_Lock )
        {
            m_Unsynchronized.AddHandler( eventKey, handler );
        }
    }

    public override void RemoveHandler( object eventKey, Delegate handler )
    {
        lock ( m_Lock )
        {
            m_Unsynchronized.RemoveHandler( eventKey, handler );
        }
    }

    public override void Fire( object eventKey, object sender, EventArgs args )
    {
        Delegate []    methods    = null;
        lock ( m_Lock )
        {
            Delegate    target    = m_Unsynchronized[ eventKey ];
            if ( null != target )
            {
                methods    = target.GetInvocationList();
            }
        }
               
        if ( null == methods || methods.Length == 0 )
        {
            return;
        }
               
        object []    parms    = new object[] { sender, args };
        foreach ( Delegate current in methods )
        {
            current.DynamicInvoke( parms );
        }
    }
}

Not very tricky as I’m sure you’ll agree.

Now obviously it still has the reflection related drawbacks I discussed earlier, but the deadlocking problem has gone away. There is however one side-effect to these modifications, which is that it’s now possible to receive an event notification after you’ve removed an event handler if the removal occurred immediately after the copy of the delegate list was made but before (or during) the firing of the event to the individual methods. Now exactly how much of a drawback this is I’ll leave you to decide, and it will very much depend on the project you’re working on, but I’d say generally if you’re working in a multi-threaded environment it’s smart (and safer) to assume that you might possibly receive an event notification after you’ve unregistered your event handler than not too. Clearly if that happens you need to handle it gracefully, rather than blowing up or corrupting your application’s state.

Anyway, that’s all for now. Hopefully somebody out there will find this useful.

Sayonara!