Thursday 11 December 2008

Winforms event memory leaks; Form Zombies that just won’t die!

I have a fair few events in the business layer which are listened to by my GUI which is a winforms app.  I have been tripped up on a nasty memory leak problem which is caused by GUI objects listening to events in the business layer. 

Unless developers are really disciplined and remember to unhook event handlers (with the –= operator during the closing event or wherever) you will get the following scenario:

You close a form because the user is finished with it.  The rest of the GUI can no longer reference your form or control.  You would now expect and assume that the form would be cleared up by the garbage collector.   But!! As the form is listening to a longer lived business layer object, keeping the business object in scope will, in turn, keep your form in scope.  You have ended up with GUI objects being held in memory by references from the business layer!

This doesn’t appear to make sense as how can an observable keep an observer hanging around when the observer wants to leave! (Daddy!! Daddy!! Look what I can do Daddy!!).  The simplest way of explaining what is happening is when an event is wired up, the observed needs to know how to get hold of the observer (to tell him about the event).  In order to do this, the observed needs a reference to the observer.  From here its not hard to see that the observed now can keep the observer in scope via the reference.

In a large winforms app and a long user session, you can end up with loads of forms hanging around eavesdropping on all sorts of conversations that are nothing to do with them.  This is first of all a nuisance as winforms is quite memory hungry anyway.  More importantly it can lead to bizarre effects such as message boxes seeming to appear twice for one logical business ‘event’.

So how to go about solving this.  Well, if you only have a couple of forms and a couple of business objects or events, you can just read the code to ensure that events are being unsubscribed from when they should be.  If you only have a couple of events, chances are you aren’t reading this post as you haven’t uncovered this problem yet!

“But but! I’ve got simply millions of events!  My app is an event driven masterpiece of enterprisey stuff!”.  Well, in this scenario, I don’t like typing more than the next man so I decided to use reflection to track down the zombie forms.  I though I would piggy back my normal GUI unit binding tests, and check that unsubscribing was happening when it should be.  It should be a simple matter of binding my GUI objects. Then telling them to unbind, or close them or whatever is relevant for the subject under test.  Then I could use some sexy reflection to confirm that indeed, the events in the business layer no longer refer to the form.

It turns out this is not as easy as you might assume!  I set my sights on getting a reference to the MulticastDelegate underlying the event in the business layer.  Once I have this reference, I can enumerate the invocation list checking for reference to my form.

First of all I tried .GetType().GetEvents();  This returns an EventInfo object and I won’t detain you further with how useful I found this class to be.

So after a bit of Visual Studio debug reflection exploration, and getting no where fast, I turned to google where I was assured that my MulticastDelegate was a field.  I was sure I had looked in the GetFields() collection which was an empty array.  I then had an RTFM moment, and realised there was an overload to GetFields.  A bit of boolean logic later and I had a reference to a private field in the class that was nothing to do with my event.  Damn you internet!  Will you ever live up to your potential?

I then then set up a little test rig which convinced me I was going down the right path.  I was easily able to get a reference to the MulticastDelegate using exactly the same code that appeared to do nothing for my real code base.  I systematically introduced all the things that made my real code scenario different.  It happened to be the first thing I tried, inheritance.  Although the miss-leading GetEvents() returns the events from a base class, GetFields() doesn’t get inherited fields.  In fact, all you need to do is get to the base class’s type and you are away.  Here’s the code to get to a base class’s MulticastDelegate:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Reflection;
using NUnit.Framework;

namespace Yarp
{

    public class BaseClassWithEvent
    {
        public event EventHandler OhWhereCanIBe;


    }

    public class SubClass : BaseClassWithEvent
    {}

    [TestFixture]
    public class SubClassTester
    {
        private SubClass c1;

        public SubClassTester()
        {
            c1 = new SubClass();
            //Create the invocationlist
            this.c1.OhWhereCanIBe += new EventHandler(c1_OhWhereCanIBe);
        }

        private void c1_OhWhereCanIBe(object sender, EventArgs e)
        {
            throw new NotImplementedException();
        }

        [Test]
        public void TestFindingTheInvocationListOfAnEventInABaseClass()
        {
            //Get the subclassType
            var c1Type = c1.GetType();

            //Prove we can't access it via the type reference
            var subclassFields = c1Type.GetFields(BindingFlags.NonPublic | BindingFlags.Instance);
            Assert.IsEmpty(subclassFields);
            
            //It is accessible through the base type
            var baseFields =
                c1Type.BaseType.GetFields(BindingFlags.NonPublic | BindingFlags.Instance);
            Assert.IsNotEmpty(baseFields);

            MulticastDelegate multicastDelegate = baseFields[0].GetValue(c1) as MulticastDelegate;

            foreach(var listener in multicastDelegate.GetInvocationList())
            {
               //Do what you got to do to ensure the listener is not a zombie
            }

        }

    }

}

No comments: