Rule S3966: Objects should not be disposed more than once

[Evangelink]

Implements RSPEC-3966: Objects should not be disposed more than once

Contributes to MMF-963.

[nielsenko]

From https://docs.microsoft.com/en-us/dotnet/api/system.idisposable.dispose?redirectedfrom=MSDN&view=netframework-4.8#System_IDisposable_Dispose

If an object's Dispose method is called more than once, the object must ignore all calls after the first one. The object must not throw an exception if its Dispose method is called multiple times. Instance methods other than Dispose can throw an ObjectDisposedException when resources are already disposed.

So double dispose is explicitly supported by the IDispose contract. So why in the world do you prefer the try-catch-using combo. It makes the code longer and harder to read, especially compared to C# 8 (https://docs.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-8#using-declarations).

I know I can disable this rule, but I fail to comprehend why you would make this a default?

[nielsenko]

I suppose you blindy implemented CA2202 from FxCop, that Roslyn explicitly decided against porting (see dotnet/roslyn-analyzers#528). See http://blogs.msdn.com/b/tilovell/archive/2014/02/12/the-worst-code-analysis-rule-that-s-recommended-ca2202.aspx for a true rant.

[ericjohannsen]

The example of non-compliant code you provide is incorrect. StreamWriter.Dispose() does not call Dispose() on the underlying stream. It does call .Close() if the underlying stream is closeable, but that isn't the same thing. There is nothing wrong with a closed stream subsequently being disposed of.

https://github.com/microsoft/referencesource/blob/master/mscorlib/system/io/streamwriter.cs

For reference, here's the example you provide:

using (Stream stream = new FileStream("file.txt", FileMode.OpenOrCreate))
{
    using (StreamWriter writer = new StreamWriter(stream))  // Noncompliant: 'stream' will be disposed twice
    {
        // Use the writer object...
    }
}

[costin-zaharia-sonarsource]

Hi @ericjohannsen,

indeed, the rule description provides an incorrect example for non-compliant code. I've added an issue and you can follow the progress here: #3644

Thanks for your feedback!

[costin-zaharia-sonarsource]

Considering the feedback received for this rule we decided to:

• remove rule from Sonar way profile (Remove S3966 from SonarWay profile #3645)
• fix description and remove the FP with StreamReader and StreamWriter (Rule S3966: FP when using StreamWriter  #3644)