1

Now I understand that when I have finished with a resource that implements IDisposable, I should call the Dispose() method to clean up the resources.

To what extent should I be doing this.

My example is:

I am creating a NotifyIcon in code, so I do something like this:

var icon = new Icon(...);
var contextMenu = new ContextMenu(...);
var contextMenuButton1 = new MenuItem(...);
var contextMenuButton2 = new MenuItem(...);
var contextMenuButton3 = new MenuItem(...);
// ...
var notifyIcon = new NotifyIcon(...);

All of these have a Dispose() method. Normally I would only keep reference to the notifyIcon and just dispose that.

However, it won't dispose of the Icon or the Context Menu or its items, so should I actually be keeping a reference to everything, or at least have a List<IDisposable>?

OR can I assume because of the scope of these that the garbage collector will call dispose on these for me (when I dispose of the NotifyIcon)?

Larry
  • 17,605
  • 9
  • 77
  • 106
Cheetah
  • 13,785
  • 31
  • 106
  • 190
  • You have to only dispose NotifyIcon, ideally NotifyIcon should dispose its children in it's implementation. However if in doubt, you can use source code tool like Refractor or DotPeek to see if it does. Otherwise prepare a list of disposable items and call dispose on each item at the end. – Akash Kava Dec 04 '13 at 12:39
  • Detailed information about this: http://stackoverflow.com/questions/898828/c-sharp-finalize-dispose-pattern – Larry Dec 04 '13 at 13:02

4 Answers4

2

This depends a lot on the scenario. For example, in a lot of UI frameworks, adding something like a menu-item or icon to a parent control / container makes the parent assume responsibility for that item. Thus disposing the parent will dispose all the ancestors at the same time. So: if we assume that these new controls you are creating will be added to the UI, then no: you don't usually need to do anything manually. However, if you are frequently adding/removing controls, then you will need to figure out when you take ownership of an element (i.e. if you remove it from the UI, it is now "yours", not the UI's - and it is your responsibility to dispose it).

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • One major deficiency in Microsoft's documentation is that it does not consistently document ownership. I have not been able discern any particular pattern or logic to which object properties imply ownership and which ones don't. The `Font` property of `Control` is especially bizarre in that regard; I've seen nothing officially documenting its behavior, so I don't know what is or is not guaranteed about it). – supercat Dec 04 '13 at 19:13
0

Generally, that's what the using statement is for, eg.:

using (var icon = new Icon(...))
{
}

The using statement calls the Dispose method as soon as it ends (and even if there's an exception inside, it will still properly dispose of the variable). You can even "chain" them to reduce nesting:

using (var icon = new Icon(""))
using (var icon2 = new Icon(""))
using (var icon3 = new Icon(""))
{

}

You want to call Dispose as soon as you can. If you don't, it will still be called at some point, usually during finalization of the object, but that might already be too late (for example, disposing of a file handle will allow another file handle to be opened, while if you don't dispose of it soon enough, the second call will throw an exception).

The reason there's a Dispose method in the first place is because Icon has some native resources in the background (it uses a native Icon object, this is all related to Windows and GDI+). These resources are "scarce" (a better example is the file handle), and as such you don't want to wait for the GC to collect them eventually - that's more costly (finalization has a non-trivial cost if done extensively) and can block access to the resource (even with the Icon, I remember an issue where you could only allocate so many native bitmaps per process, then you're starved).

If you don't care about the cost, and if the resource isn't scarce enough to matter, and if the using or Dispose is more trouble than it's worth - you don't have to call Dispose. It will be called eventually.

As examples of where you should pretty much always call dispose manually: Files, Streams, Sockets, large blocks of native memory.

