Merge pull request #1074 from losttech/bugs/1073

filmor · web-flow · commit 55abd29839a8 · 2021-11-23T18:43:17.000+01:00
Track Runtime run number
diff --git a/src/embed_tests/Codecs.cs b/src/embed_tests/Codecs.cs
@@ -421,13 +421,18 @@ public PyObject TryEncode(object value)
             }
         }
 
-        class InstancelessExceptionDecoder : IPyObjectDecoder
+        class InstancelessExceptionDecoder : IPyObjectDecoder, IDisposable
         {
             readonly PyObject PyErr = Py.Import("clr.interop").GetAttr("PyErr");
 
             public bool CanDecode(PyType objectType, Type targetType)
                 => PythonReferenceComparer.Instance.Equals(PyErr, objectType);
 
+            public void Dispose()
+            {
+                PyErr.Dispose();
+            }
+
             public bool TryDecode<T>(PyObject pyObj, out T value)
             {
                 if (pyObj.HasAttr("value"))
diff --git a/src/embed_tests/GlobalTestsSetup.cs b/src/embed_tests/GlobalTestsSetup.cs
@@ -9,6 +9,22 @@ namespace Python.EmbeddingTest
     [SetUpFixture]
     public partial class GlobalTestsSetup
     {
+        [OneTimeSetUp]
+        public void GlobalSetup()
+        {
+            Finalizer.Instance.ErrorHandler += FinalizerErrorHandler;
+        }
+
+        private void FinalizerErrorHandler(object sender, Finalizer.ErrorArgs e)
+        {
+            if (e.Error is RuntimeShutdownException)
+            {
+                // allow objects to leak after the python runtime run
+                // they were created in is gone
+                e.Handled = true;
+            }
+        }
+
         [OneTimeTearDown]
         public void FinalCleanup()
         {
diff --git a/src/embed_tests/Inheritance.cs b/src/embed_tests/Inheritance.cs
@@ -24,6 +24,7 @@ public void SetUp()
         [OneTimeTearDown]
         public void Dispose()
         {
+            ExtraBaseTypeProvider.ExtraBase.Dispose();
             PythonEngine.Shutdown();
         }
 
diff --git a/src/embed_tests/TestDomainReload.cs b/src/embed_tests/TestDomainReload.cs
@@ -179,116 +179,6 @@ public static void CrossDomainObject()
 
         #endregion
 
-        #region Tempary tests
-
-        // https://github.com/pythonnet/pythonnet/pull/1074#issuecomment-596139665
-        [Test]
-        public void CrossReleaseBuiltinType()
-        {
-            void ExecTest()
-            {
-                try
-                {
-                    PythonEngine.Initialize();
-                    var numRef = CreateNumReference();
-                    Assert.True(numRef.IsAlive);
-                    PythonEngine.Shutdown(); // <- "run" 1 ends
-                    PythonEngine.Initialize(); // <- "run" 2 starts
-
-                    GC.Collect();
-                    GC.WaitForPendingFinalizers(); // <- this will put former `num` into Finalizer queue
-                    Finalizer.Instance.Collect();
-                    // ^- this will call PyObject.Dispose, which will call XDecref on `num.Handle`,
-                    // but Python interpreter from "run" 1 is long gone, so it will corrupt memory instead.
-                    Assert.False(numRef.IsAlive);
-                }
-                finally
-                {
-                    PythonEngine.Shutdown();
-                }
-            }
-
-            var errorArgs = new List<Finalizer.ErrorArgs>();
-            void ErrorHandler(object sender, Finalizer.ErrorArgs e)
-            {
-                errorArgs.Add(e);
-            }
-            Finalizer.Instance.ErrorHandler += ErrorHandler;
-            try
-            {
-                for (int i = 0; i < 10; i++)
-                {
-                    ExecTest();
-                }
-            }
-            finally
-            {
-                Finalizer.Instance.ErrorHandler -= ErrorHandler;
-            }
-            Assert.AreEqual(errorArgs.Count, 0);
-        }
-
-        [Test]
-        public void CrossReleaseCustomType()
-        {
-            void ExecTest()
-            {
-                try
-                {
-                    PythonEngine.Initialize();
-                    var objRef = CreateConcreateObject();
-                    Assert.True(objRef.IsAlive);
-                    PythonEngine.Shutdown(); // <- "run" 1 ends
-                    PythonEngine.Initialize(); // <- "run" 2 starts
-                    GC.Collect();
-                    GC.WaitForPendingFinalizers();
-                    Finalizer.Instance.Collect();
-                    Assert.False(objRef.IsAlive);
-                }
-                finally
-                {
-                    PythonEngine.Shutdown();
-                }
-            }
-
-            var errorArgs = new List<Finalizer.ErrorArgs>();
-            void ErrorHandler(object sender, Finalizer.ErrorArgs e)
-            {
-                errorArgs.Add(e);
-            }
-            Finalizer.Instance.ErrorHandler += ErrorHandler;
-            try
-            {
-                for (int i = 0; i < 10; i++)
-                {
-                    ExecTest();
-                }
-            }
-            finally
-            {
-                Finalizer.Instance.ErrorHandler -= ErrorHandler;
-            }
-            Assert.AreEqual(errorArgs.Count, 0);
-        }
-
-        private static WeakReference CreateNumReference()
-        {
-            var num = 3216757418.ToPython();
-            Assert.AreEqual(num.Refcount, 1);
-            WeakReference numRef = new WeakReference(num, false);
-            return numRef;
-        }
-
-        private static WeakReference CreateConcreateObject()
-        {
-            var obj = new Domain.MyClass().ToPython();
-            Assert.AreEqual(obj.Refcount, 1);
-            WeakReference numRef = new WeakReference(obj, false);
-            return numRef;
-        }
-
-        #endregion Tempary tests
-
         //github.com/ <summary>
         //github.com/ This is a magic incantation required to run code in an application
         //github.com/ domain other than the current one.
diff --git a/src/embed_tests/pyinitialize.cs b/src/embed_tests/pyinitialize.cs
@@ -40,8 +40,10 @@ public static void LoadSpecificArgs()
             {
                 using (var argv = new PyList(Runtime.Runtime.PySys_GetObject("argv")))
                 {
-                    Assert.AreEqual(args[0], argv[0].ToString());
-                    Assert.AreEqual(args[1], argv[1].ToString());
+                    using var v0 = argv[0];
+                    using var v1 = argv[1];
+                    Assert.AreEqual(args[0], v0.ToString());
+                    Assert.AreEqual(args[1], v1.ToString());
                 }
             }
         }
@@ -54,12 +56,16 @@ public void ImportClassShutdownRefcount()
 
             PyObject ns = Py.Import(typeof(ImportClassShutdownRefcountClass).Namespace);
             PyObject cls = ns.GetAttr(nameof(ImportClassShutdownRefcountClass));
+            BorrowedReference clsRef = cls.Reference;
+#pragma warning disable CS0618 // Type or member is obsolete
+            cls.Leak();
+#pragma warning restore CS0618 // Type or member is obsolete
             ns.Dispose();
 
-            Assert.Less(cls.Refcount, 256);
+            Assert.Less(Runtime.Runtime.Refcount(clsRef), 256);
 
             PythonEngine.Shutdown();
-            Assert.Greater(cls.Refcount, 0);
+            Assert.Greater(Runtime.Runtime.Refcount(clsRef), 0);
         }
 
         //github.com/ <summary>
@@ -176,6 +182,7 @@ public static void TestRunExitFuncs()
                 {
                     Assert.Fail(msg);
                 }
+                PythonEngine.InteropConfiguration = InteropConfiguration.MakeDefault();
                 return;
             }
             bool called = false;
@@ -187,6 +194,7 @@ public static void TestRunExitFuncs()
             atexit.Dispose();
             Runtime.Runtime.Shutdown();
             Assert.True(called);
+            PythonEngine.InteropConfiguration = InteropConfiguration.MakeDefault();
         }
     }
 
