Mister Goodcat

Peter's home of all things life

Tuesday, 8/31/2010 2:13 PM
by Peter Kuhn
12 Comments

Fixing the memory leaks in BlogEngine.NET 1.6.1

Tuesday, 8/31/2010 2:13 PM by Peter Kuhn | 12 Comments

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<stringXmlDocument> _xmlDocumentByZone = new Dictionary<stringXmlDocument>();

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<stringXmlDocument> newDocs = new Dictionary<stringXmlDocument>();

    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.

Comments (12) -

Interesting post. I tried applying the WidgetZone changes (as that's the only one of the three items I use) and it fails to render the various Zones I have defined on my site. I use zones extensively to control everything from the header, left column and footers and after applying the above changes the zone all render the same content. I haven't had a chance to debug it but wanted to post a comment in case others look to try this.

Hallo Steve. Thanks for pointing this out. Since the default template doesn't use multiple zones, I missed that. I've updated the post with a solution that works for multiple zones too. Feel free to test and post any feedback :).

Thanks for letting me know! When I get a chance I'll give the new bits a spin...

The latest bits indeed correct the multi-zone issues. Thanks.

Spoke too soon. Clicking the Edit button of a zone doesn't edit the correct content. In my case it always edits the About zone content.

Sorry I can't contribute more than comments at this point. My schedule this month is simply crazy otherwise I'd pitch in.

Hi Steve. I've tested this with a clean installation of BE, two widget zones and my fixes applied, and I was able to edit each content correctly. If you're able to send me a repro (maybe some other customization interfering?), I'm willing to take a deeper look.

I think the Blogroll and CodeFormatter changes are good.

The WidgetZone changes cannot be made like that.  If you are running multiple widget zones, changing those members to static ends up so every widget zone has the same widgets in it.

Stepping thru the code, it doesn't look like the XML_DOCUMENT is cached at all (RetrieveXml() gets called during each page request).  Caching the XML_DOCUMENT *for each* widget zone would be an enhancement, but as it is now, it looks like the WidgetZone constructor that wires up WidgetEditBase.Saved can actually be removed (or commented out) since all it's doing is clearing out the XML_DOCUMENT which is being re-retrieved from the data store on each page request.

Glad you posted these findings.

Hallo Ben. Yes, you're right. After Steve had commented on that issue, I've looked into a different solution for multiple zones and updated the post. The solution seems to work well and also documents are cached correctly. But since I'm very new to BlogEngine.NET, I cannot tell if there are other implications.

I think you can submit this issue with patched code to the issue tracking list found there http://blogengine.codeplex.com/workitem/list/basic , this will help the community :).

Also I want to ask bout the performance test you did , using what tool ..?

According to Ben Amada, these fixes already are included in the trunk.

For analyses like that I use WinDbg and the built-in profiler of Visual Studio. Please note that you need Visual Studio Premium or better for that, it's not included in the Professional or Express versions.

Hi,

Thanks a lot for this fix. because of this memory leak my site was bringing down other sites also in a shared hosting environment. I even opted for a dedicated app pool but the memory leak brought down that too.. it was using around 3Gb of RAM !  After ur fix, memory usage went down drastically now the usage is around 200MB.
You have indeed done a great job!! Keep it up!!

Nishanth

Thanks!! I was searching for the 1.6.1 source code! Implementing your changes didn't harm my blog site so thanks again!

Pingbacks and trackbacks (1)+

Comments are closed