Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Preventing to dispose objects multiple times

Consider the following code:

using (Stream stream = new FileStream("file.txt", FileMode.OpenOrCreate))
{
    using (StreamWriter writer = new StreamWriter(stream))
    {
        // Use the writer object...
    }
}

When the writer stream is beeing disposed it disposes internally the FileStream stream.

Is there any other desgin for this other than the MSDN recommendation to dispose the external used stream in the finally clause:

Stream stream = null;
try
{
    stream = new FileStream("file.txt", FileMode.OpenOrCreate);
    using (StreamWriter writer = new StreamWriter(stream))
    {
        stream = null;
        // Use the writer object...
    }
}
finally
{
    if(stream != null)
        stream.Dispose();
}
like image 529
CloudyMarble Avatar asked Mar 13 '13 13:03

CloudyMarble


2 Answers

This is a case where FxCop is strongly at odds with design choices in the .NET Framework. The problem is induced by the StreamWriter assuming ownership of the stream. Which is in general a "fall in the pit of success" design choice, most programmers will assume that closing the StreamWriter is sufficient to get the stream disposed. Particularly so when they use Close() instead of Dispose().

Which works well in the vast majority of cases. Most cases, one particular usage where it is very problematic is CryptoStream. A class that requires flushing and undiagnosably malfunctions when the underlying stream is closed before the CryptoStream is flushed and disposed. A case where the FxCop warning would be appropriate, albeit that it is too cryptic to easily recognize the specific problem ;)

And the general case where a programmer wrote his own Dispose() method and forgot to make it safe from being called more than once. Which is what the FxCop warning was meant to bring to the attention, it isn't otherwise smart enough to be able to see that a Dispose method is in fact safe.

In this specific case, the FxCop warning is just useless. All .NET Framework provided Dispose() method implementations are safe. FxCop should automatically suppress these kind of warnings for .NET framework code. But doesn't, Microsoft uses it too. There are a lot of [SuppressMessage] attributes in the .NET framework source code.

Working around the warning is way too ugly and error prone. And pointless since nothing actually goes wrong. Do keep in mind that FxCop is just a diagnostic tool, designed to generate "have you considered this" messages. It is not a police cop that's going to put you in jail when you ignore the rules. That's the job of a compiler.

Use the [SuppressMessage] attribute to turn off the warning.

like image 157
Hans Passant Avatar answered Oct 22 '22 05:10

Hans Passant


The solution for this particular case is to call the overload of the StreamWriter constructor that lets you tell it not to dispose the underlying stream.

Unfortunately, that's only for .Net 4.5; otherwise you'll have to do what you're already doing.

Also, see this thread: Is there any way to close a StreamWriter without closing its BaseStream?

Incidentally, the code in the OP does NOT cause an exception when I try it!

The sample below assumes a folder called "C:\TEST" exists:

using System;
using System.IO;

namespace Demo
{
    public static class Program
    {
        public static void Main(string[] args)
        {
            // This does NOT cause any exceptions:

            using (Stream stream = new FileStream("c:\\test\\file.txt", FileMode.OpenOrCreate))
            {
                using (StreamWriter writer = new StreamWriter(stream))
                {
                    writer.Write("TEST");
                }
            }
        }
    }
}
like image 41
Matthew Watson Avatar answered Oct 22 '22 06:10

Matthew Watson