Fixes dotnet#35935 (dotnet#36342)

* Fixes dotnet#35935

* More fixes

* Apply suggestions from code review

Co-authored-by: Bill Wagner <wiwagn@microsoft.com>

---------

Co-authored-by: Bill Wagner <wiwagn@microsoft.com>

Author: IEvangelist

docs/standard/garbage-collection/implementing-dispose.md

@@ -1,7 +1,7 @@
 ---
 title: "Implement a Dispose method"
 description: In this article, learn to implement the Dispose method, which releases unmanaged resources used by your code in .NET.
-ms.date: 02/16/2023
+ms.date: 07/20/2023
 dev_langs:
   - "csharp"
   - "vb"
@@ -13,11 +13,11 @@ ms.topic: how-to
 
 # Implement a Dispose method
 
-The <xref:System.IDisposable.Dispose%2A> method is primarily implemented to release unmanaged resources. When working with instance members that are <xref:System.IDisposable> implementations, it's common to cascade <xref:System.IDisposable.Dispose%2A> calls. There are additional reasons for implementing <xref:System.IDisposable.Dispose%2A>, for example, to free memory that was allocated, remove an item that was added to a collection, or signal the release of a lock that was acquired.
+The <xref:System.IDisposable.Dispose%2A> method is primarily implemented to release unmanaged resources. When working with instance members that are <xref:System.IDisposable> implementations, it's common to cascade <xref:System.IDisposable.Dispose%2A> calls. There are other reasons for implementing <xref:System.IDisposable.Dispose%2A>, for example, to free memory that was allocated, remove an item that was added to a collection, or signal the release of a lock that was acquired.
 
-The [.NET garbage collector](index.md) does not allocate or release unmanaged memory. The pattern for disposing an object, referred to as the dispose pattern, imposes order on the lifetime of an object. The dispose pattern is used for objects that implement the <xref:System.IDisposable> interface. This pattern is common when interacting with file and pipe handles, registry handles, wait handles, or pointers to blocks of unmanaged memory, because the garbage collector is unable to reclaim unmanaged objects.
+The [.NET garbage collector](index.md) doesn't allocate or release unmanaged memory. The pattern for disposing an object, referred to as the dispose pattern, imposes order on the lifetime of an object. The dispose pattern is used for objects that implement the <xref:System.IDisposable> interface. This pattern is common when interacting with file and pipe handles, registry handles, wait handles, or pointers to blocks of unmanaged memory, because the garbage collector is unable to reclaim unmanaged objects.
 
-To help ensure that resources are always cleaned up appropriately, a <xref:System.IDisposable.Dispose%2A> method should be idempotent, such that it is callable multiple times without throwing an exception. Furthermore, subsequent invocations of <xref:System.IDisposable.Dispose%2A> should do nothing.
+To help ensure that resources are always cleaned up appropriately, a <xref:System.IDisposable.Dispose%2A> method should be idempotent, such that it's callable multiple times without throwing an exception. Furthermore, subsequent invocations of <xref:System.IDisposable.Dispose%2A> should do nothing.
 
 The code example provided for the <xref:System.GC.KeepAlive%2A?displayProperty=nameWithType> method shows how garbage collection can cause a finalizer to run while an unmanaged reference to the object or its members is still in use. It may make sense to utilize <xref:System.GC.KeepAlive%2A?displayProperty=nameWithType> to make the object ineligible for garbage collection from the start of the current routine to the point where this method is called.
 
@@ -29,7 +29,7 @@ Writing code for an object's finalizer is a complex task that can cause problems
 
 A <xref:System.Runtime.InteropServices.SafeHandle?displayProperty=nameWithType> is an abstract managed type that wraps an <xref:System.IntPtr?displayProperty=nameWithType> that identifies an unmanaged resource. On Windows it might identify a handle, and on Unix, a file descriptor. The `SafeHandle` provides all of the logic necessary to ensure that this resource is released once and only once, either when the `SafeHandle` is disposed of or when all references to the `SafeHandle` have been dropped and the `SafeHandle` instance is finalized.
 
