Ca2202, How to Solve This Case

How To Solve CA2202:To avoid generating a System.ObjectDisposedException Warning

MSDN:

Nested using statements (Using in Visual Basic) can cause violations
of the CA2202 warning. If the IDisposable resource of the nested inner
using statement contains the resource of the outer using statement,
the Dispose method of the nested resource releases the contained
resource. When this situation occurs, the Dispose method of the outer
using statement attempts to dispose its resource for a second time.

Problem:

using (StreamReader file = File.OpenText(file_path))
{
    using (JsonTextReader reader = new JsonTextReader(file))
    {
        return (JObject)JToken.ReadFrom(reader);//the warning is here
    }   //"file" will be disposed here for first time when "reader" releases it
}   //and here it will be disposed for the second time and will throw "ObjectDisposedException"

Solution:

You need to do it like this(disposing the object in finally block when all goes right, or in catch block when an error occurs):

public static JObject ReadJson(string file_path)
{   
    StreamReader file = null;
    try {
        JObject o1 = JObject.Parse(File.ReadAllText(file_path));
        file = File.OpenText(file_path);
        using (JsonTextReader reader = new JsonTextReader(file))
        {
            return (JObject)JToken.ReadFrom(reader);
        }
    }
    catch
    {
        return default(JObject);
    }
    //dispose "file" when exiting the method
    finally
    {
        if(file != null)
            file.Dispose();
    }
}

How do I fix the CA2202 warning?

The reason for getting this warning is that XmlTextWriter.Dispose() will ensure that the under lying MemoryStream object is also disposed. So when the using scope of the MemoryStream ends, it will try disposing the MemoryStream object and hence the warning.

using block compiles into a try-finally block. Inner using block in your code would call Dispose on your writer. That will call Dispose on your MemoryStream object mStream. At the exit of control from inner using block, the outer using block will try to dispose the object writer, but since it has already been disposed, you are getting the warning on Code analysis tool.

To get rid of the warning, you can remove the first using statement and use a try-finally block. But remember to set mStream to null as soon as you enter the second using statement. This has been explained at CA2202: Do not dispose objects multiple times

Your code would look like:

public static String PrintXML(String XML)
{
    String result = "";
    string[] xmlSeperators = new string[] { "<?" };
    string[] splitResults = new string[2];

    if (!String.IsNullOrEmpty(XML))
    {
        MemoryStream mStream  = null;
        try
        {
            mStream = new MemoryStream();
            using (XmlTextWriter writer = new XmlTextWriter(mStream, Encoding.Unicode))
            {
                mStream = null; // important 
                XmlDocument document = new XmlDocument();
                try
                {
                    // Load the XmlDocument with the XML.
                    //Check if it is only XML 
                    if (XML.StartsWith("<?"))
                    {
                        document.LoadXml(XML);
                    }
                    else
                    {
                        //Split the string appended before XML
                        splitResults = XML.Split(xmlSeperators, 2, StringSplitOptions.None);
                        if (splitResults.Length > 1)
                        {
                            string d = "<?" + splitResults[1];
                            document.LoadXml(d);
                        }
                    }
                    writer.Formatting = System.Xml.Formatting.Indented;
                    // Write the XML into a formatting XmlTextWriter
                    document.WriteContentTo(writer);
                    //xx.WriteTo(writer);
                    writer.Flush();
                    mStream.Flush();
                    // Have to rewind the MemoryStream in order to read its contents.
                    mStream.Position = 0;
                    // Read MemoryStream contents into a StreamReader.
                    StreamReader sReader = new StreamReader(mStream);
                    // Extract the text from the StreamReader.
                    String FormattedXML = sReader.ReadToEnd();

                    if (splitResults[0] != null)
                    {
                        result = splitResults[0] + "\n" + FormattedXML;
                    }
                    else
                    {
                        result = FormattedXML;
                    }
                }
                catch (XmlException xe)
                {
                    Log.Error(xe);
                    throw;
                }
            }
        }
        finally 
        {

            if (mStream != null)
            {
                mStream.Dispose();
            }

        }
    }
    return result;
}

How to satisfy CA2202 (Do not dispose objects multiple times)

This is a literal answer to your question, in that it will not issue CA warnings without suppressing them, and will only ever call every Dispose once:

MemoryStream encryptedStream = null;
CryptoStream cryptStream = null;
try {
    encryptedStream = new MemoryStream();
    cryptStream = new CryptoStream(encryptedStream, cryptoTransform, CryptoStreamMode.Write);
    cryptStream.Write(inputInBytes, 0, inputInBytes.Length);
    cryptStream.FlushFinalBlock();
    result = encryptedStream.ToArray();
} finally {
    if (cryptStream != null) {
        cryptStream.Dispose();
   } else {
        if (encryptedStream != null) encryptedStream.Dispose();
   }
}
string output = Convert.ToBase64String(result);

But any developer worth their salt should take a look at this and go "hmm, it's like they didn't know using, I'd better rewrite that". Do not do this in production code. Suppress the warning. Getting code like this correct (and having it remain correct in the face of changes) is actually harder than writing code that uses using with suppression of spurious warnings (indeed, I'm not entirely sure the above code is correct!). It defeats the entire point of having static code analysis in the first place: to write reliable code. You should see code analysis as a tool, not an arbiter of correctness.

You should not call Dispose more than one time on an object (CA2202)

This code analysis warning is total baloney. The contract for IDisposable requires that extra calls to Dispose are accepted and do nothing (in particular, they should not throw ObjectDisposedException or any other exception).

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.

Source: IDisposable.Dispose documentation on MSDN

Unfortunately some framework code was written without reading the contract, and forbids calling Dispose more than once. Those objects you should be careful not to double dispose. But the universal contract is still that for an arbitrary IDisposable, calling Dispose multiple times is permitted.

What is causing 'CA2202: Do not dispose objects multiple times' in this code and how can I refactor?

It is a false warning, caused by XmlWriter disposing the stream you pass. Which makes your StringWriter disposed twice, first by XmlWriter and again by your Using statement.

This is not a problem, disposing .NET framework objects twice is not an error and doesn't cause any trouble. It could be a problem if the Dispose() method is poorly implemented, FxCop doesn't take its chances to not tell you about it because it isn't otherwise smart enough to know if a Dispose() method is correct.

There is not any way to rewrite the code to avoid a warning. StringWriter doesn't actually have anything to dispose so moving it out of the Using statement is okay. But that will produce another warning, CA2000. Best thing to do is to just ignore this warning. Use the SuppressMessageAttribute if you don't want to look at it again.

Dispose of object more than one time error. CA2202. Is there a better way?

Yes, imho you really should only call it once.

Alternatively you could use the using syntax on ns, which makes the whole situation even clearer.

using (ns = new NetworkStream(CreateConnection(), true)) {
   ...
}

Cannot get rid of CA2202 warning

If you don't use the underlying streams after you've "passed ownership" of these objects to other objects, you can silence the warning like this:

        FileStream fileStream = null;
        BufferedStream bufferedStream = null;
        try
        {
            fileStream = File.Open(...);
            bufferedStream = new BufferedStream(fileStream);

            fileStream = null;

            using (StreamReader streamReader = new StreamReader(bufferedStream))
            {

                bufferedStream = null;

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

            if (fileStream != null)
            {
                fileStream.Dispose();
            }
        }

You'll want the assignments to null to occur immediately following the constructor call that "takes ownership" of the disposable object. In this way, you ensure that if the constructor throws, you dispose the inner object and if the constructor succeeds then it is then going to arrange for disposal to occur.

---

Related Topics