views:

332

answers:

8

I'm new to C#, so apologies if this is an obvious question.

In the MSDN Dispose example, the Dispose method they define is non-virtual. Why is that? It seems odd to me - I'd expect that a child class of an IDisposable that had its own non-managed resources would just override Dispose and call base.Dispose() at the bottom of their own method.

Thanks!

+2  A: 

Although methods in an interface are not "virtual" in the usual sense, they can nevertheless still be implemented in classes that inherit them. This is apparently a convenience built into the C# language, allowing the creation of interface methods without requiring the virtual keyword, and implementing methods without requiring the override keyword.

Consequently, although the IDisposable interface contains a Dispose() method, it does not have the virtual keyword in front of it, nor do you have to use the override keyword in the inheriting class to implement it.

The usual Dispose pattern is to implement Dispose in your own class, and then call Dispose in the base class so that it can release the resources it owns, and so on.

A type's Dispose method should release all the resources that it owns. It should also release all resources owned by its base types by calling its parent type's Dispose method. The parent type's Dispose method should release all resources that it owns and in turn call its parent type's Dispose method, propagating this pattern through the hierarchy of base types.

http://msdn.microsoft.com/en-us/library/fs2xkftw.aspx

Robert Harvey
@Robert Harvey - What document does this cite?
Joel Etherton
+1 It boils down to the fact that you don't want to rely on children to clean things up when they might not know how to best clean up the resources of the parent.
Justin Niessner
@Joel: The one the OP linked to.
Robert Harvey
It would mean it should be virtual, no?
Grzenio
@Grzenio Nope, if a child class overrides Dispose, the base class' Dispose method would no longer be accessible and thus the parent's resources would never get freed.
Mark LeMoine
A: 

If the base class has resources that need to be cleaned up at Dispose() time, then having a virtual Dispose method that's overridden by an inheriting class prevents those resources from being released unless the inheriting class specifically calls the base's Dispose method. A better way would implement it would be to have each derived class implement IDisposable.

SwDevMan81
Well...if you're overriding virtual methods and not calling the base implementation, you're already in for a world of hurt.
Kirk Woll
That is true, which is why I suggested implementing `IDisposable` for each derived class. Making `Dispose` virtual is possible, but the derived classes must call the base class `Dispose` method
SwDevMan81
+2  A: 

The reason the sample's Dispose() method is non-virtual is because they take over the entire process in that example, and leave subclasses with the virtual Dispose(bool disposing) method to override. You'll notice that in the example, it stores a boolean field to ensure that the Dispose logic does not get invoked twice (potentially once from IDisposable, and once from the destructor). Subclasses who override the provided virtual method do not have to worry about this nuance. This is why the main Dispose method in the example is non-virtual.

Kirk Woll
Example would be more obvious if it also had a finalizer calling Dispose(bool).
Joel Rondeau
+3  A: 

Typical usage is that Dispose() is overloaded, with a public, non-virtual Dispose() method, and a virtual, protected Dispose(bool). The public Dispose() method calls Dispose(true), and subclasses can use this protected virtual method to free up their own resorces, and call base.Dispose(true) for parent classes.

If the class owning the public Dispose() method also implements a finalizer, then the finalizer calls Dispose(false), indicating that the protected Dispose(bool) method was called during garbage collection.

If there is a finalizer, then the public Dispose() method is also responsible for calling GC.SuppressFinalize() to make sure that the finalizer is no longer active, and will never be called. This allows the garbage collector to treat the class normally. Classes with active finalizers generally get collected only as a last resort, after gen0, gen1, and gen2 cleanup.

