CODEWITHSUNDEEP
c#.net
noctonura
noctonura

Reputation: 13121

Anonymous method for event handler not a leak?

Whenever you add a delegate to an event handler, you should remove it later, right? So if you attach an anonymous method to an event, does this create an event handler leak since you can't later remove it? This code sample from http://msdn.microsoft.com/en-us/library/0yw3tz5k%28VS.80%29.aspx seems to imply that this an okay practice though.

// Create a handler for a click event
button1.Click += delegate(System.Object o, System.EventArgs e)
                   { System.Windows.Forms.MessageBox.Show("Click!"); };

Is this really an okay practice?

Upvotes: 5

Views: 5291

Answers (4)

KeithS
KeithS

Reputation: 71563

The use of inline anonymous delegates as event handlers does prevent those handlers being removed, as you stated. This can indeed cause problems, especially if the delegate comes from a different object. If you had a class A with a public event E, and E was subscribed to by a class B (that has no containing/contains relationship to A) attaching an anonymous handler H, H is technically a member of B, and so as long as it's referred to (by being attached to A's event), B won't be GCed. B's lifetime becomes tied to A's, and if you didn't want or expect that you now have a memory leak.

Now, that's not always a problem. If B contains A, or the other way around, then A and B's lifetimes are already tied together anyway; in your case, the button itself will very probably be disposed of before its containing control that attached the handler. Similarly, A and B could be long-lived objects (say, the program's main form and a data repository) and won't be GCed until the program terminates anyway. As long as you never have to care about a difference in lifetime between A and B, and you never need to detach H from E to stop exhibiting whatever behavior H has, then you can attach all the anonymous handlers you like.

Upvotes: 3

Servy
Servy

Reputation: 203820

Whenever you add a delegate to an event handler, you should remove it later, right?

Well, not always. There are two reasons why you'd want to remove an event handler that you're adding:

  1. You're constantly adding handlers to the same instance that are short lived. If you didn't remove them then there would be more and more handlers added, when most of them aren't needed.
  2. Your handler internally holds onto a reference to an object who's lifetime is much shorter than the lifetime of whatever object the event belongs to, and the event handler won't be (or can't be) called once that other object goes out of scope. Leaving the event handler attached will either force it to stay in memory longer than desired, or possibly result in using an object that's "stale" and shouldn't be used anymore. (For example, if the resource has already been disposed you don't want that event to fire anymore.)

The reason #2 is an issue is because of how Garbage Collection works in C#. It marks all objects that it can be 100% sure are in scope as "alive", and then follows everything that any of those "alive" objects reference as also being "alive" until it's followed every reference in every live object. Anything that was never marked as "alive" is then deemded "dead" and eligible for garbage collection.

When you have an event handler attached to an event that delegate contains two things, an instance of an object and a method to run on that object. That referenced object won't be able to be garbage collected until either:

  1. The object with the event is no longer "alive".
  2. You remove the event handler (i.e. the reference to) your delegate, allowing your object to be freed earlier.

That said, a significant percentage of cases don't apply to either of those, so there's no need to bother removing the event handlers.

As an example, I often see people removing event handlers just before the event object goes out of scope. That's pointless. If the object is out of scope there's no problem with it holding onto references to...whatever.

Now, if you are in one of those few situations in which you do need to unsubscribe the event handler, and you're using an anonymous method you need to...not. Just create a class that can make it a named method and use that.

Upvotes: 6

Sergey Teplyakov
Sergey Teplyakov

Reputation: 11647

Subscibing to event with anonymous method (or lambda expression) potentially can lead to memory leaks, but not in this case.

In this case compiler will generate an anonymous method inside current class and you don't have any memory related issues as long as your button will not live much longer than your object.

Upvotes: 3

Jon Skeet
Jon Skeet

Reputation: 1500505

Whenever you add a delegate to an event handler, you should remove it later, right?

Not necessarily, no. Often you want the event handler to stay valid for as long as the event itself can be raised - that's certainly very common with UIs.

Is this really an okay practice?

Absolutely, so long as you don't need to unhook the handler. Think about the point at which you'd unhook the event handler. If it's "when the form (or button, or whatever) is elegible for garbage collection" then what benefit is there in removing the handler? Just let it be garbage collected with the form...

Upvotes: 11

Related Questions