Make extension/CLR-object registries thread-safe

greateggsgreg · filmor · commit 441954f7e589 · 2026-05-15T22:21:02.000+02:00
ExtensionType.loadedExtensions and CLRObject.reflectedObjects are
"borrowed reference" registries written on every alloc and read or
removed from finalizer-thread paths.  Under free-threaded Python the
plain HashSet&lt;IntPtr&gt; tears reliably; under the GIL the same tears
were happening more rarely but still mostly observable as Debug.Assert
firings during shutdown.

Convert both to ConcurrentDictionary&lt;IntPtr, byte&gt; with the equivalent
TryAdd/TryRemove operations, and update the few non-mutating callers
(NullGCHandles, RuntimeData snapshot LINQ) to enumerate Keys.
diff --git a/src/runtime/StateSerialization/RuntimeData.cs b/src/runtime/StateSerialization/RuntimeData.cs
@@ -178,7 +178,7 @@ private static SharedObjectsState SaveRuntimeDataObjects()
             var contexts = new Dictionary<PyObject, Dictionary<string, object?>>(PythonReferenceComparer.Instance);
             var extensionObjs = new Dictionary<PyObject, ExtensionType>(PythonReferenceComparer.Instance);
             // make a copy with strongly typed references to avoid concurrent modification
-            var extensions = ExtensionType.loadedExtensions
+            var extensions = ExtensionType.loadedExtensions.Keys
                                 .Select(addr => new PyObject(
                                     new BorrowedReference(addr),
                                     // if we don't skip collect, finalizer might modify loadedExtensions
@@ -199,7 +199,7 @@ private static SharedObjectsState SaveRuntimeDataObjects()
             var wrappers = new Dictionary<object, List<CLRObject>>();
             var userObjects = new CLRWrapperCollection();
             // make a copy with strongly typed references to avoid concurrent modification
-            var reflectedObjects = CLRObject.reflectedObjects
+            var reflectedObjects = CLRObject.reflectedObjects.Keys
                                     .Select(addr => new PyObject(
                                         new BorrowedReference(addr),
                                         // if we don't skip collect, finalizer might modify reflectedObjects
diff --git a/src/runtime/Types/ClassBase.cs b/src/runtime/Types/ClassBase.cs
@@ -360,7 +360,7 @@ public static int tp_clear(BorrowedReference ob)
             if (TryFreeGCHandle(ob))
             {
                 IntPtr addr = ob.DangerousGetAddress();
-                bool deleted = CLRObject.reflectedObjects.Remove(addr);
+                bool deleted = CLRObject.reflectedObjects.TryRemove(addr, out _);
                 Debug.Assert(deleted);
             }
 
diff --git a/src/runtime/Types/ClrObject.cs b/src/runtime/Types/ClrObject.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Runtime.InteropServices;
@@ -13,8 +14,8 @@ internal sealed class CLRObject : ManagedType
 
         internal static bool creationBlocked = false;
 
-        // "borrowed" references
-        internal static readonly HashSet<IntPtr> reflectedObjects = new();
+        // "borrowed" references; thread-safe (see ExtensionType.loadedExtensions).
+        internal static readonly ConcurrentDictionary<IntPtr, byte> reflectedObjects = new();
         static NewReference Create(object ob, BorrowedReference tp)
         {
             if (creationBlocked)
@@ -28,7 +29,7 @@ static NewReference Create(object ob, BorrowedReference tp)
             GCHandle gc = GCHandle.Alloc(self);
             InitGCHandle(py.Borrow(), type: tp, gc);
 
-            bool isNew = reflectedObjects.Add(py.DangerousGetAddress());
+            bool isNew = reflectedObjects.TryAdd(py.DangerousGetAddress(), 0);
             Debug.Assert(isNew);
 
             // Fix the BaseException args (and __cause__ in case of Python 3)
@@ -73,7 +74,7 @@ protected override void OnLoad(BorrowedReference ob, Dictionary<string, object?>
             GCHandle gc = GCHandle.Alloc(this);
             SetGCHandle(ob, gc);
 
-            bool isNew = reflectedObjects.Add(ob.DangerousGetAddress());
+            bool isNew = reflectedObjects.TryAdd(ob.DangerousGetAddress(), 0);
             Debug.Assert(isNew);
         }
     }
diff --git a/src/runtime/Types/ExtensionType.cs b/src/runtime/Types/ExtensionType.cs
@@ -42,14 +42,15 @@ public virtual NewReference Alloc()
 
         public PyObject AllocObject() => new(Alloc().Steal());
 
-        // "borrowed" references
-        internal static readonly HashSet<IntPtr> loadedExtensions = new();
+        // "borrowed" references; thread-safe to survive free-threaded Python and
+        // .NET finalizer-thread races.
+        internal static readonly System.Collections.Concurrent.ConcurrentDictionary<IntPtr, byte> loadedExtensions = new();
         void SetupGc (BorrowedReference ob, BorrowedReference tp)
         {
             GCHandle gc = GCHandle.Alloc(this);
             InitGCHandle(ob, tp, gc);
 
-            bool isNew = loadedExtensions.Add(ob.DangerousGetAddress());
+            bool isNew = loadedExtensions.TryAdd(ob.DangerousGetAddress(), 0);
             Debug.Assert(isNew);
 
             // We have to support gc because the type machinery makes it very
@@ -104,7 +105,7 @@ public static int tp_clear(BorrowedReference ob)
 
             if (TryFreeGCHandle(ob))
             {
-                bool deleted = loadedExtensions.Remove(ob.DangerousGetAddress());
+                bool deleted = loadedExtensions.TryRemove(ob.DangerousGetAddress(), out _);
                 Debug.Assert(deleted);
             }
 

Original file line number	Diff line number	Diff line change
`@@ -360,7 +360,7 @@ public static int tp_clear(BorrowedReference ob)`
`360`	`360`	`if (TryFreeGCHandle(ob))`
`361`	`361`	`{`
`362`	`362`	`IntPtr addr = ob.DangerousGetAddress();`
`363`		`- bool deleted = CLRObject.reflectedObjects.Remove(addr);`
	`363`	`+ bool deleted = CLRObject.reflectedObjects.TryRemove(addr, out _);`
`364`	`364`	`Debug.Assert(deleted);`
`365`	`365`	`}`
`366`	`366`