Preventing to dispose objects multiple times

Question

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();
}

You should be able to call Dispose() for the same object multiple times, according to Microsoft. — Matthew Watson, Mar 13 '13 at 13:46
@MatthewWatson, a reference quoting Microsoft on that would be nice.. — Mike Dinescu, Mar 13 '13 at 13:49
@MatthewWatson when the 1nd using try to dispose an object which is allready disposed its causing an error — CloudyMarble, Mar 13 '13 at 14:07
No, it's not trying to dispose an object which is already disposed, it's trying to USE that object (by closing/flushing it). — Matthew Watson, Mar 13 '13 at 14:22
@Miky: See http://msdn.microsoft.com/en-GB/library/system.idisposable.dispose.aspx, which says *If an object's Dispose method is called more than once, the object must ignore all calls after the first one.* — Matthew Watson, Mar 13 '13 at 14:23
I also have to point out that the first section of code in the OP does *not* cause an exception. — Matthew Watson, Mar 13 '13 at 14:28
@MatthewWatson - unfortunately the documentation is at odds with the Warning: `Do not suppress a warning from this rule. Even if Dispose for the object is known to be safely callable multiple times, the implementation might change in the future.` in http://msdn.microsoft.com/en-us/library/vstudio/ms182334%28v=vs.110%29.aspx — Mike Dinescu, Mar 13 '13 at 14:35
I'm not saying you're not right in that it's safe to call dispose multiple times on the Stream.. just that the Warning is strange per MS documentation. — Mike Dinescu, Mar 13 '13 at 14:36
@MatthewWatson your right, i was assuming there could be a emmoryleak out of this, i tested the code you posted and it works. still not correctl implemented dispose methods could be an issue if disposed more than once. thank you for clearing this point. — CloudyMarble, Mar 13 '13 at 14:40
@Miky: Yep, because there is no language-level support to ensure that calling Dispose() multiple times is ok, you have to assume it isn't - which is annoying. ;) — Matthew Watson, Mar 13 '13 at 14:44
I would personally ignore the CA2202 rule; it seems very misguided. As per the documentation for Dispose (msdn.microsoft.com/en-us/library/…), "The object *must not* throw an exception if its Dispose method is called multiple times." I suppose there could be some poorly-implemented objects that do throw, but on the whole it's better to Dispose everything that's IDisposable, and not worry if something gets disposed twice (which should be a no-op). — Bradley Grainger, Mar 13 '13 at 15:09
It's very funny because the code suggested by Microsoft actually triggers the rule. :/ — Maria Ines Parnisari, Mar 27 '17 at 21:33

score 3 · Answer 1 · edited May 23 '17 at 12:25

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");
                }
            }
        }
    }
}

Why are you talking about exceptions? This is a code analysis warning. — Hans Passant, Mar 13 '13 at 15:39
Just an Info: In my case i just noticed even .net 4.5 wouldnt have helped me since the "inner" stream in my case is XmlTextWriter which does not have the leaveOpen flag! — CloudyMarble, Mar 14 '13 at 07:22

score 3 · Accepted Answer · answered Mar 13 '13 at 16:00

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.

Its really funny to know that Microsoft uses SupressesMessage to supress its warnings. alot of people see these rules as a programming bible. In my case it was more complicated as i used a stringwriter and textxmlwriter, so i would have to keep the string in a tempvariable before i can set the stringwriter to null as suggested on msdn. — CloudyMarble, Mar 14 '13 at 06:47

Preventing to dispose objects multiple times

2 Answers2