4

None of the guides/notes/articles that discuss IDisposable pattern suggest that one should set the internal members to null in the Dispose(bool) method (especially if they are memory hogging beasts).

I've come to realize the importance of it while debugging an internal benchmark tool. What used to happen was that, there was this buffer that contained a big array inside it. We used to use a static buffer for the whole benchmark program. Once we're done with the buffer, there was no way we could release this internal array, neither could we make this buffer releasable (as it was static).

So, I believe that, after Dispose() is called, the class should do everything it can so that it releases all the resources it is using and make them available again, even if the object being disposed itself is not collected back by GC, and not setting members to null, thereby, not allowing the internal objects to be collected by the GC implies that the Dispose implementation is not perfect.

What's your opinion on this ?

Scott Dorman
  • 42,236
  • 12
  • 79
  • 110
mherle
  • 504
  • 3
  • 12

3 Answers3

6

Releasing any additional references during Dispose is certainly something I try to do, for two reasons:

  • it allows the inner objects to be garbage collected even if the disposed object is still in scope
  • if the inner objects are disposable, it means we only dispose them once even if Dispose() is called repeatedly on the outer object

For example, I tend to use things like:

if(someDisposableObject != null)
{
    someDisposableObject.Dispose();
    someDisposableObject = null;
}
(for non-disposable, just set to null)
someNonDisposableObject = null; // etc

You might also want to set any events to null:

someEventHandler = null;

This can help minimise the impact if the caller can't fully release their reference (or simply forgets) at the moment. While you should try to release the outer object (for GC), it is relatively easy to accidentally extend the life of the object, for example via a captured variable (anonymous method/lambda), an event, etc.

If you have a finalizer, then during the GC process there is no benefit doing this, and you shouldn't really call methods on external objects (even Dispose()) - so in short: don't do any of this during a GC sweep.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • Marc, you must *always* set events to null!!! This can cause a memory leak in some instances. Upvote: great answer. – Jonathan C Dickinson Jan 19 '09 at 06:06
  • That "some instances" probably relates to the infamous "static events" issue which crashed a robot. As long as the object is eventually eligible for garbage collection, events do not normally directly cause memory leaks. However, static events are never collected - try to avoid them if possible ;-p – Marc Gravell Jan 19 '09 at 06:12
  • And of course, static events don't tie into IDisposable (which is an instance concept). So while I agree with setting events to null, it isn't entirely true to state that it can cause memory leaks... better to release early, though. – Marc Gravell Jan 19 '09 at 06:14
  • Could you clarify what you mean by your last sentence ? Is it just that "don't touch the managed objects" in Dispose(false) case (which gets called from the finalizer) ? – mherle Jan 19 '09 at 12:06
  • 1
    Indeed, don't touch other managed object in a finalizer, as you don't know if they have already been collected. And there is no point clearing fields since your object is about to be obliterated anyway. – Marc Gravell Jan 19 '09 at 13:13
  • @Marc Gravell: Static events do tie into IDisposable; if an object subscribes to static events, its IDisposable.Dispose should clean up those subscriptions. – supercat Mar 11 '11 at 17:48
  • @supercat Please explain how Dispose() can be called in a static context (without an instance reference). – Kevin P. Rice Jul 22 '11 at 02:51
  • @Kevin I believe he is saying that the *event* is static; the *object(s)* is(/are) instance-based and implement `IDisposable`. When the objects are `Dispose()`d they could unsubscribe from the static event. I might not do it that way personally, but the reasoning isn't terrible. – Marc Gravell Jul 22 '11 at 06:17
  • @Marc Gravell: If an object needs to subscribe from a static event but does not unsubscribe in Dispose, how should the object ever become unsubscribed? To my mind, a fundamental part of the "object" contract requires that--to the extent possible--if an object does not implement IDisposable, it should be safe to abandon it without cleanup; if it does implement IDisposable, it should be safe to call Dispose and abandon it with no other cleanup. If you don't like unsubscribing in IDisposable, what do you like? – supercat Jul 22 '11 at 15:04
  • @supercat I think you are mixing my comment and Kevin's. Indeed IDisposable could be used to unsubscribe from a longer-life event. But static would still be unwise here IMO. – Marc Gravell Jul 22 '11 at 16:33
  • @Marc Gravell: I was responding to "I might not do it that way personally", which I read as referring to using IDisposable to unsubscribe static events, since you'd earlier said "Static events don't tie into IDisposable". You say "IDisposable *could* be used to unsubscribe". I would say it *should* be used; the only situations I can think of when it wouldn't be by far the best way of handling unsubscriptions would be those where, for whatever reason, IDisposable wasn't usable. Can you think of others? – supercat Jul 22 '11 at 17:23
1

Maybe I'm missing your point, but once your object is disposed, the root or 'sub-root' it represented relative to it's members has been detached. It seems like you are thinking of garbage collection like a ref count system (which can be done, but ... usually isn't).

Instead, think of it as a many-rooted tree, where every object has branches for what it links to. At the end of the day the 'final roots' are statics and anything instantiated from a 'main' loop.

When the garbage collector runs, the easiest way to think about what it does is to consider that it will walk the list of 'real roots' and apply a 'color' to everything it can 'reach'.

Now, assumed the collector has access to 'everything', whether it was rooted or not. Anything not colored can be cleaned up.

Getting back to your original question, when your object is disposed, one assumes (or at least hopes) that no one references it anymore. If this is the case, it is no longer rooted, and so it will not contribute to 'coloring' anything it touches.

Long story longer - if nulling out members in a Dispose routine is fixing something - I would be you have a different, and real, problem that someone is holding a link to your disposed object, and keeping it 'reachable' when it should not be.

I apologize for what may be the most over-quote-filled message I've ever written, but I'm sort of abusing standard terms.

Joe
  • 2,946
  • 18
  • 17
  • I think it may be an effort to minimize the amount of sweeps required to free those objects. Remember, the GC runs according to the process: free colored, color objects. – Jonathan C Dickinson Jan 19 '09 at 06:07
  • @Joe You wrote: "when your object is disposed, one assumes (or at least hopes) that no one references it anymore" Disposal is no assurance that an object is not still held. Perhaps a collection of objects is disposed after use to release resources but still held in the collection for a later process to remove them? Wouldn't it simply be good defensive programming to release heavy resources with a `resource = null` in Dispose()? You can't know what coding errors or bad practice might hold on to an object. Thus, `resource = null` seems good practice for heavy resources, no? – Kevin P. Rice Jul 20 '11 at 12:34
0

Well, generally, it's not going to make a difference. The only place where it will make a difference is when you have a reference to an object on the Large Object Heap, the behavior of which you have seen already).

There is a good article on the LOH which goes into this in more detail here:

http://msdn.microsoft.com/en-us/magazine/cc534993.aspx

casperOne
  • 73,706
  • 19
  • 184
  • 253