How do events cause memory leaks in C# and how do Weak References help mitigate that?

Question

There are two ways (that I know of) to cause an unintentional memory leak in C#:

1. Not disposing of resources that implement IDisposable
2. Referencing and de-referencing events incorrectly.

I don't really understand the second point. If the source object has a longer lifetime than the listener, and the listener doesn't need the events anymore when there are no other references to it, using normal .NET events causes a memory leak: the source object holds listener objects in memory that should be garbage collected.

Can you explain how events can cause memory leaks with code in C#, and how I can code to get around it using Weak References and without Weak References?

Answer 1

When a listener attaches an event listener to an event, the source object will get a reference to the listener object. This means that the listener cannot be collected by the garbage collector until either the event handler is detached, or the source object is collected.

Consider the following classes:

class Source
{
    public event EventHandler SomeEvent;
}

class Listener
{
    public Listener(Source source)
    {
        // attach an event listner; this adds a reference to the
        // source_SomeEvent method in this instance to the invocation list
        // of SomeEvent in source
        source.SomeEvent += new EventHandler(source_SomeEvent);
    }

    void source_SomeEvent(object sender, EventArgs e)
    {
        // whatever
    }
}

...and then the following code:

Source newSource = new Source();
Listener listener = new Listener(newSource);
listener = null;

Even though we assign null to listener, it will not be eligible for garbage collection, since newSource is still holding a reference to the event handler (Listener.source_SomeEvent). To fix this kind of leak, it is important to always detach event listeners when they are no longer needed.

The above sample is written to focus on the problem with the leak. In order to fix that code, the easiest will perhaps be to let Listener hold on to a reference to Source, so that it can later detach the event listener:

class Listener
{
    private Source _source;
    public Listener(Source source)
    {
        _source = source;
        // attach an event listner; this adds a reference to the
        // source_SomeEvent method in this instance to the invocation list
        // of SomeEvent in source
        _source.SomeEvent += source_SomeEvent;
    }

    void source_SomeEvent(object sender, EventArgs e)
    {
        // whatever
    }

    public void Close()
    {
        if (_source != null)
        {
            // detach event handler
            _source.SomeEvent -= source_SomeEvent;
            _source = null;
        }
    }
}

Then the calling code can signal that it is done using the object, which will remove the reference that Source has to ´Listener`;

Source newSource = new Source();
Listener listener = new Listener(newSource);
// use listener
listener.Close();
listener = null;

Answer 2

There are, strictly speaking, no "memory leaks" within the "sandbox" of a managed .NET project; there are only references held longer than the developer would think necessary. Fredrik has the right of it; when you attach a handler to an event, because the handler is usually an instance method (requiring the instance), the instance of the class containing the listener stays in memory as long as this reference is maintained. If the listener instance contains references to other classes in turn (e.g. backreferences to containing objects), the heap can stay quite large long after the listener has gone out of all other scopes.

Maybe someone with a bit more esoteric knowledge of Delegate and MulticastDelegate can shed some light on this. The way I see it, a true leak COULD be possible if all of the following were true:

• The event listener requires external/unmanaged resources to be released by implementing IDisposable, but it either does not, or
• The event multicast delegate does NOT call the Dispose() methods from its overridden Finalize() method, and
• The class containing the event does not call Dispose() on each Target of the delegate through its own IDisposable implementation, or in Finalize().

I've never heard of any best practice involving calling Dispose() on delegate Targets, much less event listeners, so I can only assume the .NET developers knew what they were doing in this case. If this is true, and the MulticastDelegate behind an event tries to properly dispose of listeners, then all that is necessary is proper implementation of IDisposable on a listening class that requires disposal.

Answer 3

Read Jon Skeet's excellent article on events. It's not a true "memory leak" in the classic sense, but more of a held reference that hasn't been disconnected. So always remember to -= an event handler that you += at a previous point and you should be golden.