.NET Event Handler Memory Leaks: Should You Always Disconnect Event Handlers?

Should you always disconnect event handlers in the Dispose method? I see this question asked again and again. Many teams even make it a coding standard: every += must have a matching -=, no matter what.

Well, perhaps this standard was proposed as a defensive practice against memory leaks. I cannot say it is a bad standard, but I personally prefer to disconnect event handlers ONLY where needed. That way, my code looks clean and less verbose. In this article, I will show you how an event handler really causes a memory leak, prove it with a memory profiler, and give you a simple flow chart so you can decide exactly when you need to detach an event handler and when you do not.

The C# += operator is actually a reference injector

In C#, the += operator looks very innocent, and many developers do not know that the right-hand side object is actually passing its reference to the left-hand side object.

Say you write a.SomethingHappened += b.SomeHandler;. From this moment, the object a holds a reference to the object b, because the delegate that a stores carries the reference of b inside it. You did not just wire up an event. You injected a reference.

The event publisher protects the event subscriber

So, if an object gets a reference to another object, what is the problem? The problem is that when the garbage collector comes to clean up and finds an object that is important to keep in memory, it will not clean up the objects that are also referenced by that important object.

Let me make it simple. Say you have an object named Customer. This Customer object has a reference to a CustomerRepository object, so it can search the repository for all of its Address objects. If the garbage collector finds that the Customer object needs to stay alive, it will keep the CustomerRepository alive too, because the Customer object holds a reference to it. That makes sense, because the Customer object needs the CustomerRepository object to function.

But does an event publisher object need an event subscriber to function? NO, right? The event publisher is independent of the event subscriber. It should not care whether a subscriber is alive or dead. But when you subscribe with the += operator, the publisher receives a reference to the subscriber, and now the garbage collector thinks the publisher needs the subscriber object. So it does not collect the subscriber.

In that way, the event publisher object "a" PROTECTS the event subscriber object "b" from being garbage collected, as long as the publisher itself stays alive.

A flow chart to make the right decision

So, if you detach the event handler, the event publisher does not hold the reference of the event subscriber anymore, and the garbage collector can freely collect the event subscriber.

But do you really need to detach the event handler all the time? The answer is no, because many event subscribers are really supposed to be living in memory as long as the event publisher lives. Whenever you are in doubt, ask yourself just one question: is the subscriber supposed to die before the publisher? This flow chart is all you need.

An example where you do not need to worry

Take the classic button click event of a WPF window:

<Window x:Class="WpfApp1.MainWindow">
    <Grid>
        <Button Content="Say Hello" Click="btnSayHello_Click" />
    </Grid>
</Window>

Here, the event publisher is the Button, and the event subscriber is the MainWindow. Apply the flow chart: is the MainWindow (event subscriber) supposed to be dead before the Button (event publisher)? Obviously no, right? That would not make any sense. Then why worry about detaching the click event handler? Most of the time, we find that the event subscriber object is as important as the event publisher object, and both are supposed to be living at the same time. In those cases, detaching buys you nothing. It just makes your code verbose.

An example where detaching is a MUST

Now, I will give you one example where the subscriber object is supposed to be dead before the publisher object. Say your MainWindow publishes an event named SomethingHappened, and you show a child window from the main window by a button click:

public partial class MainWindow
{
    public event EventHandler<EventArgs> SomethingHappened;

    private void btnSayHello_Click(object sender, RoutedEventArgs e)
    {
        ChildWindow childWindow = new ChildWindow(this);
        childWindow.Show();
    }
}

And the child window subscribes to that event of the main window:

public partial class ChildWindow
{
    private readonly MainWindow _mainWindow;

    public ChildWindow(MainWindow mainWindow)
    {
        InitializeComponent();
        _mainWindow = mainWindow;
        _mainWindow.SomethingHappened += MainWindow_SomethingHappened;
    }

    private void MainWindow_SomethingHappened(object sender, System.EventArgs e)
    {
    }
}

When the user clicks the button, the child window shows up, and when the user finishes the task, the child window is closed. Now, according to the flow chart: is the child window (event subscriber) supposed to be dead before the main window (event publisher)? The answer is yes, right? So, this is a place where you MUST detach the event handler. If you do not, every closed child window will stay in memory as long as the main window is alive. Do not just take my word for it. Let me prove it with a memory profiler.

Validating the memory leak with a profiler

I profiled this exact code using the dotMemory memory profiler from JetBrains. I started the MainWindow and clicked the button 3 times, so 3 instances of the ChildWindow showed up. Then, I closed all the child windows. Comparing a snapshot before and after, I found that 3 objects of the ChildWindow were still living in memory, even though I closed all of them.

Then, I detached the event handler in the Unloaded event of the child window, like this:

private void Window_Unloaded(object sender, RoutedEventArgs e)
{
    _mainWindow.SomethingHappened -= MainWindow_SomethingHappened;
}

I profiled again, and this time, wow! No memory leak caused by that event handler.

The bottom line

Do not make a religious rule out of detaching event handlers. Make the decision consciously, with one question: is the event subscriber supposed to die before the event publisher?

• If the answer is no, like a MainWindow subscribing to its own Button, leave it alone. Detaching adds noise, not safety.
• If the answer is yes, like a short-lived child window subscribing to a long-lived main window, you MUST detach the event handler. A natural place is the Unloaded event, or the Dispose method if your subscriber is disposable.

By the way, this article started as my answer to a Stack Overflow question: Should I always disconnect event handlers in the Dispose method? If you want to see how other developers think about this, that discussion is a good read.