CODEWITHSUNDEEP
c#.net
heijp06
heijp06

Reputation: 11798

Should I dispose XmlNodeList

In the following code I get an XmlNodeList as return value from XmlDocument.SelectNodes()

foreach (XmlNode node in doc.SelectNodes(xPath))
{
    // Do stuff
}

As it turns out, XmlNodeList implements IDisposable. Does that mean that each time I want to iterate the return value of XmlDocument.SelectNodes() I should put that in a local variable and make sure it gets disposed (i.e. put it in a using block)?

Like this:

using(XmlNodeList nodes = doc.SelectNodes(xPath))
{ 
    foreach (XmlNode node in nodes)
    {
        // Do stuff
    }
}

Upvotes: 6

Views: 1428

Answers (1)

Hans Passant
Hans Passant

Reputation: 941635

The System.Xml namespace is, erm, wonky. Nicest way I could possibly put it. XmlNodeList is an abstract class, it inherits IDisposable and implements the disposable pattern but otherwise doesn't do anything itself.

There are three internal classes that derive from XmlNodeList. One of them actually overrides the Dispose(bool) method, XmlElementList. That class has a private field of type XmlElementListener. Yet another internal class, not that obvious what it does but it appears to "listen" to list changes. The Dispose method (wait for it) unsubscribes two event handlers.

This breaks every rule in the book, it is never correct to abuse IDisposable for that. Unfortunately you'll have to walk the walk, it is next to impossible to unravel this to see whether that listener ever gets instantiated and whether those event handlers will cause a long lasting leak in your program if you don't call Dispose(). You'd better call it.

Lots of awesome code in the .NET Framework. Good code always needs bad code to make it obvious how good the good code is. That's System.Xml's job.

Upvotes: 10

Related Questions