-The <xref:System.Runtime.InteropServices.SafeHandle?displayProperty=nameWithType> is an abstract base class. Derived classes provide specific instances for different kinds of handle. These derived classes validate what values for the <xref:System.IntPtr?displayProperty=nameWithType> are considered invalid and how to actually free the handle. For example, <xref:Microsoft.Win32.SafeHandles.SafeFileHandle> derives from `SafeHandle` to wrap `IntPtrs` that identify open file handles/descriptors, and overrides its <xref:System.Runtime.InteropServices.SafeHandle.ReleaseHandle?displayProperty=nameWithType> method to close it (via the `close` function on Unix or `CloseHandle` function on Windows). Most APIs in .NET libraries that create an unmanaged resource will wrap it in a `SafeHandle` and return that `SafeHandle` to you as needed, rather than handing back the raw pointer. In situations where you interact with an unmanaged component and get an `IntPtr` for an unmanaged resource, you can create your own `SafeHandle` type to wrap it. As a result, few non-`SafeHandle` types need to implement finalizers. Most disposable pattern implementations only end up wrapping other managed resources, some of which may be `SafeHandle` objects.
+The <xref:System.Runtime.InteropServices.SafeHandle?displayProperty=nameWithType> is an abstract base class. Derived classes provide specific instances for different kinds of handle. These derived classes validate what values for the <xref:System.IntPtr?displayProperty=nameWithType> are considered invalid and how to actually free the handle. For example, <xref:Microsoft.Win32.SafeHandles.SafeFileHandle> derives from `SafeHandle` to wrap `IntPtrs` that identify open file handles/descriptors, and overrides its <xref:System.Runtime.InteropServices.SafeHandle.ReleaseHandle?displayProperty=nameWithType> method to close it (via the `close` function on Unix or `CloseHandle` function on Windows). Most APIs in .NET libraries that create an unmanaged resource wraps it in a `SafeHandle` and return that `SafeHandle` to you as needed, rather than handing back the raw pointer. In situations where you interact with an unmanaged component and get an `IntPtr` for an unmanaged resource, you can create your own `SafeHandle` type to wrap it. As a result, few non-`SafeHandle` types need to implement finalizers. Most disposable pattern implementations only end up wrapping other managed resources, some of which may be `SafeHandle` objects.
 
 The following derived classes in the <xref:Microsoft.Win32.SafeHandles> namespace provide safe handles.
 
@@ -43,7 +43,7 @@ The following derived classes in the <xref:Microsoft.Win32.SafeHandles> namespac
 
 ## Dispose() and Dispose(bool)
 
-The <xref:System.IDisposable> interface requires the implementation of a single parameterless method, <xref:System.IDisposable.Dispose%2A>. Also, any non-sealed class should have an additional `Dispose(bool)` overload method.
+The <xref:System.IDisposable> interface requires the implementation of a single parameterless method, <xref:System.IDisposable.Dispose%2A>. Also, any non-sealed class should have an `Dispose(bool)` overload method.
 
 Method signatures are:
 
@@ -52,12 +52,12 @@ Method signatures are:
 
 ### The Dispose() method
 
-Because the `public`, non-virtual (`NotOverridable` in Visual Basic), parameterless `Dispose` method is called when it is no longer needed (by a consumer of the type), its purpose is to free unmanaged resources, perform general cleanup, and to indicate that the finalizer, if one is present, doesn't have to run. Freeing the actual memory associated with a managed object is always the domain of the [garbage collector](index.md). Because of this, it has a standard implementation:
+Because the `public`, non-virtual (`NotOverridable` in Visual Basic), parameterless `Dispose` method is called when it's no longer needed (by a consumer of the type), its purpose is to free unmanaged resources, perform general cleanup, and to indicate that the finalizer, if one is present, doesn't have to run. Freeing the actual memory associated with a managed object is always the domain of the [garbage collector](index.md). Because of this, it has a standard implementation:
 
 :::code language="csharp" source="../../../samples/snippets/csharp/VS_Snippets_CLR/conceptual.disposable/cs/Disposable.cs" id="Dispose":::
 :::code language="vb" source="../../../samples/snippets/visualbasic/VS_Snippets_CLR/conceptual.disposable/vb/Disposable.vb" id="Dispose":::
 
