Do I need to Dispose XmlReader if I Dispose its underlying Stream?

Question

I have the following method GetData that creates a StreamReader from a file.

private void GetData(string file)
{
    string filename = Path.GetFileNameWithoutExtension(file);
    XmlDocument xmldoc = new XmlDocument();

    using (StreamReader sr = new StreamReader(file))
    {
        Stream bs = sr.BaseStream;
        Stream cl = mainParser.CleanMarkup(bs);
        try
        {
            xmldoc = mainParser.LoadDocument(bs);
        }
        catch (XmlException ex)
        {
            // Exceptions are usually caused by non-compliant documents.
            // These errors are not critical to the operation of this program.
            Console.WriteLine(filename + " " + ex.Message);
        }
    }
    Msdn msdnParser = new Msdn(xmldoc);

    ListViewItem lvitem = new ListViewItem(filename);
    lvitem.SubItems.Add(filename);
    foreach (string item in msdnParser.Subitems)
    {
        lvitem.SubItems.Add(item);
    }
    listView.Items.Add(lvitem);
}

mainParser.LoadDocument(bs) calls the following:

public XmlDocument LoadDocument(Stream file)
{
    XmlDocument xmldoc = new XmlDocument();
    XmlReader xmlread = XmlReader.Create(file);
    xmldoc.Load(xmlread);

    return xmldoc;
}

StreamReader is disposed by GetData. Does this mean that I don't have to dispose of XmlReader since (I believe) this would dispose of its only unmanaged resource?

Rob · Accepted Answer · 2019-04-12T06:50:41.757

8

The best "rule of thumb" to work by is:

If something implements IDisposable, always wrap it in a using() block to ensure that any unmanaged resources it owns are disposed of correctly.

Relying on the fact that the current implementation of "something" disposes of an underlying resource is dangerous and it won't hurt to wrap everything in a using, just to be on the safe side =)

edited Apr 12 '19 at 06:50

answered Aug 18 '10 at 23:24

Rob

45,296
24
122
150

2

Ahh, this is a good point. This is probably a concept that I need to keep aware of. Always nice when an example shows up in my face. – Leonard Thieu Aug 18 '10 at 23:49
1

Ah the answer is good for C# only. It would be nice if the answer can be applied to even C++/CLI. – Just a HK developer Sep 15 '14 at 03:33

score 6 · Answer 2 · answered Aug 18 '10 at 22:58

6

You're right, you don't have to dispose the reader. But in the code given, it wouldn't hurt either.

I would not put a using block inside LoadDocument() because it is designed so that it 'borrows' it's stream (it does not create it).

But there are arguments to Dispose the XmlReader anyway, just because it's IDisposable. I don't think there is a clear winner here because of the disputable design of the Reader (and Writer) family: They Dispose their baseStreams without clearly being the owner of those streams.

answered Aug 18 '10 at 22:58

H H

263,252
30
330
514

1

Based off of what I read, `XmlReaderSettings` has a property `CloseInput` that defaults to `true`. Should I consider disabling this default behavior so I can dispose of `XmlReader` without disposing the underlying `Stream`? – Leonard Thieu Aug 18 '10 at 23:53
1

My book lied to me. The default value for `CloseInput` is `false`. – Leonard Thieu Aug 19 '10 at 03:35

Do I need to Dispose XmlReader if I Dispose its underlying Stream?

2 Answers2