C# CA2000:Dispose objects before losing scope using FileStream/XmlTextReader

Question

I have lots of code like this:

FileStream fs = File.Open(@"C:\Temp\SNB-RSS.xml", FileMode.Open); 
using (XmlTextReader reader = new XmlTextReader(fs)) 
{ 
   /* Some other code */
}

This gives me the following Code Analysis warning:

CA2000 : Microsoft.Reliability : In method 'SF_Tester.Run()', object 'fs' is not disposed along all exception paths. Call System.IDisposable.Dispose on object 'fs' before all references to it are out of scope.

If I follow the suggestion and I put the File.Open in a using statement, I get this:

CA2202 : Microsoft.Usage : Object 'fs' can be disposed more than once in method 'SF_Tester.Run()'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 39

I'm using VS2010 and I can't help but think I'm doing something wrong but I don't see it. What am I doing wrong?

Answer 1

Use the using statement also on the FileStream itself just like on the XmlTextReader.

http://msdn.microsoft.com/en-us/library/system.io.filestream(VS.71).aspx.

Grz, Kris.

Answer 2

Have you tried?

using (var fs = File.Open(@"C:\Temp\SNB-RSS.xml", FileMode.Open))
using (var reader = new XmlTextReader(fs))  
{  
   /* Some other code */ 
} 

Answer 3

Sigh, exhausting isn't it. Avoid all this by using the recommended Create() method:

 using (var reader = XmlReader.Create(@"C:\Temp\SNB-RSS.xml")) {
     //...
 }

Answer 4

I am only guessing; don't have time to go through a full analysis now.

Suppose the XmlTextReader constructor 'takes ownership' of the stream passed in, and so disposing the XmlTextReader will also Dispose the underlying stream. That would explain the behavior you see. Perhaps XmlTextReader constructor can throw, and in that instance, the original warning about fs would make sense. However, given that hypothesis, this code

        var fs = File.Open(@"C:\Temp\SNB-RSS.xml", FileMode.Open);
        XmlTextReader reader = null;
        try
        {
            reader = new XmlTextReader(fs);
        }
        finally
        {
            if (reader== null)
            {
                fs.Dispose();
            }
        }
        if (reader != null)
        {
            using (reader)
            {
                /* Some other code */
            }
        }

is, I think, correct, but still yields a spurious warning. This smells like a nice example that demonstrates the limitations of static analysis tools.

As someone else said, there is another API to directly create the reader from the filename (XmlReader.Create()), which avoids all this (and shows how well-designed scenario-focused APIs are a good thing for a surprising variety of reasons).

Answer 5

It's a known issue

http://connect.microsoft.com/VisualStudio/feedback/details/535118/ca2000-and-ca2202-offer-contradictory-warnings

If you're using a StreamWriter rather than XmlTextReader (as in the solution above) you could use a similar method via the relevant constructor; e.g.

var sw = new StreamWriter("filename.txt");

or

var sw = new StreamWriter("filename.txt", /*append to file = */ false );

It is not clear from the documentation whether the first form of constructor will overwrite or append to a file.

Answer 6

As nobody provided a solution that solves this issue yet, I'm writing my working solution down here:

FileStream fs = new FileStream(fileName, FileMode.Truncate, FileAccess.ReadWrite,    FileShare.ReadWrite);
try
{
   using (var fileWriter = new StreamWriter(fs, encoding))
   {
       fs = null;
       fileWriter.Write(content);
    }
 }
 finally
 {
     if (fs != null)
         fs.Dispose();
 }

This removes CA2000.