Investigating a Silverlight crash in Firefox The joys of setting up a blog

Fixing the memory leaks in BlogEngine.NET 1.6.1

Published on Tuesday, August 31, 2010 11:13:00 AM UTC in Tools

As I wrote in my previous post, BlogEngine.NET has some heavy memory leaks in version 1.6.1 that caused my test installation to allocate several hundred additional kilobytes on each post view. As a result, either your server will crawl or even come to a halt after a while, or if you had set an application pool recycle threshold, you'll see recycling happening constantly.

I decided to do a bit of profiling on the project, and after that didn't bring up good clues, I bit the bullet and searched through the source to look out for the usual suspects. For that, I had downloaded the source code for 1.6.1 which hasn't been released as a separate package, but can easily be found here (I didn't want to use the trunk). In the end it turned out that I didn't even need the sources, because all the issues are part of the code in the App_Code folder and can be fixed directly there. Ok, here are my findings:

Blogroll

In the file "App_Code\Controls\Blogroll.cs" you will find that the constructor of the BlogRoll control is not static, which means that the BlogRollItem.Saved event is hooked up every time the control is created (and never released). To change that, make the constructor static, as well as the BlogRollItem_Saved event handler:

static Blogroll()
{
    BlogRollItem.Saved += new EventHandler<SavedEventArgs>(BlogRollItem_Saved);
}

private static void BlogRollItem_Saved(object sender, SavedEventArgs e)
{
    ...

CodeFormatter

The same problem happens in code formatter extension, in the "App_Code\Extensions\CodeFormatter\CodeFormatter.cs" file. Again the solution is to make the constructor static, but in addition you also have to change the following elements to static:

  • The "Regex codeRegex" field
  • The "ServingContent" event handler method
  • The "CodeEvaluator" method
  • The "Highlight" method

WidgetZone

Edited: As noted by Steve Trefethen in the comments, my initial fix caused problems when you had multiple zones. I've changed the fix to accommodate that. Thanks for pointing this out, Steve!

This placeholder also has a similar problem. It hooks up a "Saved" event to react to changes of the widgets layout, and again this happens in a non-static constructor. The change for this is a bit more complicated, since each zone has its own Xml document. The idea is the following: I create a static dictionary that maps zone names to Xml documents. When a zone is initialized and its document has not been loaded, it retrieves the document and adds it to the dictionary. When the save event occurs, the dictionary is re-created. To this end, the following changes have to be made:

Make the constructor static and change the event handler for the saved event, add a field for the dictionary and change the previous field XML_DOCUMENT to be a property that fetches the document from the dictionary.

static WidgetZone()
{            
    WidgetEditBase.Saved += delegate { ReloadAllXml(); };
}

private static Dictionary<string, XmlDocument> _xmlDocumentByZone = new Dictionary<string, XmlDocument>();

private const string DefaultZoneName = "be_WIDGET_ZONE";

// For backwards compatibility or if a ZoneName is omitted, provide a default ZoneName.
private string _ZoneName = DefaultZoneName;

private XmlDocument XML_DOCUMENT
{
    get
    {
        // look up the document by zone name
        if (_xmlDocumentByZone.ContainsKey(ZoneName))
        {
            return _xmlDocumentByZone[ZoneName];
        }

        return null;
    }
}

The "RetrieveXml" method now is static and takes a zone name as parameter. The "ReloadAllXml" method is new.

private static void ReloadAllXml()
{
    // simply reload all xml documents when something has changed
    Dictionary<string, XmlDocument> newDocs = new Dictionary<string, XmlDocument>();

    foreach (string zoneName in _xmlDocumentByZone.Keys)
    {
        XmlDocument doc = RetrieveXml(zoneName);
        newDocs.Add(zoneName, doc);
    }

    _xmlDocumentByZone = newDocs;
}

private static XmlDocument RetrieveXml(string zoneName)
{            
    WidgetSettings ws = new WidgetSettings(zoneName);
    ws.SettingsBehavior = new XMLDocumentBehavior();
    XmlDocument doc = (XmlDocument)ws.GetSettings();
    return doc;
}

During initialization, the document for a zone is added to the caching dictionary if it hasn't been loaded yet:

protected override void OnInit(EventArgs e)
{
    if (XML_DOCUMENT == null)
    {
        // if there's no document for this zone name yet, load it
        XmlDocument doc = RetrieveXml(ZoneName);

        if (_xmlDocumentByZone.ContainsKey(ZoneName))
        {
            _xmlDocumentByZone[ZoneName] = doc;
        }
        else
        {
            _xmlDocumentByZone.Add(ZoneName, doc);
        }
    }

    base.OnInit(e);
}

For simplicity, I've uploaded the complete file here so you don't have to do all the manual steps yourself:

WidgetZone.zip (2.03 kb)

Result

After those changes I did some testing and found that the memory leak is gone (or at least, if there are other leaks left, negligible). You will still encounter that first the memory climbs when you browse your blog, but this is due to other reasons, for example the built-in caching. For testing, you can put some forced garbage collection statements in the code, and you'll see that memory stays at a constant level.

I found another suspicious place in the open search http handler, but I'm not even sure if that's a real issue.

Tags: BlogEngine.NET · Bug Fix · Memory Leak