44

I am confused about dispose. I am trying to get my code disposing resources correctly. So I have been setting up my classes as IDisposable (with a Dispose method) them making sure that the Dispose method gets called.

But now FXCop is telling me lots of stuff about disposing = false and calling Dispose(false).

I don't see a Dispose method that takes a bool. Do I need to make one? If so, why? Why not just have a method that gets called when it is disposing?

I saw some code here: CA1063: Implement IDisposable correctly - Microsoft Docs that shows how to make a Dispose method that takes a bool. It says it is for native vs managed resourses. But I thought the whole point of dispose was for unmanaged resourses only.

Also, the line that FXCop is complaining about is this:

~OwnerDrawnPanel()
{
    _font.Dispose();
}

It says:

CA1063 : Microsoft.Design : Modify 'OwnerDrawnPanel.~OwnerDrawnPanel()' so that it calls Dispose(false) and then returns.

But Font does not have a Dispose(bool) on it (that I can find).

To sum it up:

Why do I need a Dispose(bool)? and if I do, why doesn't Font have it? and since it does not have it, why is FXCop asking me to use it?

Thanks for all the great answers. I think I understand now. Here is

The answer as I see it:

Disposing of "unmanaged" resources falls into two categories:

  1. Resources that are wrapped in a managed class (ie Bitmap, Font etc), but still need Dispose to be called to clean them up properly.
  2. Resources that you have allocated, which are representations of native resources (ie device contexts that need to be released)

Dispose(bool) is used to tell the difference between the two:

  1. When Dispose is directly called on your object, you want to free both kinds of "unmanaged" resources.
  2. When your object is up for Garbage Collection, you don't need to worry about the first kind of resources. The garbage collector will take care of them when it cleans them up. You only need to worry about true native resources that you have allocated (if any).
Community
  • 1
  • 1
Vaccano
  • 78,325
  • 149
  • 468
  • 850

10 Answers10

10

Dispose(bool) is a pattern to implement Finalize and Dispose to Clean Up Unmanaged Resources , see this for detail

Community
  • 1
  • 1
Kumar
  • 997
  • 5
  • 8
  • 3
    Good link. The dispose pattern is a must-know for dealing with managed resources. – Ritch Melton Mar 07 '11 at 21:06
  • 2
    @Ritch Melton: I consider it an anti-pattern. A class should deal with either managed resources, one unmanaged resource, or one tightly-coupled group of unmanaged resources. Classes which deal with any managed resources shouldn't handle any unmanaged resources. Since their descendants will deal with managed resources, their descendants shouldn't deal with unmanaged resources either. Why allow for the addition of a finalizer, when descendant classes shouldn't do that? – supercat Mar 07 '11 at 21:59
  • I don't see the issue. If you cache an object that needs to be disposed, then your class should implement dispose so that it can clean up the objects. If the class deals with unmanaged resources then it should implement a finalizer. If it implements a finalizer, you implement the dispose pattern. I don't understand how what you are describing changes that situation. – Ritch Melton Mar 07 '11 at 22:30
10

IDisposable provides a method with the signature 

public void Dispose()

Microsoft best practices (Implement a Dispose method) recommend making a second private method with the signature

private void Dispose(bool)

Your public Dispose method and finalizer should call this private Dispose method to prevent disposing managed resources multiple times.

You can fix the warning you are getting by either implementing IDisposable and disposing of your font object in the dispose method, or creating a Dispose(bool) method in your class, and make your finalizer call that method.

Eliahu Aaron
  • 4,103
  • 5
  • 27
  • 37
Kyle Trauberman
  • 25,414
  • 13
  • 85
  • 121
  • 1
    In the link you provided, when will Dispose(false) be called. I don't see a reference to it. (It seems as if Dispose(true) is the only thing ever called...) Is there some convention that will get Dispose(false) called? – Vaccano Mar 07 '11 at 21:20
  • You should only be passing `false` in your finalizer. Your Dispose method should always pass `true`. The examples in Kumar's link gives a good example of why this is. The finalizer is only ever run by the garbage collecter, and so you only want to dispose of unmanaged resources in that case. If you are calling Dispose yourself, then you want to dispose of managed and unmanaged resources. – Kyle Trauberman Mar 07 '11 at 21:23
  • 1
    So that MSDN example missed adding a method like this: `~DisposableResource{Dispose(false);}` right? – Vaccano Mar 07 '11 at 21:27
  • Ahhhh, I understand now! If my class had some resources that were truly unmanaged then it would need the destructor with the dispose call. Since all the "unmanaged" resources I am using are wrapped in managed classes then they will be cleaned up at GC time anyway. No special clean up needed. – Vaccano Mar 07 '11 at 21:32
  • Yes and no. You should still call Dispose on objects that you're done with. You should be fine with using a Dispose method that calls Dispose(true) in the example you posted in your question. This will give other classes that have an instance of your class the ability to cleanup after themselves. – Kyle Trauberman Mar 07 '11 at 21:36
8

Dispose(bool) is not meant to be public and that is why you don't see it on Font.

In case some user of your class forgets to call Dispose on your method, you will release the unmanaged resources only by making a call to Dispose(false) in the Finalizer.

In case IDispose is called correctly, you call the Dispose on managed resources and also take care of the unmanaged.

The flag is to distinguish the two cases.

It is a pattern recommended by MSDN.

