Reasoning for the docs recommendation of declaring Dispose(bool)

[jnm2]

The docs currently recommend that a Dispose(bool) method should always be declared in a non-sealed class, even if it has no finalizer. They state very clearly what the pattern is without stating why. I'm giving input to community analyzers and more generally making recommendations in scenarios where there is the conflicting opinion that Dispose(bool) is a leftover from the time of finalizers and should not be declared. I would like to gain the reasoning needed in order to represent the documented position robustly.

Here's my question: Why isn't it better for a non-sealed base class to declare only public virtual void Dispose() than to declare public void Dispose() + protected virtual void Dispose(bool)?

If the base class has only virtual void Dispose(), it seems that derived classes of any depth can correctly implement disposal with or without a finalizer of their own. The derived class can know for sure that the base class's Dispose() is implicitly Dispose(isDisposing: true) and simply make no call to the base class for Dispose(false).

Here's the only problem I was able to think of so far. Let's say we get the idea that we want to change the base class by introducing a finalizer, perhaps after shipping the class as public API. Adding a finalizer means adding a Dispose(bool) method that the derived class would have to call. It's too late for that, so we're stuck. We can't add the finalizer to the base class.
But isn't this a good thing? It seems there is always a better option than allowing the base class to add a finalizer, so this door doesn't need to be left open by planting Dispose(bool) ahead of time. The better option would be for the base class to encapsulate the finalizer in a new single-purpose class. The new class would probably be based on SafeHandle, but it wouldn't have to be. Also, the new class can be internal even if the base class is public.

I'm not aware of all the factors yet, but if the only thing you lose is the ability for the base class to gain a finalizer later on, wouldn't you expect the docs to recommend losing this ability? I thought for sure I'd see the docs recommending virtual void Dispose() rather than virtual void Dispose(bool). Is there perhaps room for the docs to move away from unconditionally recommending Dispose(bool) for all non-sealed classes?

[tannergooding]

CC. @stephentoub, @bartonjs

[bartonjs]

The biggest reason is consistency... so that there's not one pattern for types that were declared in 2020 and another for older types.

We had a big discussion about this last winter and that was our basic conclusion: we've had a pattern for 15+ years, and it's too disruptive to change it (and the pattern isn't very expensive).

In addition to having a place where the finalizer could be inserted (though the new Framework Design Guidelines says not to make public types finalizable (but remember: not all types derived from public types are public)), it makes it much clearer when a type /does/ introduce a finalizer where the SuppressFinalize goes: the place that originally declared it was IDisposable, in the non-virtual Dispose method.

[svick]

The biggest reason is consistency... so that there's not one pattern for types that were declared in 2020 and another for older types.

There are many inconsistencies between new and old types, because of new language features, APIs and practices. I don't see why this should be any different.

We've had a pattern for 15+ years, and it's too disruptive to change it

I don't understand, how is it disruptive to change what you suggest people should do? Especially considering that the new suggestion would basically be what someone new would do naturally.

the pattern isn't very expensive

In terms of extra lines of code and concepts to learn, I'd say it's quite a bit more expensive than it needs to be.

it makes it much clearer when a type /does/ introduce a finalizer where the SuppressFinalize goes: the place that originally declared it was IDisposable, in the non-virtual Dispose method

Personally, I think it makes much more sense if the responsibility for calling SuppressFinalize was on the type that declared the finalizer. There is no need to burden every single base class with this (or the whole Dispose(bool) pattern) just because there is a minuscule chance that a derived class will have a finalizer.

[jnm2]

@bartonjs Thanks! The answer is clarifying. I agree with everything @svick just said, but I'll open an actual issue if I have something to propose.

[svick]

Previous discussion on this topic: dotnet/docs#5485.