Docs: The Dispose guidelines really should be updated to point developers in the right direction

[BrunoJuchli]

The guidelines should read:

When working with unmanaged resources:

1. use an existing SafeHandle if possible

2. If not possible: subclass SafeHandle to create one that meets your needs. This class should not do anything more than managing unmanaged resources. It should be sealed.

3. If that's not possible: create your own class which implements IDisposable + Finalizer

3.1 The class should be sealed

3.2 If sealing the class is not possible: add protected void Dispose(bool disposeManagedResources)

99% of .net developers should never ever actually implement the "full blown" dispose pattern.

CA1063 should be updated accordingly.

---

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

• ID: 572ef6c6-5066-4a14-76a0-c3fb4b62e604
• Version Independent ID: 9c754da1-c234-fb12-580b-0fcf9ffe2c20
• Content: Implementing a Dispose method
• Content Source: docs/standard/garbage-collection/implementing-dispose.md
• Product: dotnet
• GitHub Login: @rpetrusha
• Microsoft Alias: ronpet

[mairaw]

@Maoni0 can you help with this feedback about GC?

[Maoni0]

sounds reasonable to me. but this is more a BCL issue. @stephentoub do we already have some sort of guidelines wrt using SafeHandle somewhere?

[stephentoub]

I don't believe the Framework Design Guidelines that @KrzysztofCwalina previously published covered SafeHandle, other than in comments included in the annotated version. Guidance like the above is reasonable. In particular, I view the discussion related specifically to finalizers: you should really think hard before adding a finalizer to manage unmanaged resources... if you find yourself thinking you need to, you most likely should just use a SafeHandle.

[KrzysztofCwalina]

@bartonjs and I talked about starting an effort that would serve as an "errata"/update to the FDGs. @bartonjs, maybe this is a good candidate for the first update topic?

[bartonjs]

FDG2e has a pretty solid opinion here:

