3

The Run Code Analysis command in Visual Studio 2010 Ultimate returns a warning when seeing a certain pattern with MemoryStream and XmlTextWriter.

This is the warning:

Warning 7 CA2202 : Microsoft.Usage : Object 'ms' can be disposed more than once in method 'KinteWritePages.GetXPathDocument(DbConnection)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 421 C:\Visual Studio 2010\Projects\Songhay.DataAccess.KinteWritePages\KinteWritePages.cs 421 Songhay.DataAccess.KinteWritePages

This is the form:

static XPathDocument GetXPathDocument(DbConnection connection)
{
    XPathDocument xpDoc = null;
    var ms = new MemoryStream();
    try
    {
        using(XmlTextWriter writer = new XmlTextWriter(ms, Encoding.UTF8))
        {
            using(DbDataReader reader = CommonReader.GetReader(connection, Resources.KinteRssSql))
            {

                writer.WriteStartDocument();
                writer.WriteStartElement("data");

                do
                {
                    while(reader.Read())
                    {
                        writer.WriteStartElement("item");
                        for(int i = 0; i < reader.FieldCount; i++)
                        {
                            writer.WriteRaw(String.Format("<{0}>{1}</{0}>", reader.GetName(i), reader[i].ToString()));
                        }
                        writer.WriteFullEndElement();
                    }

                } while(reader.NextResult());

                writer.WriteFullEndElement();
                writer.WriteEndDocument();

                writer.Flush();
                ms.Position = 0;

                xpDoc = new XPathDocument(ms);
            }
        }

    }
    finally
    {
        ms.Dispose();
    }

    return xpDoc;
}

The same kind of warning is produced for this form:

XPathDocument xpDoc = null;
using(var ms = new MemoryStream())
{
    using(XmlTextWriter writer = new XmlTextWriter(ms, Encoding.UTF8))
    {
        using(DbDataReader reader = CommonReader.GetReader(connection, Resources.KinteRssSql))
        {
            //...
        }
    }

}

return xpDoc;

By the way, the following form produces another warning:

XPathDocument xpDoc = null;
var ms = new MemoryStream();
using(XmlTextWriter writer = new XmlTextWriter(ms, Encoding.UTF8))
{
    using(DbDataReader reader = CommonReader.GetReader(connection, Resources.KinteRssSql))
    {
        //...
    }
}

return xpDoc;

The above produces the warning:

Warning 7 CA2000 : Microsoft.Reliability : In method 'KinteWritePages.GetXPathDocument(DbConnection)', object 'ms' is not disposed along all exception paths. Call System.IDisposable.Dispose on object 'ms' before all references to it are out of scope. C:\Visual Studio 2010\Projects\Songhay.DataAccess.KinteWritePages\KinteWritePages.cs 383 Songhay.DataAccess.KinteWritePages

In addition to the following, what are my options?:

  • Supress warning CA2202.
  • Supress warning CA2000 and hope that Microsoft is disposing of MemoryStream (because Reflector is not showing me the source code).
  • Rewrite my legacy code to recognize the wonderful XDocument and LINQ to XML.
rasx
  • 5,288
  • 2
  • 45
  • 60
  • possible duplicate of [C# CA2000:Dispose objects before losing scope using FileStream/XmlTextReader](http://stackoverflow.com/questions/3128446/c-ca2000dispose-objects-before-losing-scope-using-filestream-xmltextreader) – Hans Passant Dec 21 '10 at 21:59

2 Answers2

4

First of all, you should never use new XmlTextWriter(). It has been deprecated since .NET 2.0. Use XmlWriter.Create() instead.

Secondly, the assignment to ms should be in a using block:

using (var ms = new MemoryStream())
{
    using (var writer = XmlWriter.Create(ms))
    {
        // ...
    }
}

I believe that the warning is correct. The MemoryStream could be disposed when the XmlTextWriter is disposed, then again in the "finally" block.

John Saunders
  • 160,644
  • 26
  • 247
  • 397
  • 1
    Something to note here: In later versions of .Net Framework, XmlWriter.Create(stream) creates an object where XmlWriter.Settings.CloseOutput is 'false', and you do actually need to dispose of the MemoryStream separately. The problem is that CA2202 is still raised even though Dispose() is only being called on the underlying stream precisely once. I just suppressed the warning and justified it as a false positive. – Russell Phillips Mar 04 '19 at 01:35
  • @russell that makes good sense. If you pass it an existing stream, then it belongs to you, and you know when to close it. – John Saunders Mar 05 '19 at 02:15
1

If this were my code-base I'd suppress it. Code Analysis is there to warn you of potential problems and as long as you (and everyone else on your team, including all future devs) are aware of the potential problems you're fine. Here's Microsoft's way of avoiding the issue (which defeats the purpose of using() in my opinion.

On a similar note, here's some code that shows you how you might actually run into the bug that this fixes. The first code block writes to a MemoryStream closes the StreamWriter and then tries to read it in another StreamReader. Unfortunately Dispose() on the StreamWriter also closes the MemoryStream. The solution is to create the StreamReader from within the StreamWriter.

Chris Haas
  • 53,986
  • 12
  • 141
  • 274