diff --git a/src/runtime/Codecs/DecoderGroup.cs b/src/runtime/Codecs/DecoderGroup.cs
@@ -8,7 +8,7 @@ namespace Python.Runtime.Codecs
     //github.com/ <summary>
     //github.com/ Represents a group of <see cref="IPyObjectDecoder"/>s. Useful to group them by priority.
     //github.com/ </summary>
-    public sealed class DecoderGroup: IPyObjectDecoder, IEnumerable<IPyObjectDecoder>
+    public sealed class DecoderGroup: IPyObjectDecoder, IEnumerable<IPyObjectDecoder>, IDisposable
     {
         readonly List<IPyObjectDecoder> decoders = new List<IPyObjectDecoder>();
 
@@ -46,6 +46,15 @@ public bool TryDecode<T>(PyObject pyObj, out T value)
         //github.com/ <inheritdoc />
         public IEnumerator<IPyObjectDecoder> GetEnumerator() => this.decoders.GetEnumerator();
         IEnumerator IEnumerable.GetEnumerator() => this.decoders.GetEnumerator();
+
+        public void Dispose()
+        {
+            foreach (var decoder in this.decoders.OfType<IDisposable>())
+            {
+                decoder.Dispose();
+            }
+            this.decoders.Clear();
+        }
     }
 
     public static class DecoderGroupExtensions
