call tp_clear (if any) of base unmanaged type when a reflected CLR object instance is cleared

lostmsu · lostmsu · commit 2ef6b506e55c · 2021-09-14T12:18:22.000-07:00
fixes #1476 : C# exceptions inherit from Python BaseException class, which has tp_clear. Prior to this change it was not called, leaving dangling references to BaseException.args and BaseException.__cause__.call tp_clear of base unmanaged type when a reflected CLR object instance is cleared fixes #1476 : C# exceptions inherit from Python BaseException class, which has tp_clear. Prior to this change it was not called, leaving dangling references to BaseException.args and BaseException.__cause__.
diff --git a/src/embed_tests/Inheritance.cs b/src/embed_tests/Inheritance.cs
@@ -1,7 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
-using System.Runtime.InteropServices;
 
 using NUnit.Framework;
 
@@ -100,6 +98,18 @@ public void SetAdHocAttributes_WhenExtraBasePresent()
             int actual = scope.Eval<int>($"{nameof(instance)}.{nameof(Inherited.XProp)}");
             Assert.AreEqual(expected: Inherited.X, actual);
         }
+
+        // https://github.com/pythonnet/pythonnet/issues/1476
+        [Test]
+        public void BaseClearIsCalled()
+        {
+            using var scope = Py.CreateScope();
+            scope.Set("exn", new Exception("42"));
+            var msg = scope.Eval("exn.args[0]");
+            Assert.AreEqual(2, msg.Refcount);
+            scope.Set("exn", null);
+            Assert.AreEqual(1, msg.Refcount);
+        }
     }
 
     class ExtraBaseTypeProvider : IPythonBaseTypeProvider
diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Runtime.InteropServices;
 
 namespace Python.Runtime
 {
@@ -365,11 +366,30 @@ public static int tp_clear(IntPtr ob)
             if (!isTypeObject)
             {
                 ClearObjectDict(ob);
+
+                int baseClearResult = BaseUnmanagedClear(ob);
+                if (baseClearResult != 0)
+                {
+                    return baseClearResult;
+                }
             }
             if (self is not null) self.tpHandle = IntPtr.Zero;
             return 0;
         }
 
+        static unsafe int BaseUnmanagedClear(IntPtr ob)
+        {
+            var type = Runtime.PyObject_TYPE(new BorrowedReference(ob));
+            var unmanagedBase = GetUnmanagedBaseType(type);
+            var clearPtr = Marshal.ReadIntPtr(unmanagedBase.DangerousGetAddress(), TypeOffset.tp_clear);
+            if (clearPtr == IntPtr.Zero)
+            {
+                return 0;
+            }
+            var clear = (delegate* unmanaged[Cdecl]<IntPtr, int>)clearPtr;
+            return clear(ob);
+        }
+
         protected override void OnSave(InterDomainContext context)
         {
             base.OnSave(context);
diff --git a/src/runtime/managedtype.cs b/src/runtime/managedtype.cs
@@ -149,6 +149,16 @@ internal static bool IsManagedType(BorrowedReference type)
             return (flags & TypeFlags.HasClrInstance) != 0;
         }
 
+        internal static BorrowedReference GetUnmanagedBaseType(BorrowedReference managedType)
+        {
+            Debug.Assert(managedType != null && IsManagedType(managedType));
+            do
+            {
+                managedType = PyType.GetBase(managedType);
+            } while (IsManagedType(managedType));
+            return managedType;
+        }
+
         public bool IsClrMetaTypeInstance()
         {
             Debug.Assert(Runtime.PyCLRMetaType != IntPtr.Zero);
diff --git a/src/runtime/pytype.cs b/src/runtime/pytype.cs
@@ -1,5 +1,6 @@
 #nullable enable
 using System;
+using System.Diagnostics;
 using System.Runtime.InteropServices;
 
 using Python.Runtime.Native;
@@ -60,6 +61,11 @@ public static bool IsType(PyObject value)
 
             return Runtime.PyType_Check(value.obj);
         }
+        /// <summary>Checks if specified object is a Python type.</summary>
+        internal static bool IsType(BorrowedReference value)
+        {
+            return Runtime.PyType_Check(value.DangerousGetAddress());
+        }
 
         /// <summary>
         /// Gets <see cref="PyType"/>, which represents the specified CLR type.
@@ -78,10 +84,7 @@ internal static PyType Get(Type clrType)
 
         internal BorrowedReference BaseReference
         {
-            get
-            {
-                return new(Marshal.ReadIntPtr(Handle, TypeOffset.tp_base));
-            }
+            get => GetBase(Reference);
             set
             {
                 var old = BaseReference.DangerousGetAddressOrNull();
@@ -100,6 +103,13 @@ internal IntPtr GetSlot(TypeSlotID slot)
             return Exceptions.ErrorCheckIfNull(result);
         }
 
+        internal static BorrowedReference GetBase(BorrowedReference type)
+        {
+            Debug.Assert(IsType(type));
+            IntPtr basePtr = Marshal.ReadIntPtr(type.DangerousGetAddress(), TypeOffset.tp_base);
+            return new BorrowedReference(basePtr);
+        }
+
         private static IntPtr EnsureIsType(in StolenReference reference)
         {
             IntPtr address = reference.DangerousGetAddressOrNull();