Cylon Cat
Your description is very precise. One remark though, the `public Dispose()` method should __always__ call `GC.SuppressFinalize()` when that declaring type is __not__ sealed.
Steven
@Steven, I'm not sure what "sealed" has to do with things, but as a broad pattern, any class that implements a finalizer also needs to implement IDisposable, so the finalizer and Dispose() implementation will be in the same class. Classes that inherit from classes implementing Dispose() should use the virtual protected Dispose(bool). If for some reason a class subclasses a Disposable class, and needs its own finalizer, then I would say that it's time to re-think the design.
Cylon Cat
@Cylon: [Part 1/3] I agree that any class with a finalizer should have a `Dispose` method. However, this doesn’t mean that a finalizer and `Dispose` should be together in the same class. This does not hold, because you not always design both classes (think about framework designers). Look for instance at `System.IO.Stream`. It implements `IDisposable`, but doesn’t have a finalizer. `FileStream` however, actually does implement a finalizer. Stream calls `SuppressFinalize`, even while it doesn’t implement a finalizer itself.
Steven
@Cylon: [Part 2/3] When `Stream` didn’t, `FileStream` had to call `SuppressFinalize` in it’s `Dispose(bool)` method, which isn’t that bad, but this doesn’t follow the ‘dispose pattern’. What would be worse is implementing an empty finalizer on the `Stream` class, because this would increase the change of objects keeping on the heap when developers forget to dispose their objects.Talking about bad design: `System.ComponentModel.Component` actually has an empty finalizer, and this causes all sorts of trouble when instances are not disposed properly (I’ve seen OOM being thrown because of this).
Steven
@Cylon: [Part 3/3] Last note, I noted “sealed”, because only when your class is sealed, you can be sure that nobody inherits from your class and thus add a finalizer. I hope this clears things up.
Steven
@Steven, I see your point about the Stream hierarchy. For the subclass, you'd want to suppress the finalizer only when Dispose(true); if it's Dispose(false), then you're already in the garbage collector and it's too late. As for discussion of when to use "sealed" or not, that's a separate discussion, but I wouldn't make a class sealed just to prevent a finalizer from being attached in a subclass.
Cylon Cat
@Cylon: I agree that you shouldn't seal a class just to not have to call `SuppressFinalize`. I think it is the other way around: You designed a class and made it sealed for a special reason (security for instance) and because it is sealed you don't have to call `SuppressFinalize`.
Steven
@Steven: Worth noting that, among the framework designers, sealing classes is the rule rather than the exception, as it can be difficult to evalue ahead of time all the different ways a user might make use of (and possibly break) your class in a derived class. Ergo, you don't need a special reason to seal a class. See http://blogs.msdn.com/b/ericlippert/archive/2004/01/22/61803.aspx
Robert Harvey
@Robert: This is very interesting. Also notice that Lippert's advice contradicts that of the Framework Design Guidelines who explicitly state: "DO NOT seal classes without having a good reason to do so." [2nd edition, page 209].
Steven
@Steven, that may depend on what the definition of "a good reason" is. For Microsoft, and considering the .NET framework, that might be a good reason to seal classes. On the other hand, I'm building a framework for use in the application I work on. If someone breaks a class, I can go talk to them. Microsoft doesn't have that option.
Cylon Cat
Steven
Steven
+1  A: 

I've got a quite detailed explanation of the dispose pattern here. Essentially, you provide a protected method to override that is more robust for unmanaged resources instead.

thecoop
+2  A: 

This is certainly not an obvious one. This pattern was especially choosen because it works well in the following scenario's:

  • Classes that don't have a finalizer.
  • Classes that do have a finalizer.
  • Classes that can be inheritted from.

While a virtual Dispose() method will work in the scenario where classes don't need finalization, it doesn't work well in the scenario were you do need finalization, because those types often need two types of clean-up. Namely: managed cleanup and unmanaged cleanup. For this reason the Dispose(bool) method was introduced in the pattern. It prevents duplication of cleanup code (this point is missing from the other answers), because the Dispose() method will normally cleanup both managed and unmanaged resources, while the finalizer can only cleanup unmanaged resources.

Steven
A: 

Calls through an interface are always virtual, regardless of whether a "normal" call would be direct or virtual. If the method that actually does the work of disposing isn't virtual except when called via the interface, then any time the class wants to dispose itself it will have to make sure to cast its self-reference to iDisposable and call that.

In the template code, the non-virtual Dispose function is expected to always be the same in the parent and the child [simply calling Dispose(True)], so there's never any need to override it. All the work is done in the virtual Dispose(Boolean).

Frankly, I think using the Dispose pattern is silly in cases where there's no reason to expect descendant classes to directly hold unmanaged resources. In the early days of .net it was often necessary for classes to directly hold unmanaged resources, but today in most situations I see zero loss from simply implementing Dispose() directly. If a future descendant class needs to use unmanaged resources, it can and typically should wrap those resources in their own Finalizable objects.

supercat
A: 

The Dispose method should not be virtual because it's not an extension point for the pattern to implement disposable. That means that the base disposable class in a hierarchy will create the top-level policy (the algorithm) for dispose and will delegate the details to the other method (Dispose(bool)). This top-level policy is stable and should not be overridden by child classes. If you allow child classes to override it, they might not call all the necessary pieces of the algorithm, which might leave the object in an inconsistent state.

This is akin to the template method pattern, in which a high-level method implements an algorithm skeleton and delegates the details to other overridable methods.

As a side note, I prefer another high-level policy for this particular pattern (which still uses a non-virtual Dispose).

Jordão