Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Nested using statements and Microsoft code Analyses

Recently I switched on additional code analyses rules. To my surprise I saw a violation in a place I was always considering as the best practice. If I have two nested disposables I am putting two using statements like this:

    using (StringReader strReader = new StringReader(xmlString))
    using (XmlReader xmlReader = XmlReader.Create(strReader))
    {
        result.ReadXml(xmlReader);
    }

This also corresponds to the high rated Q&A Nested using statements in C#

The violation I get states following:

Warning 18  CA2202 : Microsoft.Usage : Object 'strReader' can be disposed more
than once in method '????'. To avoid generating a System.ObjectDisposedException
you should not call Dispose more than one time on an object.: Lines: ??

What I did was an intuitive try and error, thinking that close of outer stream will also probably dispose the inner one I quick fixed my code like this:

    using (XmlReader xmlReader = XmlReader.Create(new StringReader(xmlString)))
    {
        result.ReadXml(xmlReader);
    }

Hura! The warning is gone. But, tada! The new one occurred:

Warning 18  CA2000 : Microsoft.Reliability : In method '????????', object 
'new StringReader(xmlString)' is not disposed along all exception paths. Call
System.IDisposable.Dispose on object 'new StringReader(xmlString)' before all 
references to it are out of scope.

Then I found a very ugly solution:

    {
        StringReader strReader = null;
        try
        {
            strReader = new StringReader(xmlString);
            using (XmlReader xmlReader = XmlReader.Create(strReader))
            {
                strReader = null;
                result.ReadXml(xmlReader);
            }
        }
        finally
        {
            if (strReader != null) strReader.Dispose();
        }
    }

As a very last step (like every good programmer) I looked into help page for CA2202 and to my surprise exactly my last UGLY solution was proposed to fix the issue?

Having try{} finally around using clutters the code very much! For me is the nested using much more readable.

Question: Is there a better way of doing things? I am looking for a solution which will be intuitively understandable. Everyone who will see this last snippet will be curios about what is happening.

Thanks in advance for your answers.

like image 826
George Mamaladze Avatar asked Nov 11 '11 15:11

George Mamaladze


1 Answers

The problem isn't because of the nested usings. They're fine and generally recommended. The problem here is that XmlReader will dispose the TextReader if you pass an XmlReaderSettings with CloseInput == true, but the CA2202 rule isn't smart enough that your code won't go down that branch. Keep your nested usings, and suppress the CA2202 violation as a false positive.

If you want to be explicit in your code in order to enhance its readability and/or maintainability, use an XmlReaderSettings with CloseInput set to false, but that is the default value, so it's not strictly necessary, and, to be clear, would not satisfy the rule.

BTW, there are similar CA2202 problem scenarios for a variety of stream and reader types. Unfortunately, they're not all the same as this one, so the best case handling can differ depending on which type is cause the problem.

like image 74
Nicole Calinoiu Avatar answered Nov 11 '22 23:11

Nicole Calinoiu