1

Having the following:

StringWriter sw = null;
try
{
    sw = new StringWriter();
    using (var xw = new XmlTextWriter(sw))
    {
        doc.WriteTo(xw);
        return sw.ToString();
    }
}
finally 
{
    sw?.Dispose();
}

triggers the CA2202 (do not dispose objects multiple times) warning in Visual Studio 2015.

But the warning is not triggered if change the fianlly block to:

finally 
{
    if (sw != null)
    {
        sw.Dispose();
    }
}

Is that some strangeness of the null-conditional operator in a finally block or something, or do the analysis tools in Visual Studio simply not understand it?

EDIT: Possibly related: Why does Code Analysis flag me when using the null conditional operator with Dispose()?

Jon List
  • 1,504
  • 1
  • 14
  • 20
  • 2
    Any reason for performing manual disposal rather than just using a `using` statement? – Damien_The_Unbeliever Jan 07 '16 at 13:10
  • 1
    Actually, the warning is correct. The `StringWriter` is disposed twice, because `XmlTextWriter.Dispose()` will dispose it. – Henrik Jan 07 '16 at 13:25
  • @Henrik: that doesn't explain why the change in behaviour, though. I'd expect the warning to trigger if you don't explicitly Dispose, because `XmlTextWriter`'s behaviour is incidental to its implementation. – Dan Puzey Jan 07 '16 at 16:19
  • I'm disposing manually to avoid the `StringWriter` being disposed twice. If `XmlTextWriter` disposes the `StringWriter` then the null conditional should ensure that it will not be disposed again. – Jon List Jan 08 '16 at 10:17
  • If you take a look at the solution for the warning: https://msdn.microsoft.com/library/ms182334.aspx you see that the problem is not in the structure of the code, but only that i am using the null conditional instead of an `if` block. Right? – Jon List Jan 08 '16 at 10:30
  • @Damien_The_Unbeliever If you are passing a steam to another stream and using a `using` around them both, your outermost stream could be disposed twice: Once if the innermost stream disposes it when that is disposed and once again when running out of `using`-scope. Take a look at this link to learn more about the problem: https://msdn.microsoft.com/library/ms182334.aspx – Jon List Jan 12 '16 at 13:39

2 Answers2

0

Because you declared xw in the using block, when you exit the using block the IDisposable method for XmlTextWriter will be called. Since your string writer is only being used with the XMLWriter it will also be disposed by the garbage collector (it is an optimization to save the GC from having to rely on reference counting to determine if the object is still in use).

Edit: Further information can be found in this MSDN article.

MikeS159
  • 1,884
  • 3
  • 29
  • 54
  • I think you're confusing disposal and memory release here. The GC won't call `Dispose` on your object if you don't (it'll call a finalizer if there is one), and it doesn't do reference counting *regardless* of this - it releases the memory of any object that isn't rooted (which basically means: that doesn't have a reference to it held anywhere in running code). – Dan Puzey Jan 07 '16 at 16:28
  • The problem is not in the code. As i say, the warning is not triggered if I use the pattern that MS provide here: https://msdn.microsoft.com/library/ms182334.aspx. The problem is that the warning is triggered when i use the null conditional operator - as if the code analysis in VS doesnt know what it does. – Jon List Jan 08 '16 at 10:28
  • 1
    I have experienced problems with the VS code analysis flagging things that shouldn't be. You could just suppress the warning for that bit of code. – MikeS159 Jan 08 '16 at 11:07
0

Warning 'CA2202' is correct.

'sw' should be deleted in 'xw' if 'xw' is created, or manually deleted if 'xw' fails.

So you need 'sw = null' after creating 'xw'.

StringWriter sw = null;
try
{
    sw = new StringWriter();
    using (var xw = new XmlTextWriter(sw))
    {
        sw = null; //need to set null
        doc.WriteTo(xw);
        return sw.ToString();
    }
}
finally
{
    sw?.Dispose();
}
Jaeho Lee
  • 42
  • 5