Search code examples
c#disposeidisposable

SqlXml not taking responsibility for disposing of MemoryStream


I was looking doing a code review today whereby someone one attempting to dispose of a MemoryStream by wrapping it up in a using block. In this case it was being passed into a SqlXml instance, and I wasn't sure what it did with it, so I took a quick look at the decompiled source. Here's the example code:

using(MemoryStream ms = new MemoryStream(data)) {
   var param = new SqlXml(ms);
   var parameter = new SqlParameter("@Xml", System.Data.SqlDbType.Xml) { Value = param };
   command.Parameters.Add(parameter);
}

return command;

Here is the decompiled source:

public SqlXml(Stream value)
{
    if (value == null)
    {
        this.SetNull();
    }
    else
    {
        this.firstCreateReader = true;
        this.m_fNotNull = true;
        this.m_stream = value;
    }
}

Now best practice says that the SqlXml should now take responsibility for disposing of this stream. But unfortunately it doesn't implement IDisposable.

On top of this I don't know when the SqlCommand containing it is going to get consumed and SqlXml is sealed, so I can realistically extend it.

Can anyone suggest a good approach for handling this? I'm planning on logging a bug somewhere with Microsoft because it's not a very nice thing to have to workaround.

My best suggestion at the moment is to use the constructor that takes an XmlReader because it at least copies the contents, so the reader can then be disposed - however internally that just creates a MemoryStream and never cleans it up either.


Solution

  • One important thing to remember when implementing IDisposable is DO NOT dispose of IDisposable instances that you do not "own" (those which your class has not specifically created). So TL;DR, it wouldn't be a good idea for SqlXml to implement IDisposable since it didn't create the underlying Stream.