Stop freeing native resources in finalizers

src/Temporalio/Bridge/ByteArrayRef.cs

@@ -10,7 +10,7 @@ namespace Temporalio.Bridge
     /// Representation of a byte array owned by .NET. Users should usually use a
     /// <see cref="Scope" /> instead of creating this directly.
     /// </summary>
-    internal class ByteArrayRef
+    internal sealed class ByteArrayRef : IDisposable
     {
         private readonly GCHandle bytesHandle;
 
@@ -42,18 +42,10 @@ public ByteArrayRef(byte[] bytes, int length)
             }
         }
 
-        /// <summary>
-        /// Finalizes an instance of the <see cref="ByteArrayRef"/> class.
-        /// </summary>
-        ~ByteArrayRef()
-        {
-            bytesHandle.Free();
-        }
-
         /// <summary>
         /// Gets empty byte array.
         /// </summary>
-        public static ByteArrayRef Empty { get; } = new(Array.Empty<byte>());
+        public static ByteArrayRef Empty { get; } = new(Array.Empty<byte>()) { DisableDispose = true };
 
         /// <summary>
         /// Gets current byte array for this ref.
@@ -70,6 +62,8 @@ public ByteArrayRef(byte[] bytes, int length)
         /// </summary>
         internal static UTF8Encoding StrictUTF8 { get; } = new(false, true);
 
+        private bool DisableDispose { get; set; }
+
         /// <summary>
         /// Convert a string to a UTF-8 byte array.
         /// </summary>
@@ -210,5 +204,20 @@ public static ByteArrayRef FromNewlineDelimited(IEnumerable<string> values)
                 return new ByteArrayRef(stream.GetBuffer(), (int)stream.Length);
             }
         }
+
+        /// <inheritdoc/>
+        public void Dispose()
+        {
+            // Note, we intentionally do not free or call Dispose from a finalizer. When working
+            // with native objects, best practice is to not free in finalizer. On process shutdown,
+            // finalizers can be called even when the object is still referenced.
+
+            // Does not need to be thread safe
+            if (!DisableDispose)
+            {
+                DisableDispose = true;
+                bytesHandle.Free();
+            }
+        }
     }
 }

src/Temporalio/Bridge/Scope.cs

@@ -17,16 +17,10 @@ internal sealed class Scope : IDisposable
                 size = UIntPtr.Zero,
             };
 
-        private readonly List<ByteArrayRef> byteArrayRefs = new();
         private readonly List<GCHandle> gcHandles = new();
         private readonly List<IDisposable> disposables = new();
         private bool disposed;
 
-        /// <summary>
-        /// Finalizes an instance of the <see cref="Scope"/> class.
-        /// </summary>
-        ~Scope() => Dispose(false);
-
         /// <summary>
         /// Create a byte array ref.
         /// </summary>
@@ -39,7 +33,7 @@ public Interop.TemporalCoreByteArrayRef ByteArray(byte[]? bytes)
                 return ByteArrayRef.Empty.Ref;
             }
             var val = new ByteArrayRef(bytes);
-            byteArrayRefs.Add(val);
+            disposables.Add(val);
             return val.Ref;
         }
 
@@ -55,7 +49,7 @@ public Interop.TemporalCoreByteArrayRef ByteArray(string? str)
                 return ByteArrayRef.Empty.Ref;
             }
             var val = ByteArrayRef.FromUTF8(str);
-            byteArrayRefs.Add(val);
+            disposables.Add(val);
             return val.Ref;
         }
 
@@ -67,7 +61,7 @@ public Interop.TemporalCoreByteArrayRef ByteArray(string? str)
         public Interop.TemporalCoreByteArrayRef ByteArray(KeyValuePair<string, string> pair)
         {
             var val = ByteArrayRef.FromKeyValuePair(pair);
-            byteArrayRefs.Add(val);
+            disposables.Add(val);
             return val.Ref;
         }
 
@@ -79,7 +73,7 @@ public Interop.TemporalCoreByteArrayRef ByteArray(KeyValuePair<string, string> p
         public Interop.TemporalCoreByteArrayRef ByteArray(KeyValuePair<string, byte[]> pair)
         {
             var val = ByteArrayRef.FromKeyValuePair(pair);
-            byteArrayRefs.Add(val);
+            disposables.Add(val);
             return val.Ref;
         }
 
@@ -95,7 +89,7 @@ public Interop.TemporalCoreByteArrayRef NewlineDelimited(IReadOnlyCollection<str
                 return ByteArrayRef.Empty.Ref;
             }
             var val = ByteArrayRef.FromNewlineDelimited(values);
-            byteArrayRefs.Add(val);
+            disposables.Add(val);
             return val.Ref;
         }
 
@@ -111,7 +105,7 @@ public Interop.TemporalCoreByteArrayRef NewlineDelimited(IReadOnlyCollection<Key
                 return ByteArrayRef.Empty.Ref;
             }
             var val = ByteArrayRef.FromNewlineDelimited(values);
-            byteArrayRefs.Add(val);
+            disposables.Add(val);
             return val.Ref;
         }
 
@@ -249,34 +243,23 @@ public IntPtr FunctionPointer<T>(T func)
         /// <inheritdoc />
         public void Dispose()
         {
-            Dispose(true);
-            GC.SuppressFinalize(this);
-        }
-
-        private void Dispose(bool disposing)
-        {
+            // Note, we intentionally do not free or call Dispose from a finalizer. When working
+            // with native objects, best practice is to not free in finalizer. On process shutdown,
+            // finalizers can be called even when the object is still referenced.
             if (disposed)
             {
                 return;
             }
-
-            byteArrayRefs.Clear();
-
             foreach (var handle in gcHandles)
             {
                 handle.Free();
             }
             gcHandles.Clear();
-
-            if (disposing)
+            foreach (var disposable in disposables)
             {
-                foreach (var disposable in disposables)
-                {
-                    disposable.Dispose();
-                }
+                disposable.Dispose();
             }
             disposables.Clear();
-
             disposed = true;
         }
     }