check if PyObject.Dispose throws any Python exceptions

lostmsu · lostmsu · commit 72fae73386ce · 2020-05-13T14:00:08.000-07:00
diff --git a/src/runtime/clrobject.cs b/src/runtime/clrobject.cs
@@ -35,13 +35,13 @@ internal CLRObject(object ob, IntPtr tp)
         }
 
 
-        internal static CLRObject GetInstance(object ob, IntPtr pyType)
+        static CLRObject GetInstance(object ob, IntPtr pyType)
         {
             return new CLRObject(ob, pyType);
         }
 
 
-        internal static CLRObject GetInstance(object ob)
+        static CLRObject GetInstance(object ob)
         {
             ClassBase cc = ClassManager.GetClass(ob.GetType());
             return GetInstance(ob, cc.tpHandle);
diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs
@@ -188,7 +188,29 @@ protected virtual void Dispose(bool disposing)
 
             if (!Runtime.IsFinalizing)
             {
-                Runtime.XDecref(this.obj);
+                long refcount = Runtime.Refcount(this.obj);
+                Debug.Assert(refcount > 0, "Object refcount is 0 or less");
+
+                if (refcount == 1)
+                {
+                    Runtime.PyErr_Fetch(out var errType, out var errVal, out var traceback);
+
+                    try
+                    {
+                        Runtime.XDecref(this.obj);
+                        Runtime.CheckExceptionOccurred();
+                    }
+                    finally
+                    {
+                        // Python requires finalizers to preserve exception:
+                        // https://docs.python.org/3/extending/newtypes.html#finalization-and-de-allocation
+                        Runtime.PyErr_Restore(errType, errVal, traceback);
+                    }
+                }
+                else
+                {
+                    Runtime.XDecref(this.obj);
+                }
             }
             this.obj = IntPtr.Zero;
         }
diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Diagnostics.Contracts;
 using System.Runtime.InteropServices;
 using System.Security;
 using System.Text;
@@ -8,7 +9,6 @@
 
 namespace Python.Runtime
 {
-
     /// <summary>
     /// Encapsulates the low-level Python C API. Note that it is
     /// the responsibility of the caller to have acquired the GIL
@@ -106,7 +106,7 @@ public class Runtime
         internal static object IsFinalizingLock = new object();
         internal static bool IsFinalizing;
 
-        internal static bool Is32Bit = IntPtr.Size == 4;
+        internal static bool Is32Bit => IntPtr.Size == 4;
 
         // .NET core: System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
         internal static bool IsWindows = Environment.OSVersion.Platform == PlatformID.Win32NT;
@@ -659,6 +659,7 @@ internal static unsafe void XDecref(IntPtr op)
 #endif
         }
 
+        [Pure]
         internal static unsafe long Refcount(IntPtr op)
         {
             var p = (void*)op;

Original file line number	Diff line number	Diff line change
`@@ -35,13 +35,13 @@ internal CLRObject(object ob, IntPtr tp)`
`35`	`35`	`}`
`36`	`36`
`37`	`37`
`38`		`- internal static CLRObject GetInstance(object ob, IntPtr pyType)`
	`38`	`+ static CLRObject GetInstance(object ob, IntPtr pyType)`
`39`	`39`	`{`
`40`	`40`	`return new CLRObject(ob, pyType);`
`41`	`41`	`}`
`42`	`42`
`43`	`43`
`44`		`- internal static CLRObject GetInstance(object ob)`
	`44`	`+ static CLRObject GetInstance(object ob)`
`45`	`45`	`{`
`46`	`46`	`ClassBase cc = ClassManager.GetClass(ob.GetType());`
`47`	`47`	`return GetInstance(ob, cc.tpHandle);`