-The `Dispose` method performs all object cleanup, so the garbage collector no longer needs to call the objects' <xref:System.Object.Finalize%2A?displayProperty=nameWithType> override. Therefore, the call to the <xref:System.GC.SuppressFinalize%2A> method prevents the garbage collector from running the finalizer. If the type has no finalizer, the call to <xref:System.GC.SuppressFinalize%2A?displayProperty=nameWithType> has no effect. Note that the actual cleanup is performed by the `Dispose(bool)` method overload.
+The `Dispose` method performs all object cleanup, so the garbage collector no longer needs to call the objects' <xref:System.Object.Finalize%2A?displayProperty=nameWithType> override. Therefore, the call to the <xref:System.GC.SuppressFinalize%2A> method prevents the garbage collector from running the finalizer. If the type has no finalizer, the call to <xref:System.GC.SuppressFinalize%2A?displayProperty=nameWithType> has no effect. The actual cleanup is performed by the `Dispose(bool)` method overload.
 
 ### The Dispose(bool) method overload
 
@@ -77,9 +77,9 @@ The body of the method consists of three blocks of code:
 
   - **Managed objects that implement <xref:System.IDisposable>.** The conditional block can be used to call their <xref:System.IDisposable.Dispose%2A> implementation (cascade dispose). If you have used a derived class of <xref:System.Runtime.InteropServices.SafeHandle?displayProperty=nameWithType> to wrap your unmanaged resource, you should call the <xref:System.Runtime.InteropServices.SafeHandle.Dispose?displayProperty=nameWithType> implementation here.
 
-  - **Managed objects that consume large amounts of memory or consume scarce resources.** Assign large managed object references to `null` to make them more likely to be unreachable. This releases them faster than if they were reclaimed non-deterministically.
+  - **Managed objects that consume large amounts of memory or consume scarce resources.** Assign large managed object references to `null` to make them more likely to be unreachable. This releases them faster than if they were reclaimed nondeterministically.
 
-If the method call comes from a finalizer, only the code that frees unmanaged resources should execute. The implementer is responsible for ensuring that the false path doesn't interact with managed objects that may have been disposed. This is important because the order in which the garbage collector disposes managed objects during finalization is non-deterministic.
+If the method call comes from a finalizer, only the code that frees unmanaged resources should execute. The implementer is responsible for ensuring that the false path doesn't interact with managed objects that may have been disposed. This is important because the order in which the garbage collector disposes managed objects during finalization is nondeterministic.
 
 ## Cascade dispose calls
 