diff --git a/src/runtime/Codecs/EncoderGroup.cs b/src/runtime/Codecs/EncoderGroup.cs
@@ -8,7 +8,7 @@ namespace Python.Runtime.Codecs
     //github.com/ <summary>
     //github.com/ Represents a group of <see cref="IPyObjectDecoder"/>s. Useful to group them by priority.
     //github.com/ </summary>
-    public sealed class EncoderGroup: IPyObjectEncoder, IEnumerable<IPyObjectEncoder>
+    public sealed class EncoderGroup: IPyObjectEncoder, IEnumerable<IPyObjectEncoder>, IDisposable
     {
         readonly List<IPyObjectEncoder> encoders = new List<IPyObjectEncoder>();
 
@@ -47,6 +47,15 @@ public PyObject TryEncode(object value)
         //github.com/ <inheritdoc />
         public IEnumerator<IPyObjectEncoder> GetEnumerator() => this.encoders.GetEnumerator();
         IEnumerator IEnumerable.GetEnumerator() => this.encoders.GetEnumerator();
+
+        public void Dispose()
+        {
+            foreach (var encoder in this.encoders.OfType<IDisposable>())
+            {
+                encoder.Dispose();
+            }
+            this.encoders.Clear();
+        }
     }
 
     public static class EncoderGroupExtensions
diff --git a/src/runtime/InteropConfiguration.cs b/src/runtime/InteropConfiguration.cs
@@ -2,10 +2,11 @@ namespace Python.Runtime
 {
     using System;
     using System.Collections.Generic;
+    using System.Linq;
 
     using Python.Runtime.Mixins;
 
-    public sealed class InteropConfiguration
+    public sealed class InteropConfiguration: IDisposable
     {
         internal readonly PythonBaseTypeProviderGroup pythonBaseTypeProviders
             = new PythonBaseTypeProviderGroup();
@@ -24,5 +25,14 @@ public static InteropConfiguration MakeDefault()
                 },
             };
         }
+
+        public void Dispose()
+        {
+            foreach (var provider in PythonBaseTypeProviders.OfType<IDisposable>())
+            {
+                provider.Dispose();
+            }
+            PythonBaseTypeProviders.Clear();
+        }
     }
 }
diff --git a/src/runtime/Mixins/CollectionMixinsProvider.cs b/src/runtime/Mixins/CollectionMixinsProvider.cs
@@ -4,7 +4,7 @@
 
 namespace Python.Runtime.Mixins
 {
-    class CollectionMixinsProvider : IPythonBaseTypeProvider
+    class CollectionMixinsProvider : IPythonBaseTypeProvider, IDisposable
     {
         readonly Lazy<PyObject> mixinsModule;
         public CollectionMixinsProvider(Lazy<PyObject> mixinsModule)
@@ -86,5 +86,13 @@ static Type[] NewInterfaces(Type type)
 
         static Type GetDefinition(Type type)
             => type.IsGenericType ? type.GetGenericTypeDefinition() : type;
+
+        public void Dispose()
+        {
+            if (this.mixinsModule.IsValueCreated)
+            {
+                this.mixinsModule.Value.Dispose();
+            }
+        }
     }
 }