Eliahu Aaron
  • 4,103
  • 5
  • 27
  • 37
  • So, if I am trying to clean up a bunch of `Bitmap` objects owned by my class (by disposing them in my class's Dispose method), should I clean that up either way or only when it is called from Dispose(true)? – Vaccano Mar 07 '11 at 21:15
  • 2
    If it is a managed object which supports IDisposable, then yes, we do it in Dispose(true). You should not worry about whether Bitmap is holding onto unmanaged resources, especially if Bitmap class follows the above pattern. –  Mar 07 '11 at 21:21
  • 2
    Right. The bitmap class has its own finalizer, as far as your class is concerned its just a managed object that needs disposed. – Ritch Melton Mar 07 '11 at 22:49
  • When `Dispose(false)` is running on some object `George`, it's likely that any `IDisposable` objects to which `George` has references have already been cleaned up, are going to be cleaned up without `George`'s help, are still in use, or cannot safely be cleaned up from the finalizer thread. Thus, in most cases `Dispose(false)` isn't going to have anything to do; a class which wouldn't have anything to do in a destructor shouldn't have one. – supercat Aug 30 '13 at 17:35
3

FxCop says you should implement the Disposable pattern like described here. Note that you should not use finalizers for disposing managed resources like _font is. Finalizers are used for cleaning up unmanaged resources. If you do not execute the cleanup logic in the Dispose method of your (sub)class they are executed non-deterministically by the garbage collector.

m0sa
  • 10,712
  • 4
  • 44
  • 91
  • In the second link you provided, when will Dispose(false) be called? I don't see a reference to it. (It seems as if Dispose(true) is the only thing ever called...) Is there some convention that will get Dispose(false) called? If not, then what is the point of having the two method signatures? – Vaccano Mar 07 '11 at 21:23
  • It gets called in the finalizer (C# destructor) of your class or any inheriting class with a finalizer. See the first link :)) There you have an example of Dispose(false) being called. – m0sa Mar 07 '11 at 21:29
  • 3
    @Vaccano: It should never get called with a parameter value of False (if a derived version of the class would need to add an unmanaged resource, that resource should be encapsulated into some other class, which could then be used as a managed resource). Microsoft recognized the problems with combining managed and unmanaged resources when they produced .net 2.0; it seems silly that they keep pushing this Dispose pattern. – supercat Mar 07 '11 at 21:52
2

Also please note that it is very rarely that you need to do anything in the destructor. Regularly, everything is taken care of by the garbage collector. For example, in your code, you don't need to dispose the _font object in the OwnerDrawnPanel's destructor. Since the panel is getting cleaned up by the GC, so will be _font, because panel was the only one who referenced it, right?

Generally, if you own disposable objects, you only need to dispose them when your own Dispose method was called. But NOT in the destructor. When your destructor is running, you can bet that all your aggregated objects are being cleaned up as well.

Fyodor Soikin
  • 78,590
  • 9
  • 125
  • 172
2

You should almost never need to use finalizers. They are only for classes that directly contain unmanaged resources, and in .NET 2.0+ those should be wrapped in SafeHandle.

Mark Sowul
  • 10,244
  • 1
  • 45
  • 51
2

I think Dispose(true) will free both the managed and unmanaged resource as we need to not call finalize again that's why we write GC.SupressFinalize() after Dispose(true).

We call Dispose(false) in destructors to free unmanaged resources and will be called by Runtime and not user's code.

Christian Specht
  • 35,843
  • 15
  • 128
  • 182
Ratnesh Sinha
  • 21
  • 1
1

Agreeing with Kumar, the Dispose(bool disposing) pattern is also documented on MSDN. The distinction is not between managed and unmanaged resources, but whether Dispose is being called by your code or the runtime.

Ben Hoffstein
  • 102,129
  • 8
  • 104
  • 120
  • 1
    Dispose will never be called by the runtime; the only way Dispose will ever be invoked with a parameter value of false will be if either your own code (typically a finalizer/destructor) does so, or if someone else derives from your class and ill-advisedly adds a finalizer/destructor which calls Dispose(false). I am unaware of *any* situation in which a class deriving from anything other than Object should add a destructor/finalizer that calls Dispose(false); I can understand some advantage to having a protected overridable Dispose method distinct from a non-overridable public Dispose, but... – supercat Mar 10 '11 at 17:15
  • 1
    ...I see no use for the Disposing parameter except maybe to ensure that any derived-class finalizers won't break anything. – supercat Mar 10 '11 at 17:18
0

I found a good article about correct implementation of IDispose interface: http://msdn.microsoft.com/en-us/library/ms244737(v=vs.80).aspx

OrahSoft
  • 783
  • 1
  • 5
  • 7
0

The pattern of implementing a public public void Dispose(), protected virtual void Dispose(bool), and ~ClassName() finalizers is a best practice recommended by Microsoft as a way to neatly organize your cleanup code for both managed and unmanaged resources.

Basically, the code that uses your Disposable class should call Dispose(), but if it doesn't, the finalizer ~ClassName() will get called by Garbage Collection, and based on which one of those is used, you set the argument to Dispose(bool) as true or false, and in your Dispose(bool), you only clean up managed resources if the argument is true.

The warning you are getting seems to specifically recommend that you use this practice in your finalize method ~ClassName().

Sam Skuce
  • 1,666
  • 14
  • 20