What’s up with Code Analysis rule CA2202: Do not dispose objects multiple times?


You thought you'd be able to make it through the entire year without suffering through a CLR Week. And you almost made it.

But here it is. Ha ha. There's no theme for this week; it's just a collection of random CLR-related articles.

We begin with one of the more frustrating Code Analysis rules. Rule CA2202: Do not dispose objects multiple times.

Officially, the rule is to defend against buggy objects that do not implement Dispose correctly.

But I have another rationale for this warning: Disposing an object multiple times means that you don't really know who is in charge of the object. Calling Dispose means, "I'm finished, and I promise not to use it any more." If two people call Dispose, then the second one thought he could still use it but he can't because somebody else disposed it!

It's like saying "Last person to leave the room turns off the lights." Suppose there are two people in the room. One person leaves the room and turns out the lights. The second person is right behind and also turns out the lights. Turning off the lights is safe to do twice (the second time does nothing). But it means that the first person turned off the lights too soon. What if the second person wasn't directly behind? The first person just turned off the lights while there's still somebody in the room!

After the first Dispose, the object is dead. But there's still code that's using it, as evidenced by the fact that that other code also tries to Dispose it. If the other code knew that the object was dead, it wouldn't bother disposing it. You don't dispose dead objects, after all.

In my opinion, that is what rule CA2202 is trying to tell you. You lost track of the object's useful lifetime.

