Why and How to avoid Event Handler memory leaks?

Question

I just came to realize, by reading some questions and answers on StackOverflow, that adding event handlers using += in C# (or i guess, other .net languages) can cause common memory leaks...

I have used event handlers like this in the past many times, and never realized that they can cause, or have caused, memory leaks in my applications.

How does this work (meaning, why does this actually cause a memory leak) ?
How can I fix this problem ? Is using -= to the same event handler enough ?
Are there common design patterns or best practices for handling situations like this ?
Example : How am I supposed to handle an application that has many different threads, using many different event handlers to raise several events on the UI ?

Are there any good and simple ways to monitor this efficiently in an already built big application?

Solution 1

The cause is simple to explain: while an event handler is subscribed, the publisher of the event holds a reference to the subscriber via the event handler delegate (assuming the delegate is an instance method).

If the publisher lives longer than the subscriber, then it will keep the subscriber alive even when there are no other references to the subscriber.

If you unsubscribe from the event with an equal handler, then yes, that will remove the handler and the possible leak. However, in my experience this is rarely actually a problem - because typically I find that the publisher and subscriber have roughly equal lifetimes anyway.

It is a possible cause... but in my experience it's rather over-hyped. Your mileage may vary, of course... you just need to be careful.

Solution 2

I have explained this confusion in a blog at https://www.spicelogic.com/Blog/net-event-handler-memory-leak-16. I will try to summarize it here so that you can have a clear idea.

Reference means, "Need":

First of all, you need to understand that, if object A holds a reference to object B, then, it will mean, object A needs object B to function, right? So, the garbage collector won't collect object B as long as object A is alive in the memory.

+= Means, injecting reference of Right side object to the left object:

The confusion comes from the C# += operator. This operator does not clearly tell the developer that, the right-hand side of this operator is actually injecting a reference to the left-hand side object.

And by doing so, object A thinks, it needs object B, even though, from your perspective, object A should not care if object B lives or not. As object A thinks object B is needed, object A protects object B from the garbage collector as long as object A is alive. But, if you did not want that protection given to the event subscriber object, then, you can say, a memory leak occurred. To emphasize this statement, let me clarify that, in the .NET world, there is no concept of memory leak like a typical C++ unmanaged program. But, as I said, object A protects object B from garbage collection and if that was not your intention, then you can say a memory leak happened because object B was not supposed to be living in the memory.

You can avoid such a leak by detaching the event handler.

How to make a decision?

There are lots of events and event handlers in your whole code-base. Does it mean, you need to keep detaching event handlers everywhere? The answer is No. If you had to do so, your codebase will be really ugly with verbose.

You can rather follow a simple flow chart to determine if a detaching event handler is necessary or not.

Most of the time, you may find the event subscriber object is as important as the event publisher object and both are supposed to be living at the same time.

Example of a scenario where you do not need to worry

For example, a button click event of a window.

Here, the event publisher is the Button, and the event subscriber is the MainWindow. Applying that flow chart, ask a question, does the Main Window (event subscriber) supposed to be dead before the Button (event publisher)? Obviously No. Right? That won't even make sense. Then, why worry about detaching the click event handler?

An example when an event handler detachment is a MUST.

I will provide 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. The child window subscribes to that event of the main window.

And, the child window subscribes to an event of the Main Window.

From this code, we can clearly understand that there is a button in the Main Window. Clicking that button shows a Child Window. The child window listens to an event from the main window. After doing something, the user closes the child window.

Now, according to the flow chart I provided if you ask a question "Does the child window (event subscriber) supposed to be dead before the event publisher (main window)? The answer should be YES. Right? So, detach the event handler. I usually do that from the Unloaded event of the Window.

A rule of thumb: If your view (i.e. WPF, WinForm, UWP, Xamarin Form, etc.) subscribes to an event of a ViewModel, always remember to detach the event handler. Because a ViewModel usually lives longer than a view. So, if the ViewModel is not destroyed, any view that subscribed event of that ViewModel will stay in memory, which is not good.

Proof of the concept using a memory profiler.

It won't be much fun if we cannot validate the concept with a memory profiler. I have used JetBrain dotMemory profiler in this experiment.

First, I have run the MainWindow, which shows up like this:

Then, I took a memory snapshot. Then I clicked the button 3 times. Three child windows showed up. I have closed all of those child windows and clicked the Force GC button in the dotMemory profiler to ensure that the Garbage Collector is called. Then, I took another memory snapshot and compared it. Behold! our fear was true. The Child Window was not collected by the Garbage collector even after they were closed. Not only that but the leaked object count for the ChildWindow object is also shown as "3" (I clicked the button 3 times to show 3 child windows).

Ok, then, I detached the event handler as shown below.

Then, I have performed the same steps and checked the memory profiler. This time, wow! no more memory leak.

