Snapshot pypath, use ConcurrentDictionary for thunks and slot holders

greateggsgreg · filmor · commit be5ba3473218 · 2026-05-15T22:21:02.000+02:00
diff --git a/src/runtime/AssemblyManager.cs b/src/runtime/AssemblyManager.cs
@@ -37,7 +37,9 @@ internal class AssemblyManager
 
         // modified from event handlers below, potentially triggered from different .NET threads
         private static readonly ConcurrentQueue<Assembly> assemblies = new();
-        internal static readonly List<string> pypath = new (capacity: 16);
+        // Snapshot; UpdatePath swaps under lock, readers iterate the captured ref.
+        internal static volatile IReadOnlyList<string> pypath = Array.Empty<string>();
+        private static readonly object _pypathLock = new();
         private AssemblyManager()
         {
         }
@@ -49,7 +51,7 @@ private AssemblyManager()
         //github.com/ </summary>
         internal static void Initialize()
         {
-            pypath.Clear();
+            pypath = Array.Empty<string>();
 
             AppDomain domain = AppDomain.CurrentDomain;
 
@@ -154,19 +156,20 @@ internal static void UpdatePath()
         {
             BorrowedReference list = Runtime.PySys_GetObject("path");
             var count = Runtime.PyList_Size(list);
-            if (count != pypath.Count)
+            if (count == pypath.Count) return;
+
+            lock (_pypathLock)
             {
-                pypath.Clear();
+                if (count == pypath.Count) return;
+                var fresh = new List<string>(checked((int)count));
                 probed.Clear();
                 for (var i = 0; i < count; i++)
                 {
                     BorrowedReference item = Runtime.PyList_GetItem(list, i);
                     string? path = Runtime.GetManagedString(item);
-                    if (path != null)
-                    {
-                        pypath.Add(path);
-                    }
+                    if (path != null) fresh.Add(path);
                 }
+                pypath = fresh;
             }
         }
 
@@ -196,7 +199,8 @@ public static string FindAssembly(string name)
 
         static IEnumerable<string> FindAssemblyCandidates(string name)
         {
-            foreach (string head in pypath)
+            var paths = pypath;
+            foreach (string head in paths)
             {
                 string path;
                 if (head == null || head.Length == 0)
diff --git a/src/runtime/Finalizer.cs b/src/runtime/Finalizer.cs
@@ -115,7 +115,8 @@ internal void ThrottledCollect()
             if (!started) throw new InvalidOperationException($"{nameof(PythonEngine)} is not initialized");
 
             if (!Enable || Interlocked.Increment(ref _throttled) < Threshold) return;
-            // Stale pointers on the queue would crash Py_DecRef during teardown.
+            // Defends against externally-driven Py_Finalize (e.g. host's atexit):
+            // queue pointers may already be freed, so skip the drain.
             if (Runtime._Py_IsFinalizing() == true) return;
             Interlocked.Exchange(ref _throttled, 0);
             this.Collect();
@@ -138,7 +139,7 @@ internal void AddFinalizedObject(ref IntPtr obj, int run
                 return;
             }
 
-            // Skip on FT: stale ob_ref_local read from the finalizer thread can crash.
+            // Skip on FT: the split refcount can race here and trip the assert spuriously.
             if (!Native.ABI.IsFreeThreaded)
             {
                 Debug.Assert(Runtime.Refcount(new BorrowedReference(obj)) > 0);
diff --git a/src/runtime/Interop.cs b/src/runtime/Interop.cs
@@ -141,7 +141,9 @@ internal static Type GetPrototype(MethodInfo method)
         }
 
 
-        internal static Dictionary<IntPtr, Delegate> allocatedThunks = new();
+        // Concurrent: documents the multi-writer contract previously enforced
+        // by callers happening to hold TypeManager._cacheCreateLock.
+        internal static ConcurrentDictionary<IntPtr, Delegate> allocatedThunks = new();
 
         internal static ThunkInfo GetThunk(MethodInfo method)
         {
diff --git a/src/runtime/Runtime.cs b/src/runtime/Runtime.cs
@@ -617,7 +617,7 @@ internal static unsafe void XIncref(BorrowedReference op)
         internal static unsafe void XDecref(StolenReference op)
         {
 #if DEBUG
-            // Racy on FT: stale ob_ref_local read may crash.
+            // Skip on FT: the split refcount can race here and trip the assert spuriously.
             Debug.Assert(op == null || Native.ABI.IsFreeThreaded || Refcount(new BorrowedReference(op.Pointer)) > 0);
             Debug.Assert(_isInitialized || Py_IsInitialized() != 0 || _Py_IsFinalizing() != false);
 #endif
diff --git a/src/runtime/TypeManager.cs b/src/runtime/TypeManager.cs
@@ -33,7 +33,9 @@ internal class TypeManager
         internal static readonly ConcurrentDictionary<MaybeType, PyType> cache = new();
         internal static readonly object _cacheCreateLock = new();
 
-        static readonly Dictionary<PyType, SlotsHolder> _slotsHolders = new(PythonReferenceComparer.Instance);
+        // Concurrent: documents the multi-writer contract previously enforced
+        // by callers happening to hold _cacheCreateLock.
+        static readonly ConcurrentDictionary<PyType, SlotsHolder> _slotsHolders = new(PythonReferenceComparer.Instance);
 
         // Slots which must be set
         private static readonly string[] _requiredSlots = new string[]
@@ -949,7 +951,7 @@ internal static SlotsHolder CreateSlotsHolder(PyType type)
         {
             type = new PyType(type);
             var holder = new SlotsHolder(type);
-            _slotsHolders.Add(type, holder);
+            _slotsHolders[type] = holder;
             return holder;
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,9 @@ internal class AssemblyManager`
`37`	`37`
`38`	`38`	`// modified from event handlers below, potentially triggered from different .NET threads`
`39`	`39`	`private static readonly ConcurrentQueue<Assembly> assemblies = new();`
`40`		`- internal static readonly List<string> pypath = new (capacity: 16);`
	`40`	`+ // Snapshot; UpdatePath swaps under lock, readers iterate the captured ref.`
	`41`	`+ internal static volatile IReadOnlyList<string> pypath = Array.Empty<string>();`
	`42`	`+ private static readonly object _pypathLock = new();`
`41`	`43`	`private AssemblyManager()`
`42`	`44`	`{`
`43`	`45`	`}`
`@@ -49,7 +51,7 @@ private AssemblyManager()`
`49`	`51`	`//github.com/ </summary>`
`50`	`52`	`internal static void Initialize()`
`51`	`53`	`{`
`52`		`- pypath.Clear();`
	`54`	`+ pypath = Array.Empty<string>();`
`53`	`55`
`54`	`56`	`AppDomain domain = AppDomain.CurrentDomain;`
`55`	`57`
`@@ -154,19 +156,20 @@ internal static void UpdatePath()`
`154`	`156`	`{`
`155`	`157`	`BorrowedReference list = Runtime.PySys_GetObject("path");`
`156`	`158`	`var count = Runtime.PyList_Size(list);`
`157`		`- if (count != pypath.Count)`
	`159`	`+ if (count == pypath.Count) return;`
	`160`	`+`
	`161`	`+ lock (_pypathLock)`
`158`	`162`	`{`
`159`		`- pypath.Clear();`
	`163`	`+ if (count == pypath.Count) return;`
	`164`	`+ var fresh = new List<string>(checked((int)count));`
`160`	`165`	`probed.Clear();`
`161`	`166`	`for (var i = 0; i < count; i++)`
`162`	`167`	`{`
`163`	`168`	`BorrowedReference item = Runtime.PyList_GetItem(list, i);`
`164`	`169`	`string? path = Runtime.GetManagedString(item);`
`165`		`- if (path != null)`
`166`		`- {`
`167`		`- pypath.Add(path);`
`168`		`- }`
	`170`	`+ if (path != null) fresh.Add(path);`
`169`	`171`	`}`
	`172`	`+ pypath = fresh;`
`170`	`173`	`}`
`171`	`174`	`}`
`172`	`175`
`@@ -196,7 +199,8 @@ public static string FindAssembly(string name)`
`196`	`199`
`197`	`200`	`static IEnumerable<string> FindAssemblyCandidates(string name)`
`198`	`201`	`{`
`199`		`- foreach (string head in pypath)`
	`202`	`+ var paths = pypath;`
	`203`	`+ foreach (string head in paths)`
`200`	`204`	`{`
`201`	`205`	`string path;`
`202`	`206`	`if (head == null \|\| head.Length == 0)`
Original file line number	Diff line number	Diff line change
`@@ -141,7 +141,9 @@ internal static Type GetPrototype(MethodInfo method)`
`141`	`141`	`}`
`142`	`142`
`143`	`143`
`144`		`- internal static Dictionary<IntPtr, Delegate> allocatedThunks = new();`
	`144`	`+ // Concurrent: documents the multi-writer contract previously enforced`
	`145`	`+ // by callers happening to hold TypeManager._cacheCreateLock.`
	`146`	`+ internal static ConcurrentDictionary<IntPtr, Delegate> allocatedThunks = new();`
`145`	`147`
`146`	`148`	`internal static ThunkInfo GetThunk(MethodInfo method)`
`147`	`149`	`{`
Original file line number	Diff line number	Diff line change
`@@ -617,7 +617,7 @@ internal static unsafe void XIncref(BorrowedReference op)`
`617`	`617`	`internal static unsafe void XDecref(StolenReference op)`
`618`	`618`	`{`
`619`	`619`	`#if DEBUG`
`620`		`- // Racy on FT: stale ob_ref_local read may crash.`
	`620`	`+ // Skip on FT: the split refcount can race here and trip the assert spuriously.`
`621`	`621`	`Debug.Assert(op == null \|\| Native.ABI.IsFreeThreaded \|\| Refcount(new BorrowedReference(op.Pointer)) > 0);`
`622`	`622`	`Debug.Assert(_isInitialized \|\| Py_IsInitialized() != 0 \|\| _Py_IsFinalizing() != false);`
`623`	`623`	`#endif`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,9 @@ internal class TypeManager`
`33`	`33`	`internal static readonly ConcurrentDictionary<MaybeType, PyType> cache = new();`
`34`	`34`	`internal static readonly object _cacheCreateLock = new();`
`35`	`35`
`36`		`- static readonly Dictionary<PyType, SlotsHolder> _slotsHolders = new(PythonReferenceComparer.Instance);`
	`36`	`+ // Concurrent: documents the multi-writer contract previously enforced`
	`37`	`+ // by callers happening to hold _cacheCreateLock.`
	`38`	`+ static readonly ConcurrentDictionary<PyType, SlotsHolder> _slotsHolders = new(PythonReferenceComparer.Instance);`
`37`	`39`
`38`	`40`	`// Slots which must be set`
`39`	`41`	`private static readonly string[] _requiredSlots = new string[]`
`@@ -949,7 +951,7 @@ internal static SlotsHolder CreateSlotsHolder(PyType type)`
`949`	`951`	`{`
`950`	`952`	`type = new PyType(type);`
`951`	`953`	`var holder = new SlotsHolder(type);`
`952`		`- _slotsHolders.Add(type, holder);`
	`954`	`+ _slotsHolders[type] = holder;`
`953`	`955`	`return holder;`
`954`	`956`	`}`
`955`	`957`	`}`