ClassManager illegally decrefed ClassObject's refcount on shutdown

lostmsu · lostmsu · commit 9616a6f87743 · 2021-09-22T10:10:40.000-07:00
addresses #1561
diff --git a/src/embed_tests/pyinitialize.cs b/src/embed_tests/pyinitialize.cs
@@ -46,6 +46,22 @@ public static void LoadSpecificArgs()
             }
         }
 
+        // regression test for https://github.com/pythonnet/pythonnet/issues/1561
+        [Test]
+        public void ImportClassShutdownRefcount()
+        {
+            PythonEngine.Initialize();
+
+            PyObject ns = Py.Import(typeof(ImportClassShutdownRefcountClass).Namespace);
+            PyObject cls = ns.GetAttr(nameof(ImportClassShutdownRefcountClass));
+            ns.Dispose();
+
+            Assert.Less(cls.Refcount, 256);
+
+            PythonEngine.Shutdown();
+            Assert.Greater(cls.Refcount, 0);
+        }
+
         /// <summary>
         /// Failing test demonstrating current issue with OverflowException (#376)
         /// and ArgumentException issue after that one is fixed.
@@ -182,4 +198,6 @@ public static void TestRunExitFuncs()
             Assert.True(called);
         }
     }
+
+    public class ImportClassShutdownRefcountClass { }
 }
diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs
@@ -67,7 +67,6 @@ internal static void DisposePythonWrappersForClrTypes()
                     // since others may still referencing it.
                     cls.CallTypeTraverse(TraverseTypeClear, visitedPtr);
                     cls.CallTypeClear();
-                    cls.DecrRefCount();
                 }
             }
             finally

Original file line number	Diff line number	Diff line change
`@@ -46,6 +46,22 @@ public static void LoadSpecificArgs()`
`46`	`46`	`}`
`47`	`47`	`}`
`48`	`48`
	`49`	`+ // regression test for https://github.com/pythonnet/pythonnet/issues/1561`
	`50`	`+ [Test]`
	`51`	`+ public void ImportClassShutdownRefcount()`
	`52`	`+ {`
	`53`	`+ PythonEngine.Initialize();`
	`54`	`+`
	`55`	`+ PyObject ns = Py.Import(typeof(ImportClassShutdownRefcountClass).Namespace);`
	`56`	`+ PyObject cls = ns.GetAttr(nameof(ImportClassShutdownRefcountClass));`
	`57`	`+ ns.Dispose();`
	`58`	`+`
	`59`	`+ Assert.Less(cls.Refcount, 256);`
	`60`	`+`
	`61`	`+ PythonEngine.Shutdown();`
	`62`	`+ Assert.Greater(cls.Refcount, 0);`
	`63`	`+ }`
	`64`	`+`
`49`	`65`	`/// <summary>`
`50`	`66`	`/// Failing test demonstrating current issue with OverflowException (#376)`
`51`	`67`	`/// and ArgumentException issue after that one is fixed.`
`@@ -182,4 +198,6 @@ public static void TestRunExitFuncs()`
`182`	`198`	`Assert.True(called);`
`183`	`199`	`}`
`184`	`200`	`}`
	`201`	`+`
	`202`	`+ public class ImportClassShutdownRefcountClass { }`
`185`	`203`	`}`
Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,6 @@ internal static void DisposePythonWrappersForClrTypes()`
`67`	`67`	`// since others may still referencing it.`
`68`	`68`	`cls.CallTypeTraverse(TraverseTypeClear, visitedPtr);`
`69`	`69`	`cls.CallTypeClear();`
`70`		`- cls.DecrRefCount();`
`71`	`70`	`}`
`72`	`71`	`}`
`73`	`72`	`finally`