Using WeakReference to resolve issue with .NET unregistered event handlers causing memory leaks

Question

The problem: Registered event handlers create a reference from the event to the event handler's instance. If that instance fails to unregister the event handler (via Dispose, presumably), then the instance memory will not be freed by the garbage collector.

Example:

    class Foo
    {
        public event Action AnEvent;
        public void DoEvent()
        {
            if (AnEvent != null)
                AnEvent();
        }
    }        
    class Bar
    {
        public Bar(Foo l)
        {
            l.AnEvent += l_AnEvent;
        }

        void l_AnEvent()
        {

        }            
    }

If I instantiate a Foo, and pass this to a new Bar constructor, then let go of the Bar object, it will not be freed by the garbage collector because of the AnEvent registration.

I consider this a memory leak, and seems just like my old C++ days. I can, of course, make Bar IDisposable, unregister the event in the Dispose() method, and make sure to call Dispose() on instances of it, but why should I have to do this?

I first question why events are implemented with strong references? Why not use weak references? An event is used to abstractly notify an object of changes in another object. It seems to me that if the event handler's instance is no longer in use (i.e., there are no non-event references to the object), then any events that it is registered with should automatically be unregistered. What am I missing?

I have looked at WeakEventManager. Wow, what a pain. Not only is it very difficult to use, but its documentation is inadequate (see http://msdn.microsoft.com/en-us/library/system.windows.weakeventmanager.aspx -- noticing the "Notes to Inheritors" section that has 6 vaguely described bullets).

I have seen other discussions in various places, but nothing I felt I could use. I propose a simpler solution based on WeakReference, as described here. My question is: Does this not meet the requirements with significantly less complexity?

To use the solution, the above code is modified as follows:

    class Foo
    {
        public WeakReferenceEvent AnEvent = new WeakReferenceEvent();

        internal void DoEvent()
        {
            AnEvent.Invoke();
        }
    }

    class Bar
    {
        public Bar(Foo l)
        {
            l.AnEvent += l_AnEvent;
        }

        void l_AnEvent()
        {

        }
    }

Notice two things: 1. The Foo class is modified in two ways: The event is replaced with an instance of WeakReferenceEvent, shown below; and the invocation of the event is changed. 2. The Bar class is UNCHANGED.

No need to subclass WeakEventManager, implement IWeakEventListener, etc.

OK, so on to the implementation of WeakReferenceEvent. This is shown here. Note that it uses the generic WeakReference<T> that I borrowed from here: http://damieng.com/blog/2006/08/01/implementingweakreferencet

class WeakReferenceEvent
{
    public static WeakReferenceEvent operator +(WeakReferenceEvent wre, Action handler)
    {
        wre._delegates.Add(new WeakReference<Action>(handler));
        return wre;
    }

    List<WeakReference<Action>> _delegates = new List<WeakReference<Action>>();

    internal void Invoke()
    {
        List<WeakReference<Action>> toRemove = null;
        foreach (var del in _delegates)
        {
            if (del.IsAlive)
                del.Target();
            else
            {
                if (toRemove == null)
                    toRemove = new List<WeakReference<Action>>();
                toRemove.Add(del);
            }
        }
        if (toRemove != null)
            foreach (var del in toRemove)
                _delegates.Remove(del);
    }
}

It's functionality is trivial. I override operator + to get the += syntactic sugar matching events. This creates WeakReferences to the Action delegate. This allows the garbage collector to free the event target object (Bar in this example) when nobody else is holding on to it.

In the Invoke() method, simply run through the weak references and call their Target Action. If any dead (i.e., garbage collected) references are found, remove them from the list.

Of course, this only works with delegates of type Action. I tried making this generic, but ran into the missing where T : delegate in C#!

As an alternative, simply modify class WeakReferenceEvent to be a WeakReferenceEvent<T>, and replace the Action with Action<T>. Fix the compiler errors and you have a class that can be used like so:

    class Foo
    {
        public WeakReferenceEvent<int> AnEvent = new WeakReferenceEvent<int>();

        internal void DoEvent()
        {
            AnEvent.Invoke(5);
        }
    }

The full code with <T>, and the operator - (for removing events) is shown here:

class WeakReferenceEvent<T>
{
    public static WeakReferenceEvent<T> operator +(WeakReferenceEvent<T> wre, Action<T> handler)
    {
        wre.Add(handler);
        return wre;
    }
    private void Add(Action<T> handler)
    {
        foreach (var del in _delegates)
            if (del.Target == handler)
                return;
        _delegates.Add(new WeakReference<Action<T>>(handler));
    }

    public static WeakReferenceEvent<T> operator -(WeakReferenceEvent<T> wre, Action<T> handler)
    {
        wre.Remove(handler);
        return wre;
    }
    private void Remove(Action<T> handler)
    {
        foreach (var del in _delegates)
            if (del.Target == handler)
            {
                _delegates.Remove(del);
                return;
            }
    }

    List<WeakReference<Action<T>>> _delegates = new List<WeakReference<Action<T>>>();

    internal void Invoke(T arg)
    {
        List<WeakReference<Action<T>>> toRemove = null;
        foreach (var del in _delegates)
        {
            if (del.IsAlive)
                del.Target(arg);
            else
            {
                if (toRemove == null)
                    toRemove = new List<WeakReference<Action<T>>>();
                toRemove.Add(del);
            }
        }
        if (toRemove != null)
            foreach (var del in toRemove)
                _delegates.Remove(del);
    }
}

Hopefully this will help someone else when they run into the mystery event caused memory leak in a garbage collected world!

Answer 1

I found the answer to my question as to why this doesn't work. Yes, indeed, I am missing a minor detail: The call to += to register the event (l.AnEvent += l_AnEvent;) creates an implicit Action object. This object is normally held only by the event itself (and the stack of the calling function). Thus, when the call returns and the garbage collector runs, the implicitly created Action object is freed (only a weak reference is point to it now), and the event is unregistered.

A (painful) solution is to hold a reference to the Action object as follows:

    class Bar
    {
        public Bar(Foo l)
        {
            _holdAnEvent = l_AnEvent;
            l.AnEvent += _holdAnEvent;
        }
        Action<int> _holdAnEvent;
        ...
    }

This works, but removes the simplicity of the solution.