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

Posted by Eric on Stack Overflow See other posts from Stack Overflow or by Eric
Published on 2010-05-11T22:33:23Z Indexed on 2010/05/11 22:34 UTC
Read the original article Hit count: 399

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 that I borrowed from here: http://damieng.com/blog/2006/08/01/implementingweakreferencet I had to add Equals() and GetHashCode() to his class, which I include below for reference.

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

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

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

public class WeakReference<T> : IDisposable
{
    private GCHandle handle;
    private bool trackResurrection;

    public WeakReference(T target)
        : this(target, false)
    {
    }

    public WeakReference(T target, bool trackResurrection)
    {
        this.trackResurrection = trackResurrection;
        this.Target = target;
    }

    ~WeakReference()
    {
        Dispose();
    }

    public void Dispose()
    {
        handle.Free();
        GC.SuppressFinalize(this);
    }

    public virtual bool IsAlive
    {
        get { return (handle.Target != null); }
    }

    public virtual bool TrackResurrection
    {
        get { return this.trackResurrection; }
    }

    public virtual T Target
    {
        get
        {
            object o = handle.Target;
            if ((o == null) || (!(o is T)))
                return default(T);
            else
                return (T)o;
        }
        set
        {
            handle = GCHandle.Alloc(value,
              this.trackResurrection ? GCHandleType.WeakTrackResurrection : GCHandleType.Weak);
        }
    }

    public override bool Equals(object obj)
    {
        var other = obj as WeakReference<T>;
        return other != null && Target.Equals(other.Target);
    }

    public override int GetHashCode()
    {
        return Target.GetHashCode();
    }
}

It's functionality is trivial. I override operator + and - to get the += and -= syntactic sugar matching events. These create 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, and replace the Action with Action. 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);
        }
    }

Hopefully this will help someone else when they run into the mystery .NET event memory leak!

© Stack Overflow or respective owner

Related posts about events

Related posts about memory-leaks