Comments (18)
  1. It can be problematic to suppress this and related warnings where you’re working with setups where ownership of objects may be passed as part of object construction and the analyzer knows about this passing of ownership. E.g. nesting streams that build upon each other.
    You create the inner stream, maybe do some preparatory work with it, then create the next stream nesting that one. If the constructor succeeds, you have to make sure you null out your own reference immediately afterwards to prevent this warning.

  2. The MAZZTer says:

    StreamReader/Writer will, by default, call .Dispose() on the stream passed into it when it itself is .Dispose()d. If you created the Stream object yourself you should be .Dispose()ing it anyway so you end up doing a .Dispose() twice.

    Except for that one time you don’t want to dispose it right away (you read some text and then some raw binary data, for example) in which case you get hit with an ObjectDisposedException because you forgot to tell StreamReader to not dispose the Stream.

    1. RKPatrick says:

      StreamReaders immediately jumped to mind when I read this, as AFAIK, it’s the only time I ever see this error. But IDisposable leads to all kinds of headaches, namely due to folks not wrapping them in ‘using’s or not reading its documentation (it’s meant for wrapping unmanaged resources!)….I have an OCD thing about proper IDisposable usage, and in fact I’ve been fighting some AccessViolation exceptions all day that I strongly suspect is due to a dependency not cleaning up its DB connections with proper ‘using’ blocks.

      1. gdalsnes says:

        “it’s meant for wrapping unmanaged resources”
        also commonly used for disconnecting from static events/events with longer lifetime, to prevent memory leak

  3. Darran Rowe says:

    The first thing that came to mind when I read this was it is a sad thing if you need to tell people why use after free (or use after dispose in this case) is a bad thing.
    It then lead me to wonder, is it managed code that really distances the idea of lifetime from the object or is it something about the quality of education in this area.

    1. RKPatrick says:

      The issue is that IDisposable is a bridge between managed and unmanaged. If you have a managed class representing an unmanaged (like an OS object) thing, when the GC cleans up the wrapper, there needs to be a mechanism to tell the unmanaged plumbing to clean up *its* corresponding object – the mechanism being IDisposable. I’d say it’s an education issue, since I rarely run into a job candidate who knows *why* IDisposable exists (“deterministic destruction” is a very imperfect answer), but the whole point of IDisposable is given on the first line of the class doc….it’s just that a lot of people don’t ever bother to read MSDN.

      1. gdalsnes says:

        “IDisposable is a bridge between managed and unmanaged”
        Not really, the finalizers is the bridge. Dispose is an optimization on top.

        1. stormhill117 says:

          No, finalizers are the safety net, the backup.
          To quote https://blogs.msdn.microsoft.com/oldnewthing/20100809-00/?p=13203/
          “A correctly-written program cannot assume that finalizers will ever run.”

          Additionally, .NET Core doesn’t run finalizers on normal process exit: https://stackoverflow.com/a/44735403/2676593

        2. RKPatrick says:

          Finalizers are non-deterministic (you don’t control when they run). This is not good for unmanaged resources, hence the use of deterministic IDisposable. And furthermore, this is the summary block for the MSDN documentation for IDisposable – “Provides a mechanism for releasing unmanaged resources.” Lots of people use them for deterministic destruction, but that is not it’s purpose. Seriously, it’s all in the documentation (and I’ve had this conversation with far too many devs – “READ MSDN!”)…even the remarks add “The primary use of this interface is to release unmanaged resources. “

  4. tballard says:

    I just did some tests with XmlReader.Create: When it is initialized from a Stream inside a Using CA2202 occurs if I use “using” on the reader and doesn’t occur if I don’t use “using” on the reader. But, if I do similar pattern using SqlConnection/SqlCommand, I don’t get CA2202 if use “using” on both…

    How are we supposed to know what will trigger it and the right way to call stuff? It seems like I always have to guess. Is there something I can look at in decompiled code to understand what triggers?

  5. KennyBiel says:

    A few simple rules will keep your usage of IDisposable 99.99% correct:
    1) If you instantiate within a code block, closure, or method and it is never seen outside that block, you own the lifetime; wrap a using around it and call it good.
    2) If you are passed an IDisposable, you do not own the lifetime; do not call Dispose() or Close(). Also, require the caller to have the object in a non-disposed or opened state.
    3) If you return an IDisposable, you do not own the lifetime; the caller must take responsibility for calling Dispose() and/or Close().
    4) If you instantiate an long-lived IDisposable in your class; you own the lifetime and your class must implement IDisposable itself. Make sure to never expose the private IDisposable object(s) you own to ensure you are in control of the lifetime.
    5) If you are passed an IDisposable in your constructor, you do not own the lifetime and do not need to implement IDisposable.

    If you follow these rules, you will never see a CA2202 and you should never have a leak assuming code you call or is calling yours does the right thing.

    As noted by others here, StreamReader/Writer is unfortunately not compliant with these rules. It takes over the lifetime of the Stream object that is passed to it. BinaryReader/Writer is slightly better in that it has a constructor overload which allows you to specify whether it should dispose of the passed Stream. Keep the using for the Stream and wrap the block with #pragma warning disable CA2202 and #pragma warning restore CA2202.

  6. It turns out sometimes you do want to double dispose, or rather would want to once a very old bug in .NET Core is fixed: https://github.com/dotnet/corefx/issues/8895

    I ended up calling Flush(true) on the filestream. Whether this is a workaround or the correct code anyway depends on the desired consistency model.

  7. TurcuT says:

    The warning is very confusing when it appears in situations like this:
    using (StreamWriter sw = new StreamWriter(fileName))
    using (JsonWriter writer = new JsonTextWriter(sw))
    {
    serializer.Serialize(writer, …);
    }

    In such cases, the developer tries to dispose two separate objects – he must dig trough documentation to know that the 2’nd objects, in some cases, “assumes” ownership over the first one and disposes it …

    1. Alex Cohn says:

      I believe this was done to encourage the following pattern:

      using (JsonWriter writer = new JsonTextWriter( new StreamWriter(fileName) ))
      {
      serializer.Serialize(writer, …);
      }

  8. VBA Derks says:

    While this rule makes sense in general, the analyzer that detects CA2202 generates too many false warnings. We tried to use it, but almost all issues it reported, were caused by the double Dispose of using statements in combination with StreamReader like objects that are going too take ownership of the disposable object. The analyzer should at least “filter” these kind of false warnings, for well known .NET framework classes.

    1. KennyBiel says:

      Or you can just use
      #pragma warning disable CA2202
      #pragma warning restore CA2202
      around known issues.

      1. VBA Derks says:

        I like to use static analysis tools, and #pragma disable/restore is very useful when only a few exceptional false warnings are reported. For our code base all reported warnings were from the “StreamReader type”. Leaving the rule enabled to catch problems like:

        obj.Dispose();
        obj.Dispose();

        wouldn’t make sense in our case.

        Note: it seems that the new Roslyn code analyzers (NuGet package Microsoft.CodeAnalysis.FxCopAnalyzers) don’t support CA2202 at the moment.

Comments are closed.

Skip to main content