fixed sending PyObject across domain boundary

removed debug code, that ensured Python GC list integrity
losttech · lostmsu · Dec 9, 2021 · Oct 24, 2021 · Oct 28, 2021 · Oct 28, 2021
commit a8ef06c5ff76573a402ee53d2a3f56a4799d3ec2
diff --git a/src/embed_tests/TestRuntime.cs b/src/embed_tests/TestRuntime.cs
@@ -32,21 +32,6 @@ 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/ReflectedClrType.cs b/src/runtime/ReflectedClrType.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Diagnostics;
+using System.Runtime.Serialization;
 
 using static Python.Runtime.PythonException;
 
@@ -10,6 +11,7 @@ internal sealed class ReflectedClrType : PyType
 {
     private ReflectedClrType(StolenReference reference) : base(reference, prevalidated: true) { }
     internal ReflectedClrType(ReflectedClrType original) : base(original, prevalidated: true) { }
+    ReflectedClrType(SerializationInfo info, StreamingContext context) : base(info, context) { }
 
     internal ClassBase Impl => (ClassBase)ManagedType.GetManagedObject(this)!;
 

diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs
@@ -337,7 +337,6 @@ public static NewReference tp_repr(BorrowedReference ob)
         /// </summary>
         public static void tp_dealloc(NewReference lastRef)
         {
-            Runtime.PyGC_ValidateLists();
             Runtime.PyObject_GC_UnTrack(lastRef.Borrow());
 
             CallClear(lastRef.Borrow());
@@ -347,12 +346,10 @@ public static void tp_dealloc(NewReference lastRef)
             Debug.Assert(deleted);
 
             DecrefTypeAndFree(lastRef.Steal());
-            Runtime.PyGC_ValidateLists();
         }
 
         public static int tp_clear(BorrowedReference ob)
         {
-            Runtime.PyGC_ValidateLists();
             GCHandle? gcHandle = TryGetGCHandle(ob);
             gcHandle?.Free();
 
@@ -363,7 +360,6 @@ public static int tp_clear(BorrowedReference ob)
             }
 
             ClearObjectDict(ob);
-            Runtime.PyGC_ValidateLists();
             return 0;
         }
 

diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs
@@ -69,7 +69,6 @@ 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
@@ -83,7 +82,6 @@ internal ClassDerivedObject(Type tp) : base(tp)
             GCHandle gc = GCHandle.Alloc(self, GCHandleType.Weak);
             SetGCHandle(ob.Borrow(), gc);
             oldHandle.Free();
-            Runtime.PyGC_ValidateLists();
         }
 
         /// <summary>

diff --git a/src/runtime/constructorbinding.cs b/src/runtime/constructorbinding.cs
@@ -152,9 +152,7 @@ public static int tp_traverse(BorrowedReference ob, IntPtr visit, IntPtr arg)
             var self = (ConstructorBinding?)GetManagedObject(ob);
             if (self is null) return 0;
 
-            Runtime.PyGC_ValidateLists();
             int res = PyVisit(self.typeToCreate, visit, arg);
-            Runtime.PyGC_ValidateLists();
             return res;
         }
     }
@@ -237,9 +235,7 @@ public static int tp_traverse(BorrowedReference ob, IntPtr visit, IntPtr arg)
             var self = (BoundContructor?)GetManagedObject(ob);
             if (self is null) return 0;
 
-            Runtime.PyGC_ValidateLists();
             int res = PyVisit(self.typeToCreate, visit, arg);
-            Runtime.PyGC_ValidateLists();
             return res;
         }
     }

diff --git a/src/runtime/extensiontype.cs b/src/runtime/extensiontype.cs
@@ -76,7 +76,6 @@ public static int tp_setattro(BorrowedReference ob, BorrowedReference key, Borro
 
         public unsafe static void tp_dealloc(NewReference lastRef)
         {
-            Runtime.PyGC_ValidateLists();
             Runtime.PyObject_GC_UnTrack(lastRef.Borrow());
 
             tp_clear(lastRef.Borrow());
@@ -86,18 +85,15 @@ public unsafe static void tp_dealloc(NewReference lastRef)
 
             // we must decref our type: https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_dealloc
             DecrefTypeAndFree(lastRef.Steal());
-            Runtime.PyGC_ValidateLists();
         }
 
         public static int tp_clear(BorrowedReference ob)
         {
-            Runtime.PyGC_ValidateLists();
             GCHandle? gcHandle = TryGetGCHandle(ob);
             gcHandle?.Free();
             if (gcHandle is not null) SetGCHandle(ob, default);
 
             int res = ClassBase.BaseUnmanagedClear(ob);
-            Runtime.PyGC_ValidateLists();
             return res;
         }
 

diff --git a/src/runtime/finalizer.cs b/src/runtime/finalizer.cs
@@ -122,19 +122,30 @@ internal List<IntPtr> GetCollectedObjects()
             return _objQueue.Select(o => o.PyObj).ToList();
         }
 
