Is it a good idea to implement a C# event with a weak reference under the hood?

Question

I have been wondering whether it would be worth implementing weak events (where they are appropriate) using something like the following (rough proof of concept code):

class Foo {

    private WeakEvent<EventArgs> _explodedEvent = new WeakEvent<EventArgs>();

    public event WeakEvent<EventArgs>.EventHandler Exploded {
        add { _explodedEvent += value; }
        remove { _explodedEvent -= value; }
    }

    private void OnExploded() {
        _explodedEvent.Invoke(this, EventArgs.Empty);
    }

    public void Explode() {
        OnExploded();
    }

}

Allowing other classes to subscribe and unsubscribe from events with the more conventional C# syntax whilst under the hood actually being implemented with weak references:

static void Main(string[] args) {
    var foo = new Foo();
    foo.Exploded += (sender, e) => Console.WriteLine("Exploded!");

    foo.Explode();
    foo.Explode();
    foo.Explode();

    Console.ReadKey();
}

Where the WeakEvent<TEventArgs> helper class is defined as follows:

public class WeakEvent<TEventArgs> where TEventArgs : EventArgs {

    public delegate void EventHandler(object sender, TEventArgs e);

    private List<WeakReference> _handlers = new List<WeakReference>();

    public void Invoke(object sender, TEventArgs e) {
        foreach (var handler in _handlers)
            ((EventHandler)handler.Target).Invoke(sender, e);
    }

    public static WeakEvent<TEventArgs> operator + (WeakEvent<TEventArgs> e, EventHandler handler) {
        e._handlers.Add(new WeakReference(handler));
        return e;
    }

    public static WeakEvent<TEventArgs> operator - (WeakEvent<TEventArgs> e, EventHandler handler) {
        e._handlers.RemoveAll(x => (EventHandler)x.Target == handler);
        return e;
    }

}

Is this a good approach? are there any undesirable side effects to this approach?

Have you checked out the WeakEventManager? https://msdn.microsoft.com/en-us/library/vstudio/aa970850(v=vs.100).aspx — lintmouse, Sep 04 '15 at 20:27
@quetzalcoatl no, and to the close-voter: no. This is a good one for this site. — AgentFire, Sep 04 '15 at 20:46
It's **nearly** a good fit for CodeReview, but example code is off-topic. — Kaz, Sep 04 '15 at 20:47

score 3 · Accepted Answer · edited May 23 '17 at 12:22

That's a bad idea because:

Your program starts to become non-deterministic because side-effects depend on the actions of the GC.
GCHandles come at a performance cost.

See the linked answer. It's a 95% duplicate but not quite enough to close the question I think. I'll quote the most relevant parts:

There also is a semantic difference and non-determinism that would be caused by weak references. If you hook up () => LaunchMissiles() to some event you might find the missiles to be launched just sometimes. Other times the GC has already taken away the handler. This could be solved with dependent handles which introduce yet another level of complexity.

I personally find it rare that the strong referencing nature of events is a problem. Often, events are hooked up between objects that have the same or very similar lifetime. For example you can hook up events all you want in the context of an HTTP request in ASP.NET because everything will be eligible for collection when the request has ended. Any leaks are bounded in size and short lived.

Erik · Answer 2 · 2015-09-04T20:45:55.083

A few comments about your particular implementation:

Check the value of handler.Target for null before invoking it so that you don't try to do it with an object that has been disposed.
C# has special access rules for how you can use events. You cannot do a.Event1 = a.Event2 + SomeOtherMethod unless that code has private access to the events. This is allowed for delegates however. Your implementation behaves much more like a delegate instead of an event. This is probably not a major concern but something to think about.
Your operator methods should return a new object instead of modifying the first argument and returning it. Implementing operator + allows for syntax like the following: a = b + c, but in your implementation, you are modifying the state of b!. This is not kosher for how one would expect these operators to work; you need to return a new object instead of modifying the existing one. (Also, because of this your implementation is not thread-safe. Calling operator + from one thread while another was raising the event would raise an exception because the collection was modified during the foreach.)

Agreed on the operator thing; like I said; that's just super rough; I think that I would use Add and Remove methods probably instead. My question is posed more towards; is this actually a good approach in general? — Lea Hayes, Sep 04 '15 at 21:00

Is it a good idea to implement a C# event with a weak reference under the hood?

2 Answers2