Luaan
  • 62,244
  • 7
  • 97
  • 116
  • Unmanaged resources need not be "native resources", nor do they need to be scarce. An object acquires an unmanaged resource when it asks an outside entity to do something on its behalf--to the detriment of other entities--until further notice. If an implementation of `IEnumerator` acquires a lock when it is constructed, and releases it on `Dispose`, that lock would be an unmanaged resource, even though it's handled entirely within the managed framework. Further, not only would many such `IEnumerator` implementations not implement finalizers, but there's little they could do even if... – supercat Dec 04 '13 at 20:49
  • ...they did; unless the `IEnumerator` was constructed on the finalizer thread (very unlikely), there would be no way for a finalizer to release its lock. To be sure, an enumerator generally shouldn't acquire and hold a lock throughout its enumeration, and enumerators which do hold lucks must be used with care to avoid deadlock, but having an enumeration hold a lock is sometimes less evil than any practical alternative approach. – supercat Dec 04 '13 at 20:53
0

From MSDN: `

When a new NotifyIcon is created, ... [it] will exist until its container releases it to garbage collection.

If you use the constructor that takes a container (typically a System.ComponentModel.Container) owned by a Form), then Dispose will be called when the container is disposed - i.e. when the Form is closed.

The examples in the MSDN documentation linked above use a Form's container in this way.

If you use the default constructor, there is no container, and you will have to manage the lifetime of the NotifyIcon yourself, and dispose it when you're finished.

Joe
  • 122,218
  • 32
  • 205
  • 338
-2

Garbage collector would eventually go and call Dispose for all the objects (assuming they are implemented according to IDisposable guidelines) but as a best practice you should be doing that always yourself. It is just a best practice and it will also enable you to detect potentially problematic areas (for example, a large Image being held in memory for too long - since you will dispose it manually, your application will throw an exception if it tries to use it where you did not intend it).

using (var icon = new Icon(...))
using (var contextMenu = new ContextMenu(...))
using (var contextMenuButton1 = new MenuItem(...))
using (var contextMenuButton2 = new MenuItem(...))
using (var contextMenuButton3 = new MenuItem(...))
{
    // ...
}

this.notifyIcon = new NotifyIcon(...);

Since you are storing (I assume that in this code sample) the this.notifyIcon value, your class also has to implement IDisposable and call this.notifyIcon.Dispose() there.

Edit: For those comments that say the GC will not call Dispose() - the standard pattern for implementing IDisposable is to implement a destructor that calls this.Dispose() - since GC does invoke the destructor/finalizer it will indirectly also invoke Dispose(). http://msdn.microsoft.com/en-us/library/ms244737.aspx

Knaģis
  • 20,827
  • 7
  • 66
  • 80
  • 2
    The GC never calls `Dispose()`, it will only call the finalizer on objects that declare one (and haven't had finalization suppressed) once they are no longer reachable. – Iridium Dec 04 '13 at 12:44
  • 1
    Please fix : the GC never calls Dispose(). – Larry Dec 04 '13 at 12:45
  • It does call Dispose (indirectly) if the class is implemented correctly. Because the standard pattern for implementing is to add the destructor `~ClassName() { this.Dispose(false); }` – Knaģis Dec 04 '13 at 14:04
  • 1
    @Knaģis: When Microsoft suggested its `Dispose` pattern, it was thought that classes would encapsulate a mixture of managed and unmanaged resources. It has since become clear that a better pattern is to encapsulate individual unmanaged resources into privately-held objects which are coded to act as managed resources. Since the wrapper objects are privately held, they need not be public, much less publicly inheritable, and thus need not follow Microsoft's original `Dispose` pattern. – supercat Dec 04 '13 at 19:00
  • @Knaģis: Further, if a short-lived object subscribes to events from long-lived objects, then one must either accept that failure to dispose the short-lived object will extend its lifetime to that of the long-lived object (creating a memory leak if an unbounded number of short-lived objects can be created within the lifetime of the longer-lived one) or impose a lot of extra complexity and overhead to prevent that from happening. – supercat Dec 04 '13 at 19:02
  • The pattern you describe may be the *suggested* implementation, but it it's by no means the only one, nor is it necessarily incorrect to implement it differently. Furthermore, the difference between calling `Dispose`, and leaving the object to be finalized may be significant, so stating that the GC calls `Dispose()` is not just wrong, but also misleading since it implies identical net result, which is decidedly not the case. – Iridium Dec 04 '13 at 19:05