diff --git a/src/runtime/converterextensions.cs b/src/runtime/converterextensions.cs
@@ -166,8 +166,8 @@ internal static void Reset()
                 {
                     clrToPython.Clear();
                     pythonToClr.Clear();
-                    encoders.Clear();
-                    decoders.Clear();
+                    encoders.Dispose();
+                    decoders.Dispose();
                 }
         }
 
diff --git a/src/runtime/finalizer.cs b/src/runtime/finalizer.cs
diff --git a/src/runtime/pybuffer.cs b/src/runtime/pybuffer.cs
diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs
diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs
diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs

Original file line number	Diff line number	Diff line change
`@@ -421,13 +421,18 @@ public PyObject TryEncode(object value)`
`421`	`421`	`}`
`422`	`422`	`}`
`423`	`423`
`424`		`- class InstancelessExceptionDecoder : IPyObjectDecoder`
	`424`	`+ class InstancelessExceptionDecoder : IPyObjectDecoder, IDisposable`
`425`	`425`	`{`
`426`	`426`	`readonly PyObject PyErr = Py.Import("clr.interop").GetAttr("PyErr");`
`427`	`427`
`428`	`428`	`public bool CanDecode(PyType objectType, Type targetType)`
`429`	`429`	`=> PythonReferenceComparer.Instance.Equals(PyErr, objectType);`
`430`	`430`
	`431`	`+ public void Dispose()`
	`432`	`+ {`
	`433`	`+ PyErr.Dispose();`
	`434`	`+ }`
	`435`	`+`
`431`	`436`	`public bool TryDecode<T>(PyObject pyObj, out T value)`
`432`	`437`	`{`
`433`	`438`	`if (pyObj.HasAttr("value"))`
Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@ public void SetUp()`
`24`	`24`	`[OneTimeTearDown]`
`25`	`25`	`public void Dispose()`
`26`	`26`	`{`
	`27`	`+ ExtraBaseTypeProvider.ExtraBase.Dispose();`
`27`	`28`	`PythonEngine.Shutdown();`
`28`	`29`	`}`
`29`	`30`
Original file line number	Diff line number	Diff line change
`@@ -40,8 +40,10 @@ public static void LoadSpecificArgs()`
`40`	`40`	`{`
`41`	`41`	`using (var argv = new PyList(Runtime.Runtime.PySys_GetObject("argv")))`
`42`	`42`	`{`
`43`		`- Assert.AreEqual(args[0], argv[0].ToString());`
`44`		`- Assert.AreEqual(args[1], argv[1].ToString());`
	`43`	`+ using var v0 = argv[0];`
	`44`	`+ using var v1 = argv[1];`
	`45`	`+ Assert.AreEqual(args[0], v0.ToString());`
	`46`	`+ Assert.AreEqual(args[1], v1.ToString());`
`45`	`47`	`}`
`46`	`48`	`}`
`47`	`49`	`}`
`@@ -54,12 +56,16 @@ public void ImportClassShutdownRefcount()`
`54`	`56`
`55`	`57`	`PyObject ns = Py.Import(typeof(ImportClassShutdownRefcountClass).Namespace);`
`56`	`58`	`PyObject cls = ns.GetAttr(nameof(ImportClassShutdownRefcountClass));`
	`59`	`+ BorrowedReference clsRef = cls.Reference;`
	`60`	`+#pragma warning disable CS0618 // Type or member is obsolete`
	`61`	`+ cls.Leak();`
	`62`	`+#pragma warning restore CS0618 // Type or member is obsolete`
`57`	`63`	`ns.Dispose();`
`58`	`64`
`59`		`- Assert.Less(cls.Refcount, 256);`
	`65`	`+ Assert.Less(Runtime.Runtime.Refcount(clsRef), 256);`
`60`	`66`
`61`	`67`	`PythonEngine.Shutdown();`
`62`		`- Assert.Greater(cls.Refcount, 0);`
	`68`	`+ Assert.Greater(Runtime.Runtime.Refcount(clsRef), 0);`
