Should GC.SuppressFinalize be called on objects that do not have a finalizer?

Question

For some reason FXCop seems to think I should be calling GC.SuppressFinalize in Dispose, regardless of whether I have a finalizer or not.

Am I missing something? Is there a reason to call GC.SuppressFinalize on objects that have no finalizer defined?

Ceilidh

Answer 1

There is always a finalizer in IL - System.Object.Finalize() exists in every class, so if you make a custom class, it has a finalizer you want to suppress. That being said, not all objects are put on the finalization queue, so you only techncially should need to suppress finalization if you implement your own finalizer.

If you're implementing IDisposable to wrap unmanaged resources, you should include a finalizer, and you should prevent this from running, since in theory you're doing the cleanup already when Dispose is called.

Bade

True. Also, you should have a finalizer that calls Dispose(). – Krissykrista

True- Object.Finalize is a null op., but if you're implementing IDisposable, you're saying you have resources to free up. From MSDN, that means you should always have a finalizer to free them, so they're handled correctly. This means having Finalize call Dispose, and Dispose suppress finalization – Bade

@Krissykrista You should only have a finalizer if you directly own unmanaged resources (such as handles). If you own them only indirectly, e.g. a reference to a FileStream, why should you write anything more than IDisposable.Dispose() { if (fs != null) { fs.Dispose(); fs = null; } }? IMO, StyleCop is an awful tool that forces a LOT of useless text to be written and maintained, and adds no business value in return. – Gustative

Answer 2

There's no need to call GC.SuppressFinalize(this) in Dispose, unless:

• You are the base class that implements virtual Dispose methods intended for overriding (again, it might not be your responsibility even here, but you might want to do it in that case)
• You have a finalizer yourself. Technically, every class in .NET has a finalizer, but if the only finalizer present is the one in Object, then the object is not considered to need finalizing and isn't put on the finalization list upon GC

I would say, assuming you don't have any of the above cases, that you can safely ignore that message.

Sasha

When should a derived class EVER add a finalizer to a non-trivial base class? Why add code to allow a derived class to do something it should never do? – Edmonds

@Edmonds If a derived class owns any unmanaged resources, it should have a finalizer to ensure they are freed. If the object is always used correctly (with a try-finally or, equivalently, a using statement) Dispose will do the cleanup and suppress the finalizer, but the finalizer ensures the unmanaged resources are freed eventually (when the object is garbage collected), even if Dispose was never called (e.g. due to an exception and failure to protect a code block). – Gustative

@TheDag: Each unmanaged resource that needs finalizer cleanup should almost always be encapsulated within its own object which should either derive from Object, or an abstract base type explicitly designed to assist such cleanup. The resulting type would then be a managed resource, to which a reference could be held by the larger type. Finalization cleanup would be taken care of by the smaller encapsulating object; the larger object wouldn't need a finalizer. – Edmonds

@Edmonds I agree. But such a class can still derive from something (that is itself perhaps hanging on to handles). I can't recall having ever needed one, but I almost never deal with any unmanaged resources. Btw, this makes me think of how it irritates me that we're told to "implement IDisposable correctly" (meaning with a protected Dispose(bool disposing)) by CA, even when we're only embedding some already-disposable type such as a StreamWriter. I usually simply create the embedded thing in the constructor and basically re-publish its Dispose method. Anything wrong with that? – Gustative

@TheDag: While might have been better to define a parameterless protected void VirtDispose() method and have IDisposable.Dispose chain to that, using the name rather than signature to distinguish the protected virtual method from the public non-virtual one, using what's effectively a dummy parameter to distinguish the methods is hardly the worst thing in the world. There's some value in having all IDisposable expose the same virtual patch-point regardless of whether they implement Dispose implicitly or explicitly, so even though the pattern isn't ideal, following it... – Edmonds

...seems like a reasonable idea; my biggest annoyance with the pattern relates to a beef I have with IDisposable in general, which is that there's no guarantee of thread safety, and with the pattern as implemented the only way to have thread-safe guarding against multiple disposal is to have every subtype add its own DisposeStarted flag. Disposal and event unsubscription are both things where thread-safety can be added easily within an implementation, and can sometimes not be done thread-safely by a client (e.g. if one thread discovers that an object or event will no longer be needed... – Edmonds

...the only way it would have to notify the thread which manages the event or object would be to request Dispose or unsubscribe; if the resource can't be cleaned up immediately, the implementation could ask the managing thread to clean it up at the first practical opportunity. – Edmonds

Answer 3

It looks like FxCop simply inspects the Dispose() and doesn't check for the presence of a destructor.

It should be safe to ignore.

Marmolada

Answer 4

All objects have a finalizer method, even if you have not implemented one by using a c# destructor (which is not actually guaranteed to be called by the GC). It's just good practice to supress the call if you have implemented IDisposable because that means you have decided to perform the finalization explictly.

devx article

Malek

During program termination, some objects might not get a chance to run their finalizers if cleanup takes too long. That could be what he's referring to. – Sasha

Yes, that's what I was referring to. – Malek

Answer 5

I don't see any need to call SuppressFinalize() if there's no finalizer defined. If you want to be defensive then it can be good to have a finalizer as well as Dispose(), so you don't need to rely on clients to always call Dispose(). Then you won't leak resources when they forget.

Sacristy