GC-related WIP

losttech · lostmsu · Dec 9, 2021 · Oct 24, 2021 · Oct 28, 2021 · Oct 28, 2021
commit 48c0dfc2102bb418a22c9ffbd2016dbfdb3ad551
diff --git a/src/embed_tests/TestRuntime.cs b/src/embed_tests/TestRuntime.cs
@@ -32,6 +32,21 @@ public static void Py_IsInitializedValue()
             Assert.AreEqual(0, Runtime.Runtime.Py_IsInitialized());
         }
 
+        [Test]
+        public static void IterAcrossRuns()
+        {
+            Runtime.Runtime.Py_Initialize();
+            BorrowedReference builtins = Runtime.Runtime.PyEval_GetBuiltins();
+            BorrowedReference iter = Runtime.Runtime.PyDict_GetItemString(builtins, "iter");
+
+            using var ownedIter = new NewReference(iter);
+            Runtime.Runtime.Py_Finalize();
+
+            Runtime.Runtime.Py_Initialize();
+            ownedIter.Dispose();
+            Runtime.Runtime.Py_Finalize();
+        }
+
         [Test]
         public static void RefCountTest()
         {

diff --git a/src/runtime/Util.cs b/src/runtime/Util.cs
@@ -28,58 +28,68 @@ internal static class Util
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal static int ReadInt32(BorrowedReference ob, int offset)
         {
+            Debug.Assert(offset >= 0);
             return Marshal.ReadInt32(ob.DangerousGetAddress(), offset);
         }
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal static long ReadInt64(BorrowedReference ob, int offset)
         {
+            Debug.Assert(offset >= 0);
             return Marshal.ReadInt64(ob.DangerousGetAddress(), offset);
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal unsafe static T* ReadPtr<T>(BorrowedReference ob, int offset)
             where T: unmanaged
         {
+            Debug.Assert(offset >= 0);
             IntPtr ptr = Marshal.ReadIntPtr(ob.DangerousGetAddress(), offset);
             return (T*)ptr;
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal unsafe static IntPtr ReadIntPtr(BorrowedReference ob, int offset)
         {
+            Debug.Assert(offset >= 0);
             return Marshal.ReadIntPtr(ob.DangerousGetAddress(), offset);
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal unsafe static BorrowedReference ReadRef(BorrowedReference @ref, int offset)
         {
+            Debug.Assert(offset >= 0);
             return new BorrowedReference(ReadIntPtr(@ref, offset));
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal static void WriteInt32(BorrowedReference ob, int offset, int value)
         {
+            Debug.Assert(offset >= 0);
             Marshal.WriteInt32(ob.DangerousGetAddress(), offset, value);
         }
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal static void WriteInt64(BorrowedReference ob, int offset, long value)
         {
+            Debug.Assert(offset >= 0);
             Marshal.WriteInt64(ob.DangerousGetAddress(), offset, value);
         }
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal unsafe static void WriteIntPtr(BorrowedReference ob, int offset, IntPtr value)
         {
+            Debug.Assert(offset >= 0);
             Marshal.WriteIntPtr(ob.DangerousGetAddress(), offset, value);
         }
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal unsafe static void WriteRef(BorrowedReference ob, int offset, in StolenReference @ref)
         {
+            Debug.Assert(offset >= 0);
             Marshal.WriteIntPtr(ob.DangerousGetAddress(), offset, @ref.DangerousGetAddress());
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal unsafe static void WriteNullableRef(BorrowedReference ob, int offset, in StolenReference @ref)
         {
+            Debug.Assert(offset >= 0);
             Marshal.WriteIntPtr(ob.DangerousGetAddress(), offset, @ref.DangerousGetAddressOrNull());
         }
 

diff --git a/src/runtime/arrayobject.cs b/src/runtime/arrayobject.cs
@@ -143,7 +143,7 @@ static NewReference NewInstance(Type elementType, BorrowedReference arrayPyType,
         public new static NewReference mp_subscript(BorrowedReference ob, BorrowedReference idx)
         {
             var obj = (CLRObject)GetManagedObject(ob)!;
-            var arrObj = (ArrayObject)GetManagedObjectType(ob)!;
+            var arrObj = (ArrayObject)GetManagedObject(Runtime.PyObject_TYPE(ob))!;
             if (!arrObj.type.Valid)
             {
                 return Exceptions.RaiseTypeError(arrObj.type.DeletedMessage);

diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs
@@ -337,33 +337,37 @@ public static NewReference tp_repr(BorrowedReference ob)
         /// </summary>
         public static void tp_dealloc(NewReference lastRef)
         {
-            GCHandle? gcHandle = TryGetGCHandle(lastRef.Borrow());
+            Runtime.PyGC_ValidateLists();
+            Runtime.PyObject_GC_UnTrack(lastRef.Borrow());
 
-            tp_clear(lastRef.Borrow());
+            CallClear(lastRef.Borrow());
 
             IntPtr addr = lastRef.DangerousGetAddress();
             bool deleted = CLRObject.reflectedObjects.Remove(addr);
             Debug.Assert(deleted);
 
-            Runtime.PyObject_GC_UnTrack(lastRef.Borrow());
-            Runtime.PyObject_GC_Del(lastRef.Steal());
-
-            gcHandle?.Free();
+            DecrefTypeAndFree(lastRef.Steal());
+            Runtime.PyGC_ValidateLists();
         }
 
         public static int tp_clear(BorrowedReference ob)
         {
+            Runtime.PyGC_ValidateLists();
+            GCHandle? gcHandle = TryGetGCHandle(ob);
+            gcHandle?.Free();
+
             int baseClearResult = BaseUnmanagedClear(ob);
             if (baseClearResult != 0)
             {
                 return baseClearResult;
             }
 
             ClearObjectDict(ob);
+            Runtime.PyGC_ValidateLists();
             return 0;
         }
 
-        static unsafe int BaseUnmanagedClear(BorrowedReference ob)
+        internal static unsafe int BaseUnmanagedClear(BorrowedReference ob)
         {
             var type = Runtime.PyObject_TYPE(ob);
             var unmanagedBase = GetUnmanagedBaseType(type);
@@ -374,10 +378,10 @@ static unsafe int BaseUnmanagedClear(BorrowedReference ob)
             }
             var clear = (delegate* unmanaged[Cdecl]<BorrowedReference, int>)clearPtr;
 
-            bool usesSubtypeClear = clearPtr == Util.ReadIntPtr(Runtime.CLRMetaType, TypeOffset.tp_clear);
+            bool usesSubtypeClear = clearPtr == TypeManager.subtype_clear;
             if (usesSubtypeClear)
             {
-                // workaround for https://bugs.python.org/issue45266
+                // workaround for https://bugs.python.org/issue45266 (subtype_clear)
                 using var dict = Runtime.PyObject_GenericGetDict(ob);
                 if (Runtime.PyMapping_HasKey(dict.Borrow(), PyIdentifier.__clear_reentry_guard__) != 0)
                     return 0;

diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs
@@ -1,13 +1,11 @@
 using System;
-using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.ComponentModel;
 using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
 using System.Reflection.Emit;
 using System.Runtime.InteropServices;
-using System.Threading;
 
 using Python.Runtime.Native;
 
@@ -71,6 +69,7 @@ internal ClassDerivedObject(Type tp) : base(tp)
 
         public new static void tp_dealloc(NewReference ob)
         {
+            Runtime.PyGC_ValidateLists();
             var self = (CLRObject)GetManagedObject(ob.Borrow())!;
 
             // don't let the python GC destroy this object
@@ -84,6 +83,7 @@ internal ClassDerivedObject(Type tp) : base(tp)
             GCHandle gc = GCHandle.Alloc(self, GCHandleType.Weak);
             SetGCHandle(ob.Borrow(), gc);
             oldHandle.Free();
+            Runtime.PyGC_ValidateLists();
         }
 
         /// <summary>
@@ -800,6 +800,8 @@ public static void InvokeSetProperty<T>(IPythonDerivedType obj, string propertyN
 
         public static void InvokeCtor(IPythonDerivedType obj, string origCtorName, object[] args)
         {
+            Debug.Assert(Runtime.PyGILState_Check() != 0);
+
             // call the base constructor
             obj.GetType().InvokeMember(origCtorName,
                 BindingFlags.InvokeMethod,
@@ -833,58 +835,12 @@ public static void InvokeCtor(IPythonDerivedType obj, string origCtorName, objec
             }
         }
 
-        static readonly ConcurrentQueue<IntPtr> finalizeQueue = new();
-        static readonly Lazy<Thread> derivedFinalizer = new(() =>
-        {
-           var thread = new Thread(DerivedFinalizerMain)
-           {
-               IsBackground = true,
-           };
-           thread.Start();
-           return thread;
-        }, LazyThreadSafetyMode.ExecutionAndPublication);
-
-        static void DerivedFinalizerMain()
-        {
-            while (true)
-            {
-                if (0 == Runtime.Py_IsInitialized())
-                {
-                    Thread.Sleep(millisecondsTimeout: 1000);
-                }
-
-                PyGILState gs = Runtime.PyGILState_Ensure();
-                try
-                {
-                    while (finalizeQueue.Count > 0)
-                    {
-                        finalizeQueue.TryDequeue(out IntPtr obj);
-                        var @ref = new BorrowedReference(obj);
-                        GCHandle gcHandle = ManagedType.GetGCHandle(@ref);
-
-                        bool deleted = CLRObject.reflectedObjects.Remove(obj);
-                        Debug.Assert(deleted);
-                        Runtime.PyObject_GC_Del(@ref);
-
-                        gcHandle.Free();
-                    }
-
-                }
-                finally
-                {
-                    Runtime.PyGILState_Release(gs);
-                }
-            }
-        }
         public static void PyFinalize(IPythonDerivedType obj)
         {
             // the C# object is being destroyed which must mean there are no more
             // references to the Python object as well
             var self = GetPyObj(obj).DangerousGetAddress();
-            finalizeQueue.Enqueue(self);
-            SetPyObj(obj, null);
-
-            GC.KeepAlive(derivedFinalizer.Value);
+            Finalizer.Instance.AddDerivedFinalizedObject(ref self);
         }
 
         internal static BorrowedReference GetPyObj(IPythonDerivedType obj)

diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs
@@ -63,23 +63,6 @@ internal static void RemoveClasses()
             cache.Clear();
         }
 
-        private static int TraverseTypeClear(BorrowedReference ob, IntPtr arg)
-        {
-            var visited = (HashSet<IntPtr>)GCHandle.FromIntPtr(arg).Target;
-            if (!visited.Add(ob.DangerousGetAddressOrNull()))
-            {
-                return 0;
-            }
-            var clrObj = ManagedType.GetManagedObject(ob);
-            if (clrObj != null)
-            {
-                BorrowedReference tp = Runtime.PyObject_TYPE(ob);
-                ManagedType.CallTypeTraverse(ob, tp, TraverseTypeClear, arg);
-                ManagedType.CallTypeClear(ob, tp);
-            }
-            return 0;
-        }
-
         internal static ClassManagerState SaveRuntimeData()
         {
             var contexts = new Dictionary<ReflectedClrType, InterDomainContext>();

diff --git a/src/runtime/constructorbinding.cs b/src/runtime/constructorbinding.cs
@@ -147,24 +147,15 @@ public static NewReference tp_repr(BorrowedReference ob)
             return new NewReference(self.repr);
         }
 
-        protected override void Clear(BorrowedReference ob)
-        {
-            Runtime.Py_CLEAR(ref this.repr);
-            base.Clear(ob);
-        }
-
         public static int tp_traverse(BorrowedReference ob, IntPtr visit, IntPtr arg)
         {
-            var self = (ConstructorBinding)GetManagedObject(ob)!;
-            int res = PyVisit(self.typeToCreate, visit, arg);
-            if (res != 0) return res;
+            var self = (ConstructorBinding?)GetManagedObject(ob);
+            if (self is null) return 0;
 
-            if (self.repr is not null)
-            {
-                res = PyVisit(self.repr, visit, arg);
-                if (res != 0) return res;
-            }
-            return 0;
+            Runtime.PyGC_ValidateLists();
+            int res = PyVisit(self.typeToCreate, visit, arg);
+            Runtime.PyGC_ValidateLists();
+            return res;
         }
     }
 
@@ -241,24 +232,15 @@ public static NewReference tp_repr(BorrowedReference ob)
             return new NewReference(self.repr);
         }
 
-        protected override void Clear(BorrowedReference ob)
-        {
-            Runtime.Py_CLEAR(ref this.repr);
-            base.Clear(ob);
-        }
-
         public static int tp_traverse(BorrowedReference ob, IntPtr visit, IntPtr arg)
         {
-            var self = (BoundContructor)GetManagedObject(ob)!;
-            int res = PyVisit(self.typeToCreate, visit, arg);
-            if (res != 0) return res;
+            var self = (BoundContructor?)GetManagedObject(ob);
+            if (self is null) return 0;
 
-            if (self.repr is not null)
-            {
-                res = PyVisit(self.repr, visit, arg);
-                if (res != 0) return res;
-            }
-            return 0;
+            Runtime.PyGC_ValidateLists();
+            int res = PyVisit(self.typeToCreate, visit, arg);
+            Runtime.PyGC_ValidateLists();
+            return res;
         }
     }
 }