• AVOID making types finalizable.
  
  
  
  • (Extend SafeHandle if you're wrapping a native resource)
  
  

9.4.2 (Finalizable Types)

[mavasani]

Tagging @sharwell @gpetrou @duracellko as we hit this issue in https://github.com/dotnet/roslyn-analyzers/issues/1950

CA1063 FxCop analyzer tried to aggressively flag all derived types overriding the destructor/finalizer and suggested to remove them, which caused CA1063 to fire on the documentation snippet cited here.

As we concluded in https://github.com/dotnet/roslyn-analyzers/issues/1950 and seems to match with the consensus on this issue, we should strongly push customers towards using SafeHandles instead of providing their own finalizer override for cleanup. We have updated the analyzer to be bit more conservative and only flag finalizer override on a Derived type _if_ there is a finalizer override in at least one of its base type in its inheritance chain, as this is almost certainly likely to cause duplicate Dispose invocations.

Can we please update this documentation to not suggest people towards writing finalizers?

[sharwell]

I disagree with item 3 in the original list. I am not aware of a circumstance where a user needs to write a finalizer. If a user reaches a step where they are writing a finalizer, they have already made a mistake in an earlier step.

New guidance for IDisposable should be written with the following rules:

1. The legacy guidance for IDisposable, identifiable by a method with the signature Dispose(bool) should only be followed if and only if the following is true:
   
   
   
   1. The type is derived from a type already following the legacy guidance
   
   

2. A type implementing IDisposable should have the following method:

   public virtual void Dispose()
   {
   }
   

3. The Dispose() implementation SHOULD NOT be sealed unless the containing type is also sealed

4. The Dispose() implementation SHOULD NOT call GC.SuppressFinalize(this) (the method is always unnecessary as the type does not have a finalizer)
5. The type MUST NOT override object.Finalize

[stephentoub]

I disagree with item 3 in the original list. I am not aware of a circumstance where a user needs to write a finalizer. If a user reaches a step where they are writing a finalizer, they have already made a mistake in an earlier step.

There are certainly times when custom finalizers are valuable; it's just the 1% case.

[bartonjs]

@sharwell Most of that guidance is in direct disagreement with the established rules (and guidance published in Framework Design Guidelines).

If nothing else, doing a 180 on the guidance would make for a very hard universe to understand, so I don't see us rewriting them as you suggested.

[BrunoJuchli]

@sharwell @stephentoub

Quoting @stephentoub

There are certainly times when custom finalizers are valuable; it's just the 1% case.

Which would be covered by item 2 on the original list (subclassing SafeHandle) (https://github.com/dotnet/docs/issues/8463#issue-371470607):

1. use an existing SafeHandle if possible
2. If not possible: subclass SafeHandle to create one that meets your needs. This class should not do anything more than managing unmanaged resources. It should be sealed.

Now, I personally can't fathom a case where it be necessary and sensible to have a finalizer outside of a SafeHandle. But maybe that's my lack of knowledge/imagination, but still, we'd be in agreement it's a very rare case, right? So:

• Does it make sense to "blow up" documentation just for a very rare case?
• Since documentation is now much more "alive" would it be an option to omit the rare case until someone actually comes up with that case and can reason why it's needed? At that time we might better know how to deal with this (how to properly write documentation). For now, this would make it simpler to progress here.

[BrunoJuchli]

@bartonjs

@sharwell Most of that guidance is in direct disagreement with the established rules (and guidance published in Framework Design Guidelines).

If nothing else, doing a 180 on the guidance would make for a very hard universe to understand, so I don't see us rewriting them as you suggested.

Could you elaborate on that? Personally I find it a very hard universe to understand where documentation is illogical and pointing me in the wrong direction (and, as a result, over the years I've had repeated discussions about this specific topic with many a co worker). Are you sure the universe is easier to understand when documentation is "wrong" as when it's "right" but it's more complicated?

---

I believe, though, that it's necessary to make the change and it's reasoning "public", so that this "universe" is not harder to understand than necessary.
Focus should be given to the new guidance but the old guidance needs to be mentioned.

How about the following documentation structure:

Pre-amble:

(put this in a box...)
The recommended implementation of IDisposable has been significantly changed in 2019. It is now recommended to handle unmanaged resource cleanup solely in SafeHandles. See [Change History].

Actual Documentation:

(doesn't need a header...)
... written as if the "legacy" documentation didn't exist.

How to handle Legacy code:

The legacy guidance (see [Change History]) for IDisposable, identifiable by a method with the signature Dispose(bool) should only be followed if and only if the following is true:

The type is derived from a type already following the legacy guidance

... Insert Legacy Guidance here...

Change History:

...reason why documentation has changed and how it was changed. Not a full blown git revision list but really just why and how the guideline was changed. Add a link to [How to handle Legacy code]

[sharwell]

Most of that guidance is in direct disagreement with the established rules (and guidance published in Framework Design Guidelines).

@bartonjs I agree, and I've been an ardent supporter of the established rules for more than a decade. However, there are two issues that came to light over the past two years which eventually resulted in me changing my mind:

1. No one outside of @stephentoub should be writing finalizers (he works on the core runtime where finalization support underneath APIs like SafeHandle is implemented). It has been argued that additional exceptions to this rule exist, but they are so uncommon that the guidance does not need to account for their existence. I can think of only one case over the past decade where it was used for a purpose, though that case occurred many years ago and I'm not sure it would stand up to scrutiny today.
2. Few people understand the current rules. It wasn't until I observed relatively widespread incorrect application of the rules in dotnet/roslyn (a place where I would assume the rules would be readily understood and followed) that I realized how difficult it is for users to understand the rules.

The updated rules would be a dramatic simplification which focuses on the only recommended implementation strategy (managed resources), eliminating consideration for a scenario which users should not encounter (finalizers). As a special case, they allow for the ongoing existence of the legacy pattern without introducing API incompatibilities.

[bartonjs]

I write finalizers all the time, they're useful in tracing object lifetimes. They don't release, but they do get written...

The current rule bullets from FDG:

9.4 (Dispose Pattern)

• DO implement the Basic Dispose Patten on types containing instances of disposable types
  
  
  
  • bartonjs summary: If you hold a disposable thing, be disposable.
  
  
• DO implement the Basic Dispose Pattern and provide a finalizer on types holding resources that need to be freed explicitly and do not [themselves] have finalizers.
  
  
  
  • bartonjs summary: If you've done the P/Invoke to create a resource, it's your job to free it. But you should have just made a SafeHandle for that resource and then this doesn't apply (because the resource is finalized already).
  
  
• CONSIDER implementing the Basic Dispose Pattern on classes that themselves don't hold unmanaged resources or disposable objects but are likely to have subtypes that do.
  
  
  
  • e.g. Stream
  
  

9.4.1 (Basic Dispose Pattern)

• DO declare a protected virtual void Dispose(bool disposing) method to centralize all logic related to releasing unmanaged resources.
  
  
  
  • bartonjs: Since Dispose has grown in scope since the book was written "unmanaged" doesn't really belong there anymore.
  
  
• DO implement the IDisposable interface by simply calling Dispose(true) followed by GC.SuppressFinalize(this).
• DO NOT make the parameterless Dispose method virtual.
• DO NOT declare any [other overloads of Dispose].
• DO allow the Dispose(bool) method to be called more than once. The method might choose to do nothing after the first call.
• AVOID throwing an exception from within Dispose(bool)...
• DO throw an ObjectDisposedException from any member that cannot be used after the object has been disposed of.
• CONSIDER providing method Close(), in addition to the Dispose(), if close is standard terminology in the area.

9.4.2 (Finalizable Types)

• AVOID making types finalizable.
  
  
  
  • bartonjs: Goes on to say "Prefer using resource wrappers such as SafeHandle..."
  
  
• DO NOT make value types finalizable.
• DO make a type finalizable if the type is responsible for releasing an unmanaged resource that does not have its own finalizer.
  
  
  
  • bartonjs: This is "if you're inventing an alternative to SafeHandle". It says that your Finalize method should simply be Dispose(false).
  
  
• DO implement the Basic Dispose Pattern on every finalizable type...
• DO NOT access finalizable objects in the finalizer code path...
• DO make your Finalize method protected.
  
  
  
  • bartonjs: C# doesn't allow the option, it just does it.
  
  
• DO NOT let exceptions escape from the finalizer logic, [unless you want the process to terminate].
• CONSIDER creating and using a critical finalizable object...

---

Yeah, it's a lot of bullets, but it makes sense with the step by step explanations. A simpler form would be:

• On a type that directly implements IDIsposable:

  • public void Dispose() { Dispose(true); GC.SuppressFinalize(this); }
  • protected virtual void Dispose(bool disposing) { ... }

• On a type that extends an IDisposable type, when needed:
  *
  ```C#
  protected override void Dispose(bool disposing)
  {
  if (disposing)
  {
  _disposableField?.Dispose();
  // other "live graph" operations
  }

  // If there are any struct fields representing resources that need to be released, do so here.
  // Make sure to clean up state so you don't try to release it twice.
  //
  // If you followed the rest of the guidelines you never have code here,
  // because it was a SafeHandle released in the above if.

  base.Dispose(disposing);
  }
  ```

• Don't produce new finalizable types

• _Exemplum, hic est_ SafeHandle _faciendi_.
• If you do, implement the finalizer as Dispose(false)

---

I'm not saying that the current docs are great (or necessarily even say what we want them to say); just that I don't think that changing the guidelines is justified:

• The current design handles both "simple dispose" and finalizable type chains.
• "Consistency is the key characteristic of a well-designed framework. It is one of the most important factors affecting productivity. ... Consistency is probably the main theme of this book." (Framework Design Guidelines (2nd edition), p6)
• Change is terrible, unless it's great (and I don't think that this change is "great")
  
  
  
  • Introducing a split like "for types created before 2019 use this Dispose pattern, and for types created in or after 2019 use this other Dispose pattern" has to have the same level of "wow" to it that async/await have over IAsyncResult/Begin/End.
  
  

Also, guidance would still have to be written for the 1% case of a new public finalizable type. It would have to explain how Finalize and Dispose interact, and how to properly handle that for unsealed finalizable types.

If we were making the framework anew, I'd probably prefer the simplified alternative.

---

My main position, I guess, is that @BrunoJuchli's original description is the right thing to do... to update the documentation to say:

• If a SafeHandle already exists for your needs, use it.
• If one doesn't, here's how you make your own SafeHandle type.
• Are you still here? Are you sure you need a finalizer? See previous points... Ugh. Okay. Here's how to do it "right" (given that doing it right is to not do it, but to instead use SafeHandle).

[stephentoub]

Which would be covered by item 2 on the original list (subclassing SafeHandle)

No, they all won't.

Now, I personally can't fathom a case where it be necessary and sensible to have a finalizer outside of a SafeHandle.

Here are just a few examples off the top of my head (I'm sure there are more):

• HttpConnection (https://github.com/dotnet/corefx/blob/5c55936c24bb6a2038d3eb5a68d4a51011d02284/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs#L1648-L1655). In SocketsHttpHandler, we need to be able to support MaxConnectionsPerServer, which means we need to be able to keep track of how many connections are still used. Hopefully the developer properly Disposes of their implicit reference to a connection, e.g. via Dispose'ing of the response object or the response stream object, but if they don't and instead just drop the connection object, what are we supposed to do? How can we know when the connection is no longer being used and we can reclaim that associated "count" from our limit? By using a finalizer. Note that this isn't wrapping a native handle, and so SafeHandle doesn't make sense, nor could we derive from SafeHandle even if we wanted to, as we can only have a single base class (and allocating yet another object is unnecessary overhead).
• DebugFinalizableAsyncStateMachineBox (https://github.com/dotnet/coreclr/blob/eff427c02a89e135c38a96032feb3c9a6a13cf5b/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs#L507-L521). If you write an async method that awaits something that never completes (e.g. await new TaskCompletionSource().Task;), your async method is never going to complete, and things that await it will never complete, and so on. So, we fire an EventSource / ETW event if we detect this happening, which we can tell by seeing whether the boxed async method builder has completed by the time it's collected. How do we do that? A finalizer. Again, SafeHandle doesn't make sense here, and we couldn't derive from it even if we wanted to.
• ThreadLocal<T> (https://github.com/dotnet/coreclr/blob/eff427c02a89e135c38a96032feb3c9a6a13cf5b/src/System.Private.CoreLib/shared/System/Threading/ThreadLocal.cs#L141). When all threads keeping a particular ThreadLocal<T> alive go away, its associated slot should be reclaimed and used for another thread local, or else we leak. How can we know that? A finalizer. Again, SafeHandle isn't relevant.
• ArrayPool (via Gen2GcCallback https://github.com/dotnet/coreclr/blob/eff427c02a89e135c38a96032feb3c9a6a13cf5b/src/System.Private.CoreLib/shared/System/Gen2GcCallback.cs#L44). ArrayPool caches some number of arrays, but that's an unnecessary "leak" if you don't actually need those arrays again, if memory pressure is causing an issue, etc., so we want to be able to trim those arrays over time when there's pressure. How can we know that there's pressure? When the GC runs, and in particular, when it's aggressively trying to collect as much as possible via a Gen2 collection. How can we know that? A finalizer.
• Timer (https://github.com/dotnet/coreclr/blob/eff427c02a89e135c38a96032feb3c9a6a13cf5b/src/System.Private.CoreLib/src/System/Threading/Timer.cs#L771-L785). Timers allocate an underlying TimerQueueTimer that's stored in a collection such that the timer can be fired accordingly. As long as the Timer is alive, we need to keep that TimerQueueTimer alive. But if the Timer is dropped by the developer without being Dispose'd, we need to remove that TimerQueueTimer from its rooted collection, otherwise it's a leak. How do we do that? A finalizer.
• RemoteExecutorTestBase (https://github.com/dotnet/corefx/blob/5c55936c24bb6a2038d3eb5a68d4a51011d02284/src/CoreFx.Private.TestUtilities/src/System/Diagnostics/RemoteExecutorTestBase.cs#L391-L396). Our unit test helper for executing test code in another process. If an exception occurs in the remote process, we want to propagate that error back into the calling process, which we do when the "handle" (not a native IntPtr handle, just an object representing the remote operation) is Dispose'd. What if it's not Dispose'd? Then the test that should fail may actually pass erroneously. How do we help avoid this? You guessed it, a finalizer. It's caught numerous errors.

[sharwell]

I write finalizers all the time, they're useful in tracing object lifetimes

@bartonjs I don't know enough about your application to know why you are tracing lifetimes in this way. However, when (rarely) faced with similar situations in the past, I've always found alternative solutions viable (typically event tracing / profiling, but could also be dependent objects).

Here are just a few examples off the top of my head

@stephentoub Your examples are fascinating, but to me they highlight how far the core framework goes out of the way to ensure users do not need to write their own finalizers. I don't see dotnet/coreclr or dotnet/corefx as the primary audience for the updated guidance.

• HttpConnection: This API is one of the closest to what could appear in client code. However, the use of a finalizer appears to be largely due to a combination of optimizing both for maximum performance (avoid unnecessary allocations) and hardening against misuse by unknown downstream clients. If a non-framework developer was implementing a similar feature, it's unlikely that avoiding the object allocation would result in observable performance overhead (similar to e.g. TaskExceptionHolder, but derived from CriticalHandle). In a case where it was observable (and mattered), the client would likely be interested in finding a way to avoid the finalizer cost altogether, instead focusing on ensuring all relevant objects are disposed at the correct times.
• Timer: TimerHolder could arguably be derived from CriticalHandle. Maybe there's a reason the core framework cannot do this, but for users facing a similar situation it would be a viable option.
• RemoteExecutorTestBase: This code could likely be improved by tracking open handles in the test class, and verifying either in Dispose or IAsyncLifetime.DisposeAsync that all handles have been closed. Unlike the finalization approach, this would result in deterministic validation of cleanup during the test lifetime, and failures would appear as test failures for the test that failed to clean up. This approach would not require (or benefit from) the use of a finalizer.

[stephentoub]

TimerHolder could arguably be derived from CriticalHandle

It doesn't have an IntPtr handle. So this is just another way of writing a finalizer (a harder way), because you have to override everything to do with that handle, instead of just writing a finalizer. Why?

This code could likely be improved by tracking open handles in the test class

That then requires way more boilerplate, doesn't work well with nesting (which these do), etc. Your suggesting a workaround for something that doesn't need a workaround.

HttpConnection

You're very focused on deriving from CriticalHandle. It represents a handle! That's not relevant here at all. There is no IntPtr handle. Any such handle we create would be completely fabricated, and we'd doing it purely to use a different syntax for authoring a finalizer.

but to me they highlight how far the core framework goes out of the way to ensure users do not need to write their own finalizers

I disagree. These aren't about ensuring users don't need to write finalizers, these are because a class was needed that needed such functionality.

[sharwell]

My claim is the current guidance for disposable objects is a large cognitive overhead, and that the cognitive overhead is substantially reduced by disallowing a specific construct (finalizers). I further claim that this is reasonable because both of the following are true:

1. Cases where custom finalizers are needed because they serve a purpose are exceedingly rare.
2. Even within the set of cases mentioned by (1), the subset of those cases for which CriticalHandle cannot be leveraged to avoid the use of a custom finalizer are exceedingly rare.

Simplification of this pattern would allow average developers to be more successful (make fewer mistakes) by avoiding a known problem area.

[stephentoub]

We should not be "disallowing" finalizers; they are a valuable and viable tool. And I agree with @bartonjs on the guidance:

• Have native resources you need to free? Use SafeHandle if at all possible, starting with built-in ones and writing your own if they don't suffice.
• Have to do some other kind of clean-up that's not as clean as calling some function to close/clear an IntPtr? Here's how to use finalizers correctly.

I don't believe the need for custom finalizers is "exceedingly" rare. A large majority of the cases are nicely handled by SafeHandle, and for everything else, there are finalizers. Saying that CriticalHandle should be used instead in such cases (where there is no handle) makes zero sense to me: you're still writing a finalizer, you're just doing it in a way that's more convoluted and more expensive.

[BrunoJuchli]

@stephentoub

I don't believe the need for custom finalizers is "exceedingly" rare.

I tried searching github for C# code containing ": IDisposable" and "\~". Unfortunately the search ignores both ":" and "\~". So it's a bit hard to get unbiased data.
So all i've got is annecdotal data: In 10 years of development I encountered exactly one finalizer implementation not part of the .Net Framework. It was in MahApps.Metro and it was faulty - the only reason it got relevant for me (fixing a bug...).
Most of my work colleagues during that years would not have been able to detect and understand the issue. They've never written a finalizer and never needed one. They have, however, been implementing IDisposable many, many times.
Almost every time the following IDisposable implementation sufficed:

public void Dispose()
{
    this.something.Dispose();
}

Sometimes this was sensible (but mostly would not be implemented...):

public void Dispose()
{
    this.something.Dispose();
    this.isDisposed = true;
}

public void Foo()
{
    if (this.isDisposed)
    {
        throw new ObjectDisposedException(this.ToString());
    }
}

And in rare cases double disposal needed to be handled (i.E. perform cleanup only the first time Dispose is called, the second time "do nothing").

Actually, quite a lot of "our" IDisposable implementations might be considered a "misuse". They only exist to be able to write:

using(var foo = new Foo())
{
    foo....
}

but do not represent an actual resource which requires cleanup.

Now, quite obviously I've never been tasked with implementing a framework or a library...

---

I do believe the need for custom finalizers to be very rare.
I'm pretty convinced that the majority of dev's is not creating librarys and frameworks. I even do believe the suggested "basic dispose pattern" is unnecessary complex. In most cases you can keep things extremely simple by making the class sealed.

---

Now, I also believe it's bad to hide existing complexity from users too much. I believe users should be guided to the "simple path" but, if necessary, should be educated about the complex use cases.
I also understand the need for documentation to cover the "legacy" guidance - as implementations will be common for many years to come.
I also believe documentation should not have the same guidance (focus, preferrence) as the Framework design guidelines. Developers are commonly not creating frameworks.

Therefore, personally, I'm back at my original suggestion: https://github.com/dotnet/docs/issues/8463#issue-371470607

I'd also like documentation to contain a proper explanation for the need of non-throwing, safe, double-disposal and the "basic dispose pattern" and proper extensibility in case of non-sealed IDisposable implementations. And of course: how to actually use SafeHandles and how to extend SafeHandle.

[KevinCathcart]

The issues I see with the current guidelines is that:

1. The full version of this pattern is optimized for a class that directly owns both managed and unmanaged resources. This should be extremely uncommon. Ideally all unmanaged resources should be owned by a SafeHandle, or at the very least a dedicated sealed class, that does not also own managed resources too.
2. Even the "basic" version of this disposable pattern (without a finalizer, because it has no unmanaged resources) is designed specifically to support having a subclass that directly owns an unmanaged resource. But once again that should be extremely uncommon.
3. I rarely see only the "basic" version of this pattern implemented (except in Microsoft code). Instead I often see people implement the full pattern, with a finalizer, despite the class in question not having any unmanaged resources. This slows down the garbage collection process for no real benefit.
4. I've repeatedly seen people not understanding what counts as unmanaged resources, and calling .Dispose() or .Close() etc on managed objects outside of the if (disposing) block. Usually this is from people mistakenly thinking that a FileStream (or similar class) counts as an unmanaged resource, since they know it must in some way wrap a file handle. (This is safe for CriticalFinalizerObjects like SafeHandles, but I'm not referring to those cases.)
5. Many developers do not fully understand that everything outside of the if (disposing) block, must be finalizer-safe, and/or do not understand what finalizer-safe code entails. There is a lot that is not safe to do/assume in a finalizer.
6. Lastly, if we restrict our focus to only the common case, which is a class owning only managed resources, all of this pattern is unnecessary overhead. For the common case, having Dispose() simply directly dispose on the each IDisposable field the class owns, is good enough.

As a reminder of how tricky finalizers are be:

• In a finalizer, you cannot assume that your object was ever fully constructed. If something in your constructor threw an exception, your object may be left in a partially constructed state when the finalizer is called. Furthermore, if you are not a sealed class, it is possible that exactly none of your constructor has run yet, since the exception could be in a field initializer of a subclass!
• In a finalizer, you cannot, in general, assume that other managed resources are in a usable state. Specifically, you are not guaranteed to be finalized before objects you reference. (Yeah, critical finalizers having the weak ordering after normal ones is one special case where you can rely on the ordering.)
• The finalizer is running on a different thread. If your class exposes some object (perhaps a collection), some other code may still have a reference to it, and be interacting with it, so it is not in general safe to to call into such objects from a finalizer, unless they are thread safe.
• When a finalizer get called, other methods of your object could still be running! It is possible to garbage collect an object while its method is still running, as long as there is no code later in the method that references this. This means that any objects stored in fields of your class, might still be being used if they were copied into a local variable in some method, even if they were not otherwise exposed.
• Your finalizer should not allow an exception to get thrown, or it could kill the finalizer thread. (At least I'm pretty sure this used to be the case. I've not paid enough attention to know if the runtime has changed this.)

To summarize writing finalizers: you cannot risk letting an exception escape your finalizer. Touching any reference-type members of your object is risky, and should probably be avoided. You cannot even assume that your constructor ran to completion (or even ran at all), so even your value type members might be in an inconsistent state.

Now remember that all of this applies to all the code in Dispose(bool) outside of the if(disposing) block, even when your class does not have a finalizer at all (just in case a subclass adds one). I think it should come as no surprise that many people mess this up.

Other uses of finalizers

Stephen Toub is of course correct that there are other valid uses of finalizers, besides unmanaged resources. And obviously this pattern is useful when a class has both managed disposable resources, and another use for a finalizer.

But let's spend a minute and pretend that these cases are weird instances of unmanaged resources, even though they really are not. In that case, the general advice would be to create some small sealed finalizable class that wraps this resource. And of course, even though these cases are not really unmanaged resources, this approach is almost always possible. (For these, I would agree with Stephen Toub that abusing CriticalHandle is wrong here. If creating a separate tiny class, it should either inherit from CriticalFinalizerObject or just from regular object.)

Putting the finalizer directly in the main object is really just an optimization to avoid an allocation, and for some reason we don't seem to be too worried about that extra allocation when using a SafeHandle, so I'm not sure why it would be that critical in these other cases.

So in practice these alternate finalizer uses can be treated like unmanaged resources.

Conclusion

I'll go out on a limb here, and say that I strongly suspect that the majority of implementations of Dispose(bool) which include code outside of the if block are buggy, and are not fully safe for running in a finalizer.

Having this guidance means that instead of a straightforward Dispose() implementation that simply calls the dispose method of each owned IDisposable resource (will almost always work correctly), some people trying to do the right thing keep ending up writing buggy Dispose(bool) methods, and often add needless finalizers, all for the benefit of a hypothetical subclass that might want to directly own an unmanaged resource. (Which is a discouraged practice in the first place, )