-        internal void AddFinalizedObject(ref IntPtr obj, int run)
+        internal void AddFinalizedObject(ref IntPtr obj, int run
+#if TRACE_ALLOC
+                                         , StackTrace stackTrace
+#endif
+        )
         {
             Debug.Assert(obj != IntPtr.Zero);
             if (!Enable)
             {
                 return;
             }
 
+            Debug.Assert(Runtime.Refcount(new BorrowedReference(obj)) > 0);
+
 #if FINALIZER_CHECK
             lock (_queueLock)
 #endif
             {
-                this._objQueue.Enqueue(new PendingFinalization { PyObj = obj, RuntimeRun = run });
+                this._objQueue.Enqueue(new PendingFinalization {
+                    PyObj = obj, RuntimeRun = run,
+#if TRACE_ALLOC
+                    StackTrace = stackTrace.ToString(),
+#endif
+                });
             }
             obj = IntPtr.Zero;
         }
@@ -165,10 +176,12 @@ internal static void Shutdown()
             Instance.started = false;
         }
 
-        private void DisposeAll()
+        internal nint DisposeAll()
         {
             if (_objQueue.IsEmpty && _derivedQueue.IsEmpty)
-                return;
+                return 0;
+
+            nint collected = 0;
 
             BeforeCollect?.Invoke(this, new CollectArgs()
             {
@@ -200,16 +213,8 @@ private void DisposeAll()
                         }
 
                         IntPtr copyForException = obj.PyObj;
-                        Runtime.PyGC_ValidateLists();
-                        var @ref = new BorrowedReference(obj.PyObj);
-                        nint refs = Runtime.Refcount(@ref);
-                        var type = Runtime.PyObject_TYPE(@ref);
-                        string typeName = Runtime.ToString(type);
-                        if (typeName == "<class 'clr.interop.PyErr'>")
-                        {
-
-                        }
                         Runtime.XDecref(StolenReference.Take(ref obj.PyObj));
+                        collected++;
                         try
                         {
                             Runtime.CheckExceptionOccurred();
@@ -218,7 +223,6 @@ private void DisposeAll()
                         {
                             HandleFinalizationException(obj.PyObj, e);
                         }
-                        Runtime.PyGC_ValidateLists();
                     }
 
                     while (!_derivedQueue.IsEmpty)
@@ -241,6 +245,7 @@ private void DisposeAll()
                         // matches correspdonging PyObject_GC_UnTrack
                         // in ClassDerivedObject.tp_dealloc
                         Runtime.PyObject_GC_Del(@ref.Steal());
+                        collected++;
 
                         gcHandle.Free();
                     }
@@ -252,6 +257,7 @@ private void DisposeAll()
                     Runtime.PyErr_Restore(errType.StealNullable(), errVal.StealNullable(), traceback.StealNullable());
                 }
             }
+            return collected;
         }
 
         void HandleFinalizationException(IntPtr obj, Exception cause)
@@ -341,6 +347,9 @@ struct PendingFinalization
     {
         public IntPtr PyObj;
         public int RuntimeRun;
+#if TRACE_ALLOC
+        public string StackTrace;
+#endif
     }
 
     public class FinalizationException : Exception

diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs
@@ -290,7 +290,6 @@ public static NewReference mp_subscript(BorrowedReference tp, BorrowedReference
         /// </summary>
         public static void tp_dealloc(NewReference lastRef)
         {
-            Runtime.PyGC_ValidateLists();
             // Fix this when we dont cheat on the handle for subclasses!
 
             var flags = PyType.GetFlags(lastRef.Borrow());
@@ -318,7 +317,6 @@ public static void tp_dealloc(NewReference lastRef)
             // We must decref our type.
             // type_dealloc from PyType will use it to get tp_free so we must keep the value
             Runtime.XDecref(StolenReference.DangerousFromPointer(op.DangerousGetAddress()));
-            Runtime.PyGC_ValidateLists();
         }
 
         private static NewReference DoInstanceCheck(BorrowedReference tp, BorrowedReference args, bool checkType)

diff --git a/src/runtime/module.cs b/src/runtime/module.cs
@@ -2,6 +2,7 @@
 using System.Linq;
 using System.Collections.Generic;
 using System.Dynamic;
+using System.Runtime.Serialization;
 
 namespace Python.Runtime
 {
@@ -42,6 +43,9 @@ internal PyModule(in StolenReference reference) : base(reference)
             }
         }
 
