Is it harmless to call GC.SuppressFinalize within the finalizer?

Question

Because the finalizer/IDisposable and so-called "IDisposable pattern" topic tends to bring out lots of posturing, pontificating, and militant opinion (not-respectively, here, here, here, and more), I really hesitate to ask this. Hoping to preempt those well-worn debates, I'm keeping to a very simple question which doesn't appear to have a concise answer on StackOverflow...

Is calling GC.SuppressFinalize(this) vacuous once the object's finalizer has begun executing? More specifically or usefully (of course), is it harmless to call GC.SuppressFinalize(this) from within the finalizer itself? (Again, we're not debating any "why" here)

So in other words, beyond the overhead for calling the API and its duly setting a flag in the object header, are there any bad, unwanted, or otherwise tangible correctness or performance effects?

Answer 1

Of course, it would be much better to avoid the finalizer altogether, and use SafeHandle, as the modern idiom dictates. Then all of this stuff about finalizers becomes completely moot.

That said, the wisdom of doing so notwithstanding, it is perfectly safe to call GC.SuppressFinalize() from the finalizer. The documentation for the method describes what the method does:

This method sets a bit in the object header of obj, which the runtime checks when calling finalizers.

The runtime may actually check this bit during a GC operation as well, which is when on finding an object unreachable, that object's finalizer would be put into the finalizer queue. If it's set at that point, finalizer doesn't even wind up in the queue.

Checking it again later, before calling the finalizer itself, also allows finalization of the object to be avoided, if it turns out that some other object's finalizer wound up disposing it even though the that object's finalizer was put in the finalization queue.

Both of these checks occur before the finalizer is called. Once the finalizer is called, the bit in the object has no purpose. Setting it is harmless, but won't accomplish anything.

As an aside: note that past implementations of .NET used Finalizer and FReachable queues. When an object was created, if it had a finalizer, it would be moved to the Finalizer queue. Once the object was unreachable, it would be moved to the FReachable queue for later finalization. Calling SuppressFinalize() would remove the object from the Finalizer queue. By the time the finalizer runs, the object is no longer in this queue, so the SuppressFinalize() call would be a NOP, similarly harmless.

Now, that said, your question is broad: "…are there any bad, unwanted, or otherwise tangible correctness or performance effects?". Much of that is in the eye of the beholder. I would argue that a finalizer that calls GC.SuppressFinalize() is incorrect. So, that would be a "tangible correctness effect" to me. I also find code that deviates from published, acknowledged standard patterns to be "unwanted". Without more specific criteria in the question to constrain it, the answer to that part of the question could be any of "yes", "no", "sometimes", etc.

There is in fact a duplicate question to yours, but no one's deigned to answer it: Calling GC.SuppressFinalize() from within a finalizer. I do find the thread of comments on point though, especially Eric Lippert's contributions:

Your supposition is that the unnecessary call to SuppressFinalize is the error in your plan. That's not the problem; the problem is the disposal of managed resources on the finalizer thread. Recall to your mind that finalizers run on their own thread, and that managed resources can be thread-affinitized, and now start to imagine the horrors that could result. Moreover: finalizers run in arbitrary order. A managed object disposed on the finalizer thread might have already been finalized; now you are possibly running finalization logic twice on one object; is it robust to that scenario? – Eric Lippert Mar 31 '16 at 21:58 1

Writing a correct finalizer is extraordinarily difficult and I recommend against you trying ever, ideally, but definitely hold off until you understand the pattern better. If you are not sufficiently scared yet, my series of articles on the subject might put more fear into you: ericlippert.com/2015/05/18/… – Eric Lippert Mar 31 '16 at 21:59

…

@Tom: The question is "I'm using the dispose pattern completely wrong; is this particular part of what I'm doing wrong?" No, the whole thing is wrong from the very first sentence. You don't use Dispose to dispose managed resources, and you certainly don't use a finalizer for that. That's the problem here. Is there anything wrong, per se, with calling SuppressFinalize from a finalizer? Well, it will work, but there should not be a situation in which that is the correct thing to do, so whether it works or not should be irrelevant. – Eric Lippert Jul 7 at 14:17

@Tom: Also, why do you call SuppressFinalize in the first place? Only because it is a performance optimization. But under what circumstances is it an optimization when called from the finalizer thread? Only when you've failed to make that optimization from the main thread! That's the place to do that optimization! – Eric Lippert Jul 7 at 14:24

IMHO, these comments bring the primary issue to a fine point: asking whether it's safe to call SuppressFinalize() from the finalizer is the wrong question. If you've gotten as far as having to ask that question, the code is already wrong, and the answer to the question is probably not all that relevant. The right approach is to fix the code so you don't have to ask that question.

Finally, while not exactly the same issue, I think it's also worth pointing out that the usual guidance to call SuppressFinalize() at the end of the Dispose() method is probably incorrect. If called, it should be called at the beginning of the Dispose() method. See Be Careful Where You Put GC.SuppressFinalize

Answer 2

In short, it's rare to use a Finalizer, you should be cleaning up your application deterministically. Also there are race conditions are other reasons why finalization in .net is problematic {insert huge blog posts and a long lists of recommendations here}.

_{When you do want a finalizer, you want it in addition to a Dispose, not instead of Dispose.}

In most use cases, if you were to use a finalizer, you would call the SuppressFinalize in your Dispose method, but to answer the question (and others)

Is calling GC.SuppressFinalize(this) vacuous once the object's finalizer has begun executing?

The call to SuppressFinalize is actually quite trivial, it just sets a flag in the object to prevent it being added to the finalize queue. It has little overhead in and of itself, and can be set in any code path. This will likely not be your problem.

However, the real problem here is preventing the GC from finalizing and your object disposing at the same time, and assumptions about the state of the system around you during their execution.

To try and mitigate this, you would call GC.SuppressFinalize(this) as soon as you can, use a flag to determine if you have disposed. or even check if the appdomain or the environment has started to unload or shut down

if (!Environment.HasShutdownStarted && !AppDomain.CurrentDomain.IsFinalizingForUnload())

Either way there is still a race condition and problems here, and mitigating or dealing with them will depend on exactly why you need the finalizer and what you are trying to clean up and how.