C# still hooked to an event after unhooking

Question

I am currently debugging a big (very big!) C# application that contains memory leaks. It mainly uses Winforms for the GUI, though a couple of controls are made in WPF and hosted with an ElementHost. Until now, I have found that many of the memory leaks were caused by events not being unhooked (by calling -=) and I've been able to solve the problem.

However, I just came across a similar problem. There is a class called WorkItem (short lived) which in the constructor registers to events of another class called ClientEntityCache (long lived). The events were never unhooked and I could see in .NET profiler that instances of WorkItem were being kept alive when they shouldn't because of those events. So I decided to make WorkItem implement IDisposable and in the Dispose() function I unhook the events this way:

public void Dispose()
{
  ClientEntityCache.EntityCacheCleared -= ClientEntityCache_CacheCleared;
  // Same thing for 10 other events
}

EDIT

Here is the code I use for subscription:

public WorkItem()
{
  ClientEntityCache.EntityCacheCleared += ClientEntityCache_CacheCleared;
  // Same thing for 10 other events
}

I also changed the code for unregistering to not call new EntityCacheClearedEventHandler.

END OF EDIT

I made the calls to Dispose at the proper places in the code that uses WorkItem and when I debug I can see that the function is really being called and I do -= for every event. But I still get a memory leak and my WorkItems still stay alive after being Disposed and in .NET profiler I can see that the instances are kept alive because the event handlers (like EntityCacheClearedEventHandler) still have them in their invocation list. I tried to unhook them more than once (multiple -=) just to make sure they were not hooked more than once but this doesn't help.

Anyone has an idea why this is happening or what I could do to solve the problem? I suppose I could change the event handlers to use weak delegates but this would require to mess a lot with a big pile of legacy code.

Thanks!

EDIT:

If this helps, here is the root path described by .NET profiler: lots of things point on ClientEntityCache, which points to EntityCacheClearedEventHandler, which points to Object[], which points to another instance of EntityCacheClearedEventHandler (I don't understand why), which points to WorkItem.

Answer 1

It might be that multiple different delegate functions are wired to the event. Hopefully the following little example will make it clearer as to what I mean.

// Simple class to host the Event
class Test
{
  public event EventHandler MyEvent;
}

// Two different methods which will be wired to the Event
static void MyEventHandler1(object sender, EventArgs e)
{
  throw new NotImplementedException();
}

static void MyEventHandler2(object sender, EventArgs e)
{
  throw new NotImplementedException();
}


[STAThread]
static void Main(string[] args)
{
  Test t = new Test();
  t.MyEvent += new EventHandler(MyEventHandler1);
  t.MyEvent += new EventHandler(MyEventHandler2); 

  // Break here before removing the event handler and inspect t.MyEvent

  t.MyEvent -= new EventHandler(MyEventHandler1);      
  t.MyEvent -= new EventHandler(MyEventHandler1);  // Note this is again MyEventHandler1    
}

If you break before the removal of the event handler you can view the invocation list in the debugger. See below, there are 2 handlers, one for MyEventHandler1 and another for the method MyEventHandler2.

Now after removing the MyEventHandler1 twice, MyEventHandler2 is still registered, because there is only one delegate left it looks a little different, it is no longer showing in the list, but until the delegate for MyEventHandler2 is removed it will still be referenced by the event.

Answer 2

When unhooking an event, it needs to be the same delegate. Like this:

public class Foo
{
     private MyDelegate Foo = ClientEntityCache_CacheCleared;
     public void WorkItem()
     {
         ClientEntityCache.EntityCacheCleared += Foo;
     }

     public void Dispose()
     {
         ClientEntityCache.EntityCacheCleared -= Foo;
     }
}

The reason is, what you are using is syntactic sugar for this:

public class Foo
{
     public void WorkItem()
     {
         ClientEntityCache.EntityCacheCleared +=
new MyDelegate(ClientEntityCache_CacheCleared);
     }

     public void Dispose()
     {
         ClientEntityCache.EntityCacheCleared -=
new MyDelegate(ClientEntityCache_CacheCleared);
     }
}

So the -= doesn't unhook the original one you subscribed with because they are different delegates.