@@ -104,7 +104,7 @@ All non-sealed classes (or Visual Basic classes not modified as `NotInheritable`
 > [!IMPORTANT]
 > It's possible for a base class to only reference managed objects and implement the dispose pattern. In these cases, a finalizer is unnecessary. A finalizer is only required if you directly reference unmanaged resources.
 
-Here's an example of the general pattern for implementing the dispose pattern for a base class that uses a safe handle.
+Here's a general example of implementing the dispose pattern for a base class that uses a safe handle.
 
 :::code language="csharp" source="../../../samples/snippets/csharp/VS_Snippets_CLR_System/system.idisposable/cs/base1.cs":::
 :::code language="vb" source="../../../samples/snippets/visualbasic/VS_Snippets_CLR_System/system.idisposable/vb/base1.vb":::

samples/snippets/csharp/VS_Snippets_CLR_System/system.idisposable/cs/base1.cs

@@ -2,16 +2,20 @@
 using System;
 using System.Runtime.InteropServices;
 
-class BaseClassWithSafeHandle : IDisposable
+public class BaseClassWithSafeHandle : IDisposable
 {
     // To detect redundant calls
     private bool _disposedValue;
 
     // Instantiate a SafeHandle instance.
-    private SafeHandle _safeHandle = new SafeFileHandle(IntPtr.Zero, true);
+    private SafeHandle? _safeHandle = new SafeFileHandle(IntPtr.Zero, true);
 
     // Public implementation of Dispose pattern callable by consumers.
-    public void Dispose() => Dispose(true);
+    public void Dispose()
+    {
+        Dispose(true);
+        GC.SuppressFinalize(this);
+    }
 
     // Protected implementation of Dispose pattern.
     protected virtual void Dispose(bool disposing)
@@ -20,7 +24,8 @@ protected virtual void Dispose(bool disposing)
         {
             if (disposing)
             {
-                _safeHandle.Dispose();
+                _safeHandle?.Dispose();
+                _safeHandle = null;
             }
 
             _disposedValue = true;

samples/snippets/csharp/VS_Snippets_CLR_System/system.idisposable/cs/base2.cs

@@ -1,6 +1,6 @@
 using System;
 
-class BaseClassWithFinalizer : IDisposable
+public class BaseClassWithFinalizer : IDisposable
 {
     // To detect redundant calls
     private bool _disposedValue;

samples/snippets/csharp/VS_Snippets_CLR_System/system.idisposable/cs/calling2.cs

@@ -14,8 +14,8 @@ public WordCountCleanup(string filename)
 
         FullName = filename;
 
-        var txt = string.Empty;
-        StreamReader streamReader = null;
+        string txt = string.Empty;
+        StreamReader? streamReader = null;
         try
         {
             streamReader = new StreamReader(filename);

samples/snippets/csharp/VS_Snippets_CLR_System/system.idisposable/cs/derived1.cs

@@ -2,13 +2,13 @@
 using System;
 using System.Runtime.InteropServices;
 
-class DerivedClassWithSafeHandle : BaseClassWithSafeHandle
+public class DerivedClassWithSafeHandle : BaseClassWithSafeHandle
 {
     // To detect redundant calls
     private bool _disposedValue;
 
     // Instantiate a SafeHandle instance.
-    private SafeHandle _safeHandle = new SafeFileHandle(IntPtr.Zero, true);
+    private SafeHandle? _safeHandle = new SafeFileHandle(IntPtr.Zero, true);
 
     // Protected implementation of Dispose pattern.
     protected override void Dispose(bool disposing)
@@ -17,7 +17,8 @@ protected override void Dispose(bool disposing)
         {
             if (disposing)
             {
-                _safeHandle.Dispose();
+                _safeHandle?.Dispose();
+                _safeHandle = null;
             }
 
             _disposedValue = true;

samples/snippets/csharp/VS_Snippets_CLR_System/system.idisposable/cs/derived2.cs

@@ -1,9 +1,9 @@
-class DerivedClassWithFinalizer : BaseClassWithFinalizer
+public class DerivedClassWithFinalizer : BaseClassWithFinalizer
 {
     // To detect redundant calls
     private bool _disposedValue;
 
-    ~DerivedClassWithFinalizer() => this.Dispose(false);
+    ~DerivedClassWithFinalizer() => Dispose(false);
 
     // Protected implementation of Dispose pattern.
     protected override void Dispose(bool disposing)

samples/snippets/visualbasic/VS_Snippets_CLR_System/system.idisposable/vb/base1.vb

@@ -1,11 +1,14 @@
 Imports Microsoft.Win32.SafeHandles
 Imports System.Runtime.InteropServices
 
-Class BaseClassWithSafeHandle : Implements IDisposable
-    ' Flag: Has Dispose already been called?
-    Dim disposed As Boolean = False
+Public Class BaseClassWithSafeHandle
+    Implements IDisposable
+
+    ' To detect redundant calls
+    Private _disposedValue As Boolean
+
     ' Instantiate a SafeHandle instance.
-    Dim handle As SafeHandle = New SafeFileHandle(IntPtr.Zero, True)
+    Private _safeHandle As SafeHandle = New SafeFileHandle(IntPtr.Zero, True)
 
     ' Public implementation of Dispose pattern callable by consumers.
     Public Sub Dispose() _
@@ -15,13 +18,15 @@ Class BaseClassWithSafeHandle : Implements IDisposable
     End Sub
 
     ' Protected implementation of Dispose pattern.
-    Protected Overridable Sub Dispose(disposing As Boolean)
-        If disposed Then Return
+    Protected Overridable Sub Dispose(ByVal disposing As Boolean)
+        If Not _disposedValue Then
 
-        If disposing Then
-            handle.Dispose()
-        End If
+            If disposing Then
+                _safeHandle?.Dispose()
+                _safeHandle = Nothing
+            End If
 
-        disposed = True
+            _disposedValue = True
+        End If
     End Sub
 End Class

samples/snippets/visualbasic/VS_Snippets_CLR_System/system.idisposable/vb/base2.vb

@@ -1,6 +1,12 @@
-Class BaseClassWithFinalizer : Implements IDisposable
-    ' Flag: Has Dispose already been called?
-    Dim disposed As Boolean = False
+Public Class BaseClassWithFinalizer
+    Implements IDisposable
+
+    ' To detect redundant calls
+    Private _disposedValue As Boolean
+
+    Protected Overrides Sub Finalize()
+        Dispose(False)
+    End Sub
 
     ' Public implementation of Dispose pattern callable by consumers.
     Public Sub Dispose() _
@@ -10,20 +16,16 @@
     End Sub
 
     ' Protected implementation of Dispose pattern.
-    Protected Overridable Sub Dispose(disposing As Boolean)
-        If disposed Then Return
+    Protected Overridable Sub Dispose(ByVal disposing As Boolean)
+        If Not _disposedValue Then
 
-        If disposing Then
-	       	' Dispose managed objects that implement IDisposable.
-	        ' Assign null to managed objects that consume large amounts of memory or consume scarce resources.
-        End If
+            If disposing Then
+                ' TODO: dispose managed state (managed objects)
+            End If
 
-        ' Free any unmanaged objects here.
-        '
-        disposed = True
-    End Sub
-
-    Protected Overrides Sub Finalize()
-        Dispose(False)
+            ' TODO free unmanaged resources (unmanaged objects) And override finalizer
+            ' TODO: set large fields to null
+            _disposedValue = True
+        End If
     End Sub
 End Class

samples/snippets/visualbasic/VS_Snippets_CLR_System/system.idisposable/vb/derived1.vb

@@ -1,25 +1,25 @@
 Imports Microsoft.Win32.SafeHandles
 Imports System.Runtime.InteropServices
 
-Class DerivedClassWithSafeHandle : Inherits BaseClassWithSafeHandle
-    ' Flag: Has Dispose already been called?
-    Dim disposed As Boolean = False
+Public Class DerivedClassWithSafeHandle
+    Inherits BaseClassWithSafeHandle
+
+    ' To detect redundant calls
+    Private _disposedValue As Boolean
+
     ' Instantiate a SafeHandle instance.
-    Dim handle As SafeHandle = New SafeFileHandle(IntPtr.Zero, True)
+    Private _safeHandle As SafeHandle = New SafeFileHandle(IntPtr.Zero, True)
 
-    ' Protected implementation of Dispose pattern.
-    Protected Overrides Sub Dispose(disposing As Boolean)
-        If disposed Then Return
+    Protected Overrides Sub Dispose(ByVal disposing As Boolean)
+        If Not _disposedValue Then
 
-        If disposing Then
-            handle.Dispose()
-            ' Free any other managed objects here.
-            '
-        End If
+            If disposing Then
+                _safeHandle?.Dispose()
+                _safeHandle = Nothing
+            End If
 
-        ' Free any unmanaged objects here.
-        '
-        disposed = True
+            _disposedValue = True
+        End If
 
         ' Call base class implementation.
         MyBase.Dispose(disposing)