+        protected PyModule(SerializationInfo info, StreamingContext context)
+            : base(info, context) { }
+
         private void InitializeBuiltins()
         {
             int res = Runtime.PyDict_SetItem(

diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs
@@ -323,16 +323,12 @@ public static int tp_traverse(BorrowedReference ob, IntPtr visit, IntPtr arg)
             var self = (ModuleObject?)GetManagedObject(ob);
             if (self is null) return 0;
 
-            Runtime.PyGC_ValidateLists();
             Debug.Assert(self.dict == GetObjectDict(ob));
             int res = PyVisit(self.dict, visit, arg);
-            Runtime.PyGC_ValidateLists();
             if (res != 0) return res;
             foreach (var attr in self.cache.Values)
             {
-                Runtime.PyGC_ValidateLists();
                 res = PyVisit(attr, visit, arg);
-                Runtime.PyGC_ValidateLists();
                 if (res != 0) return res;
             }
             return 0;

diff --git a/src/runtime/pybuffer.cs b/src/runtime/pybuffer.cs
@@ -240,7 +240,11 @@ private void Dispose(bool disposing)
 
             if (_view.obj != IntPtr.Zero)
             {
-                Finalizer.Instance.AddFinalizedObject(ref _view.obj, _exporter.run);
+                Finalizer.Instance.AddFinalizedObject(ref _view.obj, _exporter.run
+#if TRACE_ALLOC
+                    , _exporter.Traceback
+#endif
+                );
             }
 
             Dispose(false);

diff --git a/src/runtime/pydict.cs b/src/runtime/pydict.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Runtime.Serialization;
 
 namespace Python.Runtime
 {
@@ -33,6 +34,9 @@ public PyDict() : base(Runtime.PyDict_New().StealOrThrow()) { }
             }
         }
 
+        protected PyDict(SerializationInfo info, StreamingContext context)
+            : base(info, context) { }
+
 
         /// <summary>
         /// IsDictType Method

diff --git a/src/runtime/pyfloat.cs b/src/runtime/pyfloat.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Runtime.Serialization;
 
 namespace Python.Runtime
 {
@@ -70,6 +71,9 @@ public PyFloat(string value) : base(FromString(value))
         {
         }
 
+        protected PyFloat(SerializationInfo info, StreamingContext context)
+            : base(info, context) { }
+
 
         /// <summary>
         /// IsFloatType Method

diff --git a/src/runtime/pyint.cs b/src/runtime/pyint.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Runtime.Serialization;
 
 namespace Python.Runtime
 {
@@ -134,6 +135,9 @@ public PyInt(string value) : base(Runtime.PyLong_FromString(value, 0).StealOrThr
         {
         }
 
+        protected PyInt(SerializationInfo info, StreamingContext context)
+            : base(info, context) { }
+
 
         /// <summary>
         /// IsIntType Method

diff --git a/src/runtime/pyiter.cs b/src/runtime/pyiter.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Runtime.Serialization;
 
 namespace Python.Runtime
 {
@@ -89,5 +90,17 @@ public void Reset()
 
         public PyObject Current => _current ?? throw new InvalidOperationException();
         object System.Collections.IEnumerator.Current => Current;
+
+        protected PyIter(SerializationInfo info, StreamingContext context)
+            : base(info, context)
+        {
+            _current = (PyObject?)info.GetValue("c", typeof(PyObject));
+        }
+
+        protected override void GetObjectData(SerializationInfo info, StreamingContext context)
+        {
+            base.GetObjectData(info, context);
+            info.AddValue("c", _current);
+        }
     }
 }
diff --git a/src/runtime/pyiterable.cs b/src/runtime/pyiterable.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Runtime.Serialization;
 
 namespace Python.Runtime
 {
@@ -9,6 +10,8 @@ public class PyIterable : PyObject, IEnumerable<PyObject>
     {
         internal PyIterable(BorrowedReference reference) : base(reference) { }
         internal PyIterable(in StolenReference reference) : base(reference) { }
+        protected PyIterable(SerializationInfo info, StreamingContext context)
+            : base(info, context) { }
 
         /// <summary>
         /// Creates new instance from an existing object.