2

I get the CA2202 warning on the following piece of code

using (MemoryStream msDecrypt = new MemoryStream(encrypted))
        using (CryptoStream csDecrypt = new CryptoStream(msDecrypt, decryptor, CryptoStreamMode.Read))
using (StreamReader srDecrypt = new StreamReader(csDecrypt))
        return srDecrypt.ReadToEnd();

This code is triggering on both msDecrypt and csDecrypt having their own using statements. Is there a preferred object to dispose of? The outer (msDecrypt) or the inner (csDecrypt)- and if so why?

This question is not a duplicate of this thread because I want to know generally speaking - which is better to dispose of - the inner/later object or the outer/earlier object and why?

Community
  • 1
  • 1
bkr
  • 1,444
  • 1
  • 11
  • 22
  • 1
    Dispose exceptions are not hidden. Polite objects will not dispose of themselves twice, however. (At least, disposable code *I* would write.) – Anthony Pegram May 21 '15 at 21:03
  • hmm, you're right. https://msdn.microsoft.com/en-us/library/aa355056%28v=vs.110%29.aspx still, assuming I dispose of only one... which one. and why? – bkr May 21 '15 at 21:40
  • 1
    Dispose of both. Suppress the warning or ignore it. – Blorgbeard May 21 '15 at 22:09
  • that's what I'm currently doing (as shown in the code above). what I'm asking though is if we just dispose of one - is there a reason to choose one over the other? – bkr May 21 '15 at 22:12
  • @bkr Given a good option A, a bad option B and a bad option C, asking which of B or C is better generally doesn't make sense. It's like asking if for help choosing the language for coding a CMS, where you then limit the choices to FORTRAN and COBOL. –  May 21 '15 at 22:16
  • I'm not saying you're wrong @hvd - but I've also not seen any reasons to consider why A is the good option and B and C are bad options. with option A as you put it - we are effectively calling dispose twice on the underlying object. why is that the best option and why shouldn't we try limit calling dispose twice? why is calling dispose twice so obviously the best solution - when it can in certain conditions cause exceptions? Also - I already know how to do A. what I'm asking is "why would I use B or C instead of A?". calling them "bad option" and "good option" is not helpful. – bkr May 21 '15 at 22:24
  • 2
    Basically, calling `Dispose()` is guaranteed to be safe, it is mentioned in the documentation of `IDisposable` that it will have no bad effects. If you worry about classes that claim to implement `IDisposable`, but actively break explicit guarantees made by that interface, why not worry just as much about classes where even the first `Dispose()` fails? –  May 21 '15 at 22:29
  • it specifically says in multiple places that while a properly implemented Dispose should not throw an exception - it is NOT guaranteed. if that wasn't the case, then presumably there would be no warning CA2202 as it would be pointless? see the linked article from John Hodge below – bkr May 22 '15 at 05:04
  • @bkr The [`IDisposable.Dispose()`](https://msdn.microsoft.com/en-us/library/system.idisposable.dispose%28v=vs.110%29.aspx) is the authoritative reference for that method, and it says "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." That's "must", not "should". John Hodge links to Code Analysis documentation, and that documentation is misleading in its presentation of what's required and what's only recommended. –  May 22 '15 at 07:26
  • I agree with you in that a Dispose() method that would throw an exception if its called multiple times is incorrectly implemented. It's a question of do you want to protect against bad implementations or not I guess. If it's a MS library vs a 3rd party may influence you one way or another. – bkr May 22 '15 at 16:23

1 Answers1

2

This is explained here if you scroll down to the Example section. In short, this is caused by the resource in the inner using block also containing the resource of the outer using block. When you call Dispose on the inner resource, it also disposes of the outer resource that is contained therein.

The suggested fix is to wrap the whole thing in a try block, put the inner resource in a using block, and then call Dispose on the outer resource inside of a finally block if it is not already null.

To answer your question more directly, the inner resource should be the one that is preferable to dispose of.

John Hodge
  • 1,645
  • 1
  • 13
  • 13
  • I'm going to mark this as the answer since it provides "official" guidance - dispose of the inner and verify if somehow the outer still needs to be disposed. However - it's an unwieldy way to do things and I'm not convinced it isn't without it's own potential issues and would have preferred something with more in-depth info. – bkr May 22 '15 at 05:42