Are there pitfalls to using static class/event as an application message bus

Question

I have a static generic class that helps me move events around with very little overhead:

public static class MessageBus<T> where T : EventArgs
{
    public static event EventHandler<T> MessageReceived;
    public static void SendMessage(object sender, T message)
    {
        if (MessageReceived != null)
            MessageReceived(sender, message);
    }
}

To create a system-wide message bus, I simply need to define an EventArgs class to pass around any arbitrary bits of information:

class MyEventArgs : EventArgs
{
    public string Message { get; set; }
}

Anywhere I'm interested in this event, I just wire up a handler:

MessageBus<MyEventArgs>.MessageReceived += (s,e) => DoSomething();

Likewise, triggering the event is just as easy:

MessageBus<MyEventArgs>.SendMessage(this, new MyEventArgs() {Message="hi mom"});

Using MessageBus and a custom EventArgs class lets me have an application wide message sink for a specific type of message. This comes in handy when you have several forms that, for example, display customer information and maybe a couple forms that update that information. None of the forms know about each other and none of them need to be wired to a static "super class".

I have a couple questions:

1. fxCop complains about using static methods with generics, but this is exactly what I'm after here. I want there to be exactly one MessageBus for each type of message handled. Using a static with a generic saves me from writing all the code that would maintain the list of MessageBus objects.

2. Are the listening objects being kept "alive" via the MessageReceived event?

For instance, perhaps I have this code in a Form.Load event:

MessageBus<CustomerChangedEventArgs>.MessageReceived += (s,e) => DoReload();

When the Form is Closed, is the Form being retained in memory because MessageReceived has a reference to its DoReload method? Should I be removing the reference when the form closes:

MessageBus<CustomerChangedEventArgs>.MessageReceived -= (s,e) => DoReload();

Answer 1

Well, yes, you should, but if you use the lambda syntax as you've done in your example I think it won't work (by which I mean, the handler will not be de-registered successfully).

Someone correct me if I'm wrong, but I believe this is true because using the lambda syntax effectively creates a new EventHandler<CustomerChangedEventArgs> object, with its own place in memory. When you try to remove this handler, using the lambda syntax again, this creates yet another new EventHandler<CustomerChangedEventArgs> object, which is not equal to the first one you created; and so the first one never gets de-registered.

Sadly, I think you'll need to actually define a method like this:

DoReload(object sender, CustomerChangedEventArgs e) {
    DoReload(); // your original overload, which doesn't actually care
                // about the sender and e parameters
}

This way you can do:

MessageBus<CustomerChangedEventArgs>.MessageReceived += DoReload;

And later:

MessageBus<CustomerChangedEventArgs>.MessageReceived -= DoReload;

Answer 2

Yes, there are problems. Your event handlers will cause the form object to stay referenced, you have to explicitly unregister the event handlers. The lambdas make this impossible, you'll have to write an explicit handler.

This pattern has a name, "Event Broker service". It is part of the Composite UI Application Block, published by Microsoft's Pattern and Practices team. Beg, borrow and steal (if not use) what you can from this.

Answer 3

You could use weak references to store the event handlers. That way, unhooked handlers won't prevent garbage collection of the objects.

public static class MessageBus<T> where T : EventArgs
{
    private static List<WeakReference> _handlers = new List<WeakReference>();

    public static event EventHandler<T> MessageReceived
    {
        add
        {
            _handlers.Add(new WeakReference(value));
        }
        remove
        {
            // also remove "dead" (garbage collected) handlers
            _handlers.RemoveAll(wh => !wh.IsAlive  || wh.Target.Equals(value));
        }
    }

    public static void SendMessage(object sender, T message)
    {
        foreach(var weakHandler in _handlers)
        {
            if (weakHandler.IsAlive)
            {
                var handler = weakHandler.Target as EventHandler<T>;
                handler(sender, message);
            }
        }            
    }
}