27

I recently downloaded Visual Studio 2013 and I ran the Code Analysis on a project I'm working on. Its thrown up a couple of issues that I'm working through but one in particular is about how I am using the "using" IDisposable statement.

Here's an example of my code:

using (MemoryStream msDecrypt = new MemoryStream(encryptedText.ToBase64Byte()))
{
    using (CryptoStream csDecrypt = new CryptoStream(msDecrypt, decryptor, CryptoStreamMode.Read))
    {
        using (StreamReader srDecrypt = new StreamReader(csDecrypt))
        {
            return srDecrypt.ReadToEnd();
        }
    }
}

I understand the Analysis warning is trying to tell me that disposing of multiple objects in this way could throw an object disposed issue.

I'm sure that disposing on one object is not going to throw an exception in the case above. So should I modify my code or keep as is?

ylun.ca
  • 2,504
  • 7
  • 26
  • 47
Rob
  • 6,819
  • 17
  • 71
  • 131
  • 2
    Such practice has to date never failed for me so I think it's perfectly fine. – Mo Patel Mar 11 '14 at 10:34
  • Thanks everyone for your feedback. I was sure that my implementation of the using statement was fine but started to doubt myself when the Analysis tool highlighted it to me. I'll mark the first answer as correct but all are very valid! Thanks! – Rob Mar 11 '14 at 10:37
  • You could also have all of these in one `using` statement, with the variable type set to `Stream`, the only issue is you would have to cast inside the block for `ReadToEnd` to be present. It does remove the warning though. See [this question](http://stackoverflow.com/questions/21118201/can-using-with-more-than-one-resource-cause-a-resource-leak) – Joe Mar 11 '14 at 10:39
  • That said, just ignore the warning, you shouldn't have to add unnecessary obscurity to avoid a false-positive code analysis warning. – Joe Mar 11 '14 at 10:40
  • 1
    Some of these streams will get disposed several times, since `StreamReader` owns and disposes the associated stream. – CodesInChaos Mar 11 '14 at 15:02
  • I've recently written an article on some of the subtle cases involving exceptions and the `using` statement, which might interest you. http://blog.coverity.com/2014/02/26/resources-vs-exceptions – Eric Lippert Mar 11 '14 at 21:43
  • Style tip: you can skip the braces in the outer `using` blocks, then Format Document. The `using`s will be vertically aligned. – Jay Bazuzi Mar 11 '14 at 23:12

5 Answers5

22

There should be no issue with your code as far as I can tell, I have always used nested using statements in the past.

From looking through other questions I believe the problem is with the code analysis tool itself and both CA2000 and CA2202 rules. False positives often occur when using various stream and reader types with using statements.

You should ignore the warnings and continue as alternative methods (such as try/finally) will produce bad code and your code is valid.

Community
  • 1
  • 1
Matthew Hudson
  • 1,306
  • 15
  • 36
17

The reason From 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.

in your case though. it is fine to use it like this. So, you should just suppress the warning.

OR

you can just go the ugly way of suppressing the warning by modifying your code by implementing Try/finally. Although, you should really avoid that.

Ehsan
  • 31,833
  • 6
  • 56
  • 65
14

I don't see any issue about using the multiple using statements to dispose the objects. It's common to have nested using statements and your code is valid.

Using objects of different types, you can nest using the one-line syntax if you want to save space and improve readability:

using (MemoryStream msDecrypt = new MemoryStream(encryptedText.ToBase64Byte()))
using (CryptoStream csDecrypt = new CryptoStream(msDecrypt, decryptor, CryptoStreamMode.Read))
using (StreamReader srDecrypt = new StreamReader(csDecrypt))
{
     return srDecrypt.ReadToEnd();
}

The issue you encountered is recurrent using the Code Analysis (you can find the cause of the warning here). You can then ignore the warning about the multiple using statements, because this is a false positive and your code is fine and valid.

You can also suppress the warning in this way (thanks to @Jordão), as described in this nice answer:

[SuppressMessage("Microsoft.Usage", "CA2202:Do not dispose objects multiple times")]
public void MyMethodWithUsings()
{
     using (MemoryStream msDecrypt = new MemoryStream(encryptedText.ToBase64Byte()))
     using (CryptoStream csDecrypt = new CryptoStream(msDecrypt, decryptor, CryptoStreamMode.Read))
     using (StreamReader srDecrypt = new StreamReader(csDecrypt))
     {
           return srDecrypt.ReadToEnd();
     }
}

If you want to remove the warning, you'll have to rewrite your code using the block try/finally and call the Dispose() method of your objects in the finally block, but this will produce awful and not easy to read code, as you can see in this question.

Community
  • 1
  • 1
Alberto Solano
  • 7,972
  • 3
  • 38
  • 61
7

That's a recurrent issue with code analysis and both CA2000 and CA2202.

You have the possibility to fix those warnings by not using using, but using try/finally/dipose. That's not recommended because it produces awful code.

Your code will work, simply suppress those warnings.

Also see https://stackoverflow.com/a/21849534/870604 for StreamReader/Writer and underlying stream ownership.

Community
  • 1
  • 1
ken2k
  • 48,145
  • 10
  • 116
  • 176
7

For this type of warning you can easily ignore the warning, because it usually is a false positive. Check the following links for more information:

Community
  • 1
  • 1