Solution 3

Yes, -= is enough, However, it could be quite hard to keep track of every event assigned, ever. (for detail, see Jon's post). Concerning design pattern, have a look at the weak event pattern.

Solution 4

An event is really a linked list of event handlers

When you do += new EventHandler on the event it doesn’t really matter if this particular function has been added as a listener before, it will get added once per +=.

When the event is raised it go through the linked list, item by item and call all the methods (event handlers) added to this list, this is why the event handlers are still called even when the pages are no longer running as long as they are alive (rooted), and they will be alive as long as they are hooked up. So they will get called until the eventhandler is unhooked with a -= new EventHandler.

See Here

and MSDN HERE

View more solutions

Comments

[Cody Gray]

Also see: blogs.msdn.com/b/tess/archive/2006/01/23/…

[gillyb]

...I've seen some people writing about this on answers to questions like "what's the most common memory leaks in .net".

[JSBձոգչ]

A way to get around this from the publisher's side is to set the event to null once you're sure that you won't fire it any more. This will implicitly remove all of the subscribers, and can be useful when certain events are only fired during certain stages of the object's lifetime.

[Davi Fiamenghi]

Dipose method would be a good moment for setting the event to null

[Jon Skeet]

@DaviFiamenghi: Well, if something is being disposed, that's at least a likely indication that it's going to be eligible for garbage collection soon, at which point it doesn't matter what subscribers there are.

[Davi Fiamenghi]

Yes, it makes sense, I said so based on a situation I had with a long living publisher with many periodic subscribers, in a certain moment I had to reset/clear the instance resources, but the references to subscribers were still there, so I set it to null on Dispose and called it to clear those "unmanaged" resources. Maybe a better way would be call the methods ClearSubscriptions and ClearData and call Dispose later when I no longer need the publisher, before GC? Thanks for reply

[Femaref]

msdn.microsoft.com/en-us/library/aa970850(v=vs.100).aspx the 4.0 version still has it.

[Shimmy Weitzhandler]

If I know a publisher is gonna live longer than the subscriber, I make the subscriber IDisposable and unsubscribe from event.

[mike]

For me "being careful" is the rule: the subscriber is responsible for unsubscribing. I often let the subscribers unsubscribe in their Dispose method. Setting the event to null (by the publisher) seems to be a brute-force approach and I don't consider it the responsibility of the publisher. In the imagined scenario I would have the publisher expose an additional event, PleaseStopListeningToMe, and it would be the responsibility of the subscribers to act accordingly.

[BrainSlugs83]

What if the event method is static? Then there's no instance to hold on to, right? -- Seems like a pretty easy workaround (and the typical event pattern includes a sender anyway...).

[Jon Skeet]

@BrainSlugs83: "and the typical event pattern includes a sender anyway" - yes, but that's the event producer. Typically the event subscriber instance is relevant, and the sender isn't. So yes, if you can subscribe using a static method, this isn't an issue - but that's rarely an option in my experience.

[BrainSlugs83]

@JonSkeet -- oh, I agree, it's not always an option (and yes, I understand that sender is the event producer), but thank you for validating my hypothesis there, that's useful. 🙂

[ahdung]

If use anonymous method (or lambda) as handler, will it still leak?

[ahdung]

Nice illustration.

[Jon Skeet]

@ahdung: It depends on exactly what you mean by "leak" and exactly what the code does. But you do need to be careful about how you unsubscribe an event handler written as an anonymous method or lambda expression.

[ahdung]

@JonSkeet leak is subscriber's lifetime short than publisher, especially when publisher is a static class. Do you mean if used any member of subscriber in handler, then leak will happen? it doesn't matter handler is a normal method or lambda?

[Jon Skeet]

@ahdung: If the publisher is a static class, there can't be an instance, so there's no concept of a lifetime for that publisher to be longer or shorter than that of the subscriber. This is why the details matter. I suggest you come up with a concrete example of what you mean and ask about it in a new question.

[ahdung]

@JonSkeet No instance doesn't mean no life cycle, on the contrary, it is very long, can't believe you say that. myform suscribed a StaticClass.Event, so I worry myform never be released, that's my worry.

[Jon Skeet]

@ahdung: Again, it would have been better to ask in a new question - partly because it depends on exactly what the anonymous function does. If it captures "this" (the form) then yes, that would effectively leak. (Whether that's a problem or not would depend on the app, of course. If there's only ever a single instance of the form anyway, it's probably a more theoretical issue.)

[ahdung]

@JonSkeet I got it, thank you for your patience.

[rollsch]

Weak Event handlers are fantastic EXCEPT when you have thousands of them. This is as they take up far more memory than a normal event handler. I only use them when there is a small number.

[rollsch]

I had no idea you could simply set the eventhandler to null. That MASSIVELY simplifies things.