For Microsoft built classes that inherit IDisposable, do I explicitly have to call Dispose?

Question

Regarding the Microsoft built classes that inherit IDisposable, do I explicitly have to call Dispose to prevent memory leaks?

I understand that it is best practice to call Dispose (or better yet use a using block), however when programming, typically I don't always immediately realise that a class inherits from IDisposable.

I also understand that Microsoft implementation of IDisposable is a bit borked, which is why they created the article explaining the correct usage of IDisposable.

Long story short, in which instances is it okay to forget to call Dispose?

Answer 1

There are a couple of issues in the primary question

Do I explicitly have to call Dispose to prevent memory leaks?

Calling Dispose on any type which implements IDisposable is highly recomended and may even be a fundamental part of the types contract. There is almost no good reason to not call Dispose when you are done with the object. An IDisposable object is meant to be disposed.

But will failing to call Dispose create a memory leak? Possibly. It's very dependent on what exactly that object does in it's Dispose method. Many free memory, some unhook from events, others free handles, etc ... It may not leak memory but it will almost certainly have a negative effect on your program

In which instances is it okay to forget to call Dispose?

I'd start with none. The vast majority of objects out there implement IDisposable for good reason. Failing to call Dispose will hurt your program.

Answer 2

It depends on two things:

1. What happens in the Dispose method
2. Does the finalizer call Dispose

Dispose functionlity
Dispose can do several type of actions, like closing a handle to a resource (like file stream), change the class state and release other components the class itself uses.
In case of resource being released (like file) there's a functionality difference between calling it explicitly and waiting for it to be called during garbage collection (assuming the finalizer calls dispose).
In case there's no state change and only components are released there'll be no memory leak since the object will be freed by the GC later.

Finalizer
In most cases, disposable types call the Dispose method from the finalizer. If this is the case, and assuming the context in which the dispose is called doesn't matter, then there's a high chance that you'll notice no difference if the object will not be disposed explicitly. But, if the Dispose is not called from the finalizer then your code will behave differently.

Bottom line - in most cases, it's better to dispose the object explicitly when you're done with it.

A simple example to where it's better to call Dispose explicitly: Assuming you're using a FileStream to write some content and enable no sharing, then the file is locked by the process until the GC will get the object. The file may also not flush all the content to the file so if the process crashes in some point after the write was over it's not guaranteed that it will actually be saved.

Answer 3

It can be safe to not call Dispose, but the problem is knowing when this is the case.

A good 95% of IEnumerator<T> implementations have a Dispose that's safe to ignore, but the 5% is not just 5% that'll cause a bug, but 5% that'll cause a nasty hard to trace bug. More to the point, code that gets passed an IEnumerator<T> will see both the 95% and the 5% and won't be able to dynamically tell them apart (it's possible to implement the non-generic IEnumerable without implementing IDisposable, and how well that turned out can be guessed at by MS deciding to make IEnumerator<T> inherit from IDisposable!).

Of the rest, maybe there's 3 or 4% of the time it's safe. For now. You don't know which 3% without looking at the code, and even then the contract says you have to call it, so the developer can depend on you doing so if they release a new version where it is important.

In summary, always call Dispose(). (I can think of an exception, but it's frankly too weird to even go into the details of, and it's still safe to call it in that case, just not vital).

On the question of implementing IDisposable yourself, avoid the pattern in that accursed document.

I consider that pattern an anti-pattern. It is a good pattern for implementing both IDisposable.Dispose and a finaliser in a class that holds both managed and unmanaged resources. However holding both managed IDisposable and unmanaged resources is a bad idea in the first place.

Instead:

If you have an unmanaged resource, then don't have any unmanaged resources that implement IDisposable. Now the Dispose(true) and Dispose(false) code paths are the same, so really they can become:

public class HasUnmanaged : IDisposable
{
  IntPtr unmanagedGoo;
  private void CleanUp()
  {
    if(unmanagedGoo != IntPtr.Zero)
    {
      SomeReleasingMethod(unmanagedGoo);
      unmanagedGoo = IntPtr.Zero;
    }
  }
  public void Dispose()
  {
    CleanUp();
    GC.SuppressFinalize(this);
  }
  ~HasUnmanaged()
  {
    CleanUp();
  }
}

If you have managed resources that need to be disposed, then just do that:

public class HasUnmanaged : IDisposable
{
  IDisposable managedGoo;
  public void Dispose()
  {
    if(managedGoo != null)
      managedGoo.Dispose();
  }
}

There, no cryptic "disposing" bool (how can something be called Dispose and take false for something called disposing?) No worrying about finalisers for the 99.99% of the time you won't need them (the second pattern is way more common than the first). All good.

Really need something that has both a managed and an unmanaged resource? No, you don't really, wrap the unmanaged resource in a class of your own that works as a handle to it, and then that handle fits the first pattern above and the main class fits the second.

Only implement the CA10634 pattern when you're forced to because you inherited from a class that did so. Thankfully, most people aren't creating new ones like that any more.

Answer 4

It is never OK to forget to call Dispose (or, as you say, better yet use using).

_{I guess if the goal of your program is to cause unmanaged resource leaks. Then maybe it would be OK.}

Answer 5

The implementation of IDisposable indicates that a class uses un-managed resources. You should always call Dispose() (or use a using block when possible) when you're sure you're done with the class. Otherwise you are unnecessarily keeping un-managed resources allocated.

In other words, never forget to call Dispose().

Answer 6

Yes, always call dispose. Either explicitly or implicitly (via using). Take, for example, the Timer class. If you do not explicitly stop a timer, and do not dispose it, then it will keep firing until the garbage collector gets around to collecting it. This could actually cause crashes or unexpected behavior.

It's always best to make sure Dispose is called as soon as you are done with it.

Answer 7

Microsoft (probably not officially) says it is ok to not call Dispose in some cases.

Stephen Toub from Microsoft writes (about calling Dispose on Task):

In short, as is typically the case in .NET, dispose aggressively if it's easy and correct to do based on the structure of your code. If you start having to do strange gyrations in order to Dispose (or in the case of Tasks, use additional synchronization to ensure it's safe to dispose, since Dispose may only be used once a task has completed), it's likely better to rely on finalization to take care of things. In the end, it's best to measure, measure, measure to see if you actually have a problem before you go out of your way to make the code less sightly in order to implement clean-up functionality.

[bold emphasize is mine]

Another case is base streams

var inner = new FileStrem(...);
var outer = new StreamReader(inner, Encoding.GetEncoding(1252));
...
outer.Dispose();
inner.Dispose(); -- this will trigger a FxCop performance warning about calling Dispose twice.

(I have turned off this rule)