Memory leak via event handlers

One of our customers recently had some interesting out-of-memory issues which I thought I’d share.

The short version

The basic problem was that they had event sinks and sources. The event sinks were disposed correctly. However, in the dispose they missed removing the event sink from the event invoker list of the source. So in effect the disposed sinks were still reachable from the event source which is still in use. So GC did not de-allocate the event sinks and lead to leaked objects.

Now the not-so-long version

Consider the following code where there are EventSource and EventSink objects. EventSource exposes the event SomeEvent which the sink listens to.

EventSink sink = new EventSink();
EventSource src = new EventSource();

src.SomeEvent += sink.EventHandler;
src.DoWork();

sink = null;
// Force collection
GC.Collect();
GC.WaitForPendingFinalizers();

For the above code if you have a finalizer for EventSink and add a breakpoint/Console.WriteLine in it, you’d see that it is never hit even though sink is clearly set to null. The reason is the 3rd line where we added sink to the invoke list of source via the += operator. So even after sink being set to null the original object is still reachable (and hence not garbage) from src.

+= is additive so as the code executes and new sinks are added the older objects still stick around resulting in working set to grow and finally lead to crash at some point.

The fix is to ensure you remove the sink with –= as in

EventSink sink = new EventSink();
EventSource src = new EventSource();
src.SomeEvent += sink.EventHandler;
src.DoWork();
src.SomeEvent -= sink.EventHandler;
sink = null;

The above code is just sample and obviously you shouldn’t do event add removal in a scattered way. Make EventSink implement IDisposable and encapsulate the += –= in ctor/dispose.