Document lock acquisition sites and strong->weak GCHandle swap

Comment-only changes. Adds inline notes at lock acquisitions where the "why" is not obvious from the field declaration alone: - ClassDerivedObject.Reset / GetModuleBuilder: explain that both builder caches must update atomically and that DefineDynamicAssembly / DefineDynamicModule produce duplicates under contention. - TypeManager.GetType: note that CreateType + cache write must be atomic. - ReflectedClrType.GetOrCreate: cross-file lock; mention it also serialises ClassManager.cache and TypeManager._slotsHolders writes. - Runtime.ResetPyMembers: explain the snapshot-then-dispose pattern (Dispose() callbacks would deadlock if invoked under the lock). Expands the strong->weak GCHandle swap in ClassDerivedObject.tp_dealloc to spell out: 1. Why the PyObject is not freed at refcount 0 (C# wrapper may still reference it; ToPython() resurrects via _Py_NewReference). 2. Why the handle is demoted to weak (lets the C# wrapper be GC'd; on collection PyFinalize enqueues the real PyObject_GC_Del). 3. Why the swap uses Interlocked.Exchange (tp_clear may race on the same slot under FT / finalizer thread; without atomic claim both threads could observe and double-free the same handle).
pythonnet · 6b1f3f4c862901c0724a65006dc10fe2617aced8
commit 6b1f3f4c862901c0724a65006dc10fe2617aced8
diff --git a/src/runtime/Runtime.cs b/src/runtime/Runtime.cs
@@ -414,6 +414,8 @@ private static void SetPyMemberTypeOf(out PyObject obj, StolenReference value)
 
         private static void ResetPyMembers()
         {
+            // Snapshot under lock; Dispose() runs outside it so a callback that
+            // re-enters SetPyMember does not deadlock.
             PyObject[] snapshot;
             lock (_pyRefsLock)
             {

diff --git a/src/runtime/TypeManager.cs b/src/runtime/TypeManager.cs
@@ -283,6 +283,7 @@ internal static PyType GetType(Type type)
         {
             // Cached with refcount 1; effectively lives until the CPython runtime is finalised.
             if (cache.TryGetValue(type, out var pyType)) return pyType;
+            // CreateType + cache write must be atomic so two threads do not both allocate.
             lock (_cacheCreateLock)
             {
                 if (cache.TryGetValue(type, out pyType)) return pyType;

diff --git a/src/runtime/Types/ClassDerived.cs b/src/runtime/Types/ClassDerived.cs
@@ -44,6 +44,8 @@ static ClassDerivedObject()
 
         public static void Reset()
         {
+            // Atomic replacement of both builder caches so a concurrent
+            // GetModuleBuilder cannot observe one new + one old.
             lock (_buildersLock)
             {
                 assemblyBuilders = new Dictionary<string, AssemblyBuilder>();
@@ -84,7 +86,21 @@ protected override void SetTypeNewSlot(BorrowedReference pyType, SlotsHolder slo
             // self may be null after Shutdown begun
             if (self is not null)
             {
-                // Atomically swap strong->weak; concurrent tp_clear may also clear the slot.
+                // Python refcount has reached 0 but we do NOT free the PyObject:
+                // the C# wrapper (via IPythonDerivedType.__pyobj__) may still
+                // reference it, and a later ToPython() on that wrapper will
+                // resurrect a strong handle and call _Py_NewReference.
+                //
+                // Demote the GCHandle from strong -> weak so the C# wrapper can
+                // be collected; when it is, PyFinalize enqueues the actual
+                // PyObject_GC_Del.  Until then the slot keeps the wrapper
+                // reachable from Python -> C# but not the other way.
+                //
+                // Interlocked.Exchange: under FT (or .NET-finalizer-thread
+                // races) tp_clear can hit the same slot.  Whichever thread
+                // observes the original handle frees it; the loser frees the
+                // weak handle it just allocated.  Without the atomic swap both
+                // threads could see and free the same handle (double-free).
                 GCHandle weak = GCHandle.Alloc(self, GCHandleType.Weak);
                 BorrowedReference borrow = ob.Borrow();
                 int offset = Util.ReadInt32(Runtime.PyObject_TYPE(borrow), Offsets.tp_clr_inst_offset);
@@ -715,6 +731,8 @@ private static void AddPythonProperty(string propertyName, PyObject func, TypeBu
         private static ModuleBuilder GetModuleBuilder(string assemblyName, string moduleName)
         {
             var key = Tuple.Create(assemblyName, moduleName);
+            // Cache check-and-create must be atomic; DefineDynamicAssembly /
+            // DefineDynamicModule produce duplicate builders under contention.
             lock (_buildersLock)
             {
                 if (moduleBuilders.TryGetValue(key, out ModuleBuilder? existing))

diff --git a/src/runtime/Types/ReflectedClrType.cs b/src/runtime/Types/ReflectedClrType.cs
@@ -29,6 +29,8 @@ public static ReflectedClrType GetOrCreate(Type type)
         if (ClassManager.cache.TryGetValue(type, out var pyType))
             return pyType;
 
+        // Shared with ClassManager.cache + TypeManager._slotsHolders writes
+        // so the multi-step type build below is atomic.
         lock (ClassManager._cacheCreateLock)
         {
             // Re-check now that we hold the lock; another thread may have finished.