`63`	`69`	`}`
`64`	`70`
`65`	`71`	`//github.com/ <summary>`
`@@ -176,6 +182,7 @@ public static void TestRunExitFuncs()`
`176`	`182`	`{`
`177`	`183`	`Assert.Fail(msg);`
`178`	`184`	`}`
	`185`	`+ PythonEngine.InteropConfiguration = InteropConfiguration.MakeDefault();`
`179`	`186`	`return;`
`180`	`187`	`}`
`181`	`188`	`bool called = false;`
`@@ -187,6 +194,7 @@ public static void TestRunExitFuncs()`
`187`	`194`	`atexit.Dispose();`
`188`	`195`	`Runtime.Runtime.Shutdown();`
`189`	`196`	`Assert.True(called);`
	`197`	`+ PythonEngine.InteropConfiguration = InteropConfiguration.MakeDefault();`
`190`	`198`	`}`
`191`	`199`	`}`
`192`	`200`
Original file line number	Diff line number	Diff line change
`@@ -2,10 +2,11 @@ namespace Python.Runtime`
`2`	`2`	`{`
`3`	`3`	`using System;`
`4`	`4`	`using System.Collections.Generic;`
	`5`	`+ using System.Linq;`
`5`	`6`
`6`	`7`	`using Python.Runtime.Mixins;`
`7`	`8`
`8`		`- public sealed class InteropConfiguration`
	`9`	`+ public sealed class InteropConfiguration: IDisposable`
`9`	`10`	`{`
`10`	`11`	`internal readonly PythonBaseTypeProviderGroup pythonBaseTypeProviders`
`11`	`12`	`= new PythonBaseTypeProviderGroup();`
`@@ -24,5 +25,14 @@ public static InteropConfiguration MakeDefault()`
`24`	`25`	`},`
`25`	`26`	`};`
`26`	`27`	`}`
	`28`	`+`
	`29`	`+ public void Dispose()`
	`30`	`+ {`
	`31`	`+ foreach (var provider in PythonBaseTypeProviders.OfType<IDisposable>())`
	`32`	`+ {`
	`33`	`+ provider.Dispose();`
	`34`	`+ }`
	`35`	`+ PythonBaseTypeProviders.Clear();`
	`36`	`+ }`
`27`	`37`	`}`
`28`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,7 @@`
`4`	`4`
`5`	`5`	`namespace Python.Runtime.Mixins`
`6`	`6`	`{`
`7`		`- class CollectionMixinsProvider : IPythonBaseTypeProvider`
	`7`	`+ class CollectionMixinsProvider : IPythonBaseTypeProvider, IDisposable`
`8`	`8`	`{`
`9`	`9`	`readonly Lazy<PyObject> mixinsModule;`
`10`	`10`	`public CollectionMixinsProvider(Lazy<PyObject> mixinsModule)`
`@@ -86,5 +86,13 @@ static Type[] NewInterfaces(Type type)`
`86`	`86`
`87`	`87`	`static Type GetDefinition(Type type)`
`88`	`88`	`=> type.IsGenericType ? type.GetGenericTypeDefinition() : type;`
	`89`	`+`
	`90`	`+ public void Dispose()`
	`91`	`+ {`
	`92`	`+ if (this.mixinsModule.IsValueCreated)`
	`93`	`+ {`
	`94`	`+ this.mixinsModule.Value.Dispose();`
	`95`	`+ }`
	`96`	`+ }`
`89`	`97`	`}`
`90`	`98`	`}`
Original file line number	Diff line number	Diff line change
`@@ -166,8 +166,8 @@ internal static void Reset()`
`166`	`166`	`{`
`167`	`167`	`clrToPython.Clear();`
`168`	`168`	`pythonToClr.Clear();`
`169`		`- encoders.Clear();`
`170`		`- decoders.Clear();`
	`169`	`+ encoders.Dispose();`
	`170`	`+ decoders.Dispose();`
`171`	`171`	`}`
`172`	`172`	`}`
`173`	`173`