views:

767

answers:

5

Say we have the following method:

private MyObject foo = new MyObject();

// and later in the class

public void PotentialMemoryLeaker(){
  int firedCount = 0;
  foo.AnEvent += (o,e) => { firedCount++;Console.Write(firedCount);};
  foo.MethodThatFiresAnEvent();
}

If the class with this method is instantiated and the PotentialMemoryLeaker is called multiple times, do we leak memory? Is there any way to unhook that lambda event handler after we're done calling MethodThatFiresAnEvent?

+10  A: 

Yes, save it to a variable and unhook it.

DelegateType evt = (o, e) => { firedCount++; Console.Write(firedCount); };
foo.AnEvent += evt;
foo.MethodThatFiresAnEvent();
foo.AnEvent -= evt;

And yes, if you don't, you'll leak memory, as you'll hook up a new delegate object each time. You'll also notice this because each time you call this method, it'll dump to the console an increasing number of lines (not just an increasing number, but for one call to MethodThatFiresAnEvent it'll dump any number of items, once for each hooked up anonymous method).

Lasse V. Karlsen
+1  A: 

Yes in the same way that normal event handlers can cause leaks. Because the lambda is actually changed to:

someobject.SomeEvent += () => ...;
someobject.SomeEvent += delegate () {
    ...
};

// unhook
var del = () => ...;
someobject.SomeEvent += del;
someobject.SomeEvent -= del;

So basically it is just short hand for what we have been using in 2.0 all these years.

Nick Berardi
+2  A: 

You wont just leak memory, you will also get your lambda called multiple times. Each call of 'PotentialMemoryLeaker' will add another copy of the lambda to the event list, and every copy will be called when 'AnEvent' is fired.

Wilka
+1  A: 

Your example just compiles to a compiler-named private inner class (with field firedCount and a compiler-named method). Each call to PotentialMemoryLeaker creates a new instance of the closure class to which where foo keeps a reference by way of a delegate to the single method.

If you don't reference the whole object that owns PotentialMemoryLeaker, then that will all be garbage collected. Otherwise, you can either set foo to null or empty foo's event handler list by writing this:

foreach (var handler in AnEvent.GetInvocationList()) AnEvent -= handler;

Of course, you'd need access to the MyObject class's private members.

Mark Cidade
+3  A: 

Well you can extend what has been done here to make delegates safer to use (no memory leaks)

Vaibhav