From 4f657d46b34f14cd4a5ff43087ceedde54618663 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Mon, 2 Mar 2020 18:12:13 -0800 Subject: [PATCH 1/9] Track Runtime run number. Assert, that PyObjects are only disposed in the same run they were created in. --- src/runtime/pybuffer.cs | 8 ++++---- src/runtime/pyobject.cs | 44 ++++++++++++++++++++++++++--------------- src/runtime/runtime.cs | 30 +++++++++++++++++++++++++++- 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/src/runtime/pybuffer.cs b/src/runtime/pybuffer.cs index 7161a864f..21ec0d889 100644 --- a/src/runtime/pybuffer.cs +++ b/src/runtime/pybuffer.cs @@ -236,10 +236,10 @@ private void Dispose(bool disposing) ~PyBuffer() { - if (disposedValue) - { - return; - } + Debug.Assert(!disposedValue); + + _exporter.CheckRun(); + Finalizer.Instance.AddFinalizedObject(ref _view.obj); } diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index bd767307b..c53220347 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -27,6 +27,7 @@ public partial class PyObject : DynamicObject, IDisposable #endif protected internal IntPtr obj = IntPtr.Zero; + readonly int run = Runtime.GetRun(); public static PyObject None => new PyObject(new BorrowedReference(Runtime.PyNone)); internal BorrowedReference Reference => new BorrowedReference(this.obj); @@ -95,11 +96,15 @@ internal PyObject(in StolenReference reference) // when the managed wrapper is garbage-collected. ~PyObject() { - if (obj == IntPtr.Zero) - { - return; - } + Debug.Assert(obj != IntPtr.Zero); + +#if TRACE_ALLOC + CheckRun(); +#endif + Finalizer.Instance.AddFinalizedObject(ref obj); + + Dispose(false); } @@ -167,17 +172,6 @@ public object AsManagedObject(Type t) internal bool IsDisposed => obj == IntPtr.Zero; - /// - /// Dispose Method - /// - /// - /// The Dispose method provides a way to explicitly release the - /// Python object represented by a PyObject instance. It is a good - /// idea to call Dispose on PyObjects that wrap resources that are - /// limited or need strict lifetime control. Otherwise, references - /// to Python objects will not be released until a managed garbage - /// collection occurs. - /// protected virtual void Dispose(bool disposing) { if (this.obj == IntPtr.Zero) @@ -188,6 +182,8 @@ protected virtual void Dispose(bool disposing) if (Runtime.Py_IsInitialized() == 0) throw new InvalidOperationException("Python runtime must be initialized"); + CheckRun(); + if (!Runtime.IsFinalizing) { long refcount = Runtime.Refcount(this.obj); @@ -221,10 +217,26 @@ protected virtual void Dispose(bool disposing) this.obj = IntPtr.Zero; } + /// + /// The Dispose method provides a way to explicitly release the + /// Python object represented by a PyObject instance. It is a good + /// idea to call Dispose on PyObjects that wrap resources that are + /// limited or need strict lifetime control. Otherwise, references + /// to Python objects will not be released until a managed garbage + /// collection occurs. + /// public void Dispose() { - Dispose(true); GC.SuppressFinalize(this); + Dispose(true); + } + + internal void CheckRun() + { + if (run != Runtime.GetRun()) + throw new InvalidOperationException( + "PythonEngine was shut down after this object was created." + + " It is an error to attempt to dispose or to continue using it."); } internal BorrowedReference GetPythonTypeReference() diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index b4b045b4a..f8e600d7a 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -85,7 +85,15 @@ internal static Version PyVersion } } - /// + static int run = 0; + + internal static int GetRun() + { + int runNumber = run; + Debug.Assert(runNumber > 0, "This must only be called after Runtime is initialized at least once"); + return runNumber; + } + /// Initialize the runtime... /// /// Always call this method from the Main thread. After the @@ -110,6 +118,9 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd if (!interpreterAlreadyInitialized) { Py_InitializeEx(initSigs ? 1 : 0); + + NewRun(); + if (PyEval_ThreadsInitialized() == 0) { PyEval_InitThreads(); @@ -130,6 +141,16 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd { PyGILState_Ensure(); } + + BorrowedReference pyRun = PySys_GetObject("__pynet_run__"); + if (pyRun != null) + { + run = checked((int)PyLong_AsSignedSize_t(pyRun)); + } + else + { + NewRun(); + } } MainManagedThreadId = Thread.CurrentThread.ManagedThreadId; @@ -175,6 +196,13 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd inspect = GetModuleLazy("inspect"); } + static void NewRun() + { + run++; + using var pyRun = PyLong_FromLongLong(run); + PySys_SetObject("__pynet_run__", pyRun); + } + private static void InitPyMembers() { IntPtr op; From 62e2fb45eebac0a01bb4df4ed8f276c4e4d59a23 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 11 Nov 2021 18:49:50 -0800 Subject: [PATCH 2/9] dispose registered codecs and interop configuration during shutdown --- src/embed_tests/Codecs.cs | 7 ++++++- src/embed_tests/Inheritance.cs | 1 + src/embed_tests/pyinitialize.cs | 2 ++ src/runtime/Codecs/DecoderGroup.cs | 11 ++++++++++- src/runtime/Codecs/EncoderGroup.cs | 11 ++++++++++- src/runtime/InteropConfiguration.cs | 12 +++++++++++- src/runtime/Mixins/CollectionMixinsProvider.cs | 10 +++++++++- src/runtime/converterextensions.cs | 4 ++-- src/runtime/pythonengine.cs | 1 - src/runtime/runtime.cs | 13 +++++++++++++ 10 files changed, 64 insertions(+), 8 deletions(-) diff --git a/src/embed_tests/Codecs.cs b/src/embed_tests/Codecs.cs index 1beddbec9..157e60803 100644 --- 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(PyObject pyObj, out T value) { if (pyObj.HasAttr("value")) diff --git a/src/embed_tests/Inheritance.cs b/src/embed_tests/Inheritance.cs index 58d66ed96..7de5277a1 100644 --- 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/pyinitialize.cs b/src/embed_tests/pyinitialize.cs index a15aff585..4727eaccb 100644 --- a/src/embed_tests/pyinitialize.cs +++ b/src/embed_tests/pyinitialize.cs @@ -176,6 +176,7 @@ public static void TestRunExitFuncs() { Assert.Fail(msg); } + PythonEngine.InteropConfiguration = InteropConfiguration.MakeDefault(); return; } bool called = false; @@ -187,6 +188,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 index b72cd796c..cf0ee33e9 100644 --- a/src/runtime/Codecs/DecoderGroup.cs +++ b/src/runtime/Codecs/DecoderGroup.cs @@ -8,7 +8,7 @@ namespace Python.Runtime.Codecs /// /// Represents a group of s. Useful to group them by priority. /// - public sealed class DecoderGroup: IPyObjectDecoder, IEnumerable + public sealed class DecoderGroup: IPyObjectDecoder, IEnumerable, IDisposable { readonly List decoders = new List(); @@ -46,6 +46,15 @@ public bool TryDecode(PyObject pyObj, out T value) /// public IEnumerator GetEnumerator() => this.decoders.GetEnumerator(); IEnumerator IEnumerable.GetEnumerator() => this.decoders.GetEnumerator(); + + public void Dispose() + { + foreach (var decoder in this.decoders.OfType()) + { + decoder.Dispose(); + } + this.decoders.Clear(); + } } public static class DecoderGroupExtensions diff --git a/src/runtime/Codecs/EncoderGroup.cs b/src/runtime/Codecs/EncoderGroup.cs index 4f776a669..6c40623ca 100644 --- a/src/runtime/Codecs/EncoderGroup.cs +++ b/src/runtime/Codecs/EncoderGroup.cs @@ -8,7 +8,7 @@ namespace Python.Runtime.Codecs /// /// Represents a group of s. Useful to group them by priority. /// - public sealed class EncoderGroup: IPyObjectEncoder, IEnumerable + public sealed class EncoderGroup: IPyObjectEncoder, IEnumerable, IDisposable { readonly List encoders = new List(); @@ -47,6 +47,15 @@ public PyObject TryEncode(object value) /// public IEnumerator GetEnumerator() => this.encoders.GetEnumerator(); IEnumerator IEnumerable.GetEnumerator() => this.encoders.GetEnumerator(); + + public void Dispose() + { + foreach (var encoder in this.encoders.OfType()) + { + encoder.Dispose(); + } + this.encoders.Clear(); + } } public static class EncoderGroupExtensions diff --git a/src/runtime/InteropConfiguration.cs b/src/runtime/InteropConfiguration.cs index 78af5037a..30c9a1c2c 100644 --- 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()) + { + provider.Dispose(); + } + PythonBaseTypeProviders.Clear(); + } } } diff --git a/src/runtime/Mixins/CollectionMixinsProvider.cs b/src/runtime/Mixins/CollectionMixinsProvider.cs index 48ea35f1c..e01a6be0d 100644 --- 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 mixinsModule; public CollectionMixinsProvider(Lazy 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 index 2396fb0bd..dfc2ecc21 100644 --- 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/pythonengine.cs b/src/runtime/pythonengine.cs index 91e013e86..35ca1ce1e 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -381,7 +381,6 @@ public static void Shutdown(ShutdownMode mode) ExecuteShutdownHandlers(); // Remember to shut down the runtime. Runtime.Shutdown(mode); - PyObjectConversions.Reset(); initialized = false; diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index f8e600d7a..a72e0cdec 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -373,6 +373,11 @@ internal static void Shutdown(ShutdownMode mode) PyCLRMetaType = IntPtr.Zero; Exceptions.Shutdown(); + PythonEngine.InteropConfiguration.Dispose(); + DisposeLazyModule(clrInterop); + DisposeLazyModule(inspect); + PyObjectConversions.Reset(); + Finalizer.Shutdown(); InternString.Shutdown(); @@ -424,6 +429,14 @@ internal static void Shutdown() Shutdown(mode); } + static void DisposeLazyModule(Lazy module) + { + if (module.IsValueCreated) + { + module.Value.Dispose(); + } + } + private static Lazy GetModuleLazy(string moduleName) => moduleName is null ? throw new ArgumentNullException(nameof(moduleName)) From 1897d1b309e4645c72340b00a2648d3e40c70bac Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 11 Nov 2021 18:55:17 -0800 Subject: [PATCH 3/9] Finalizer raises FinalizationException when it sees an object from previous run. --- src/embed_tests/pyinitialize.cs | 14 ++++-- src/runtime/finalizer.cs | 88 +++++++++++++++++++++++++-------- src/runtime/pybuffer.cs | 7 +-- src/runtime/pyobject.cs | 28 ++++++++--- src/runtime/runtime.cs | 33 +++++++++++-- 5 files changed, 131 insertions(+), 39 deletions(-) diff --git a/src/embed_tests/pyinitialize.cs b/src/embed_tests/pyinitialize.cs index 4727eaccb..61ed9c6fb 100644 --- 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); } /// diff --git a/src/runtime/finalizer.cs b/src/runtime/finalizer.cs index 5153c13ad..0acf5254d 100644 --- a/src/runtime/finalizer.cs +++ b/src/runtime/finalizer.cs @@ -2,6 +2,7 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.ComponentModel; +using System.Diagnostics; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -30,10 +31,12 @@ public class ErrorArgs : EventArgs [DefaultValue(DefaultThreshold)] public int Threshold { get; set; } = DefaultThreshold; + bool started; + [DefaultValue(true)] public bool Enable { get; set; } = true; - private ConcurrentQueue _objQueue = new ConcurrentQueue(); + private ConcurrentQueue _objQueue = new (); private int _throttled; #region FINALIZER_CHECK @@ -79,6 +82,8 @@ internal IncorrectRefCountException(IntPtr ptr) internal void ThrottledCollect() { + if (!started) throw new InvalidOperationException($"{nameof(PythonEngine)} is not initialized"); + _throttled = unchecked(this._throttled + 1); if (!Enable || _throttled < Threshold) return; _throttled = 0; @@ -87,12 +92,13 @@ internal void ThrottledCollect() internal List GetCollectedObjects() { - return _objQueue.ToList(); + return _objQueue.Select(o => o.PyObj).ToList(); } - internal void AddFinalizedObject(ref IntPtr obj) + internal void AddFinalizedObject(ref IntPtr obj, int run) { - if (!Enable || obj == IntPtr.Zero) + Debug.Assert(obj != IntPtr.Zero); + if (!Enable) { return; } @@ -101,14 +107,20 @@ internal void AddFinalizedObject(ref IntPtr obj) lock (_queueLock) #endif { - this._objQueue.Enqueue(obj); + this._objQueue.Enqueue(new PendingFinalization { PyObj = obj, RuntimeRun = run }); } obj = IntPtr.Zero; } + internal static void Initialize() + { + Instance.started = true; + } + internal static void Shutdown() { Instance.DisposeAll(); + Instance.started = false; } private void DisposeAll() @@ -124,36 +136,31 @@ private void DisposeAll() #if FINALIZER_CHECK ValidateRefCount(); #endif - IntPtr obj; Runtime.PyErr_Fetch(out var errType, out var errVal, out var traceback); + int run = Runtime.GetRun(); + try { while (!_objQueue.IsEmpty) { - if (!_objQueue.TryDequeue(out obj)) + if (!_objQueue.TryDequeue(out var obj)) + continue; + + if (obj.RuntimeRun != run) + { + HandleFinalizationException(obj.PyObj, new RuntimeShutdownException(obj.PyObj)); continue; + } - Runtime.XDecref(obj); + Runtime.XDecref(obj.PyObj); try { Runtime.CheckExceptionOccurred(); } catch (Exception e) { - var errorArgs = new ErrorArgs - { - Error = e, - }; - - ErrorHandler?.Invoke(this, errorArgs); - - if (!errorArgs.Handled) - { - throw new FinalizationException( - "Python object finalization failed", - disposable: obj, innerException: e); - } + HandleFinalizationException(obj.PyObj, e); } } } @@ -166,6 +173,23 @@ private void DisposeAll() } } + void HandleFinalizationException(IntPtr obj, Exception cause) + { + var errorArgs = new ErrorArgs + { + Error = cause, + }; + + ErrorHandler?.Invoke(this, errorArgs); + + if (!errorArgs.Handled) + { + throw new FinalizationException( + "Python object finalization failed", + disposable: obj, innerException: cause); + } + } + #if FINALIZER_CHECK private void ValidateRefCount() { @@ -235,6 +259,12 @@ private void ValidateRefCount() #endif } + struct PendingFinalization + { + public IntPtr PyObj; + public int RuntimeRun; + } + public class FinalizationException : Exception { public IntPtr Handle { get; } @@ -259,5 +289,21 @@ public FinalizationException(string message, IntPtr disposable, Exception innerE if (disposable == IntPtr.Zero) throw new ArgumentNullException(nameof(disposable)); this.Handle = disposable; } + + protected FinalizationException(string message, IntPtr disposable) + : base(message) + { + if (disposable == IntPtr.Zero) throw new ArgumentNullException(nameof(disposable)); + this.Handle = disposable; + } + } + + public class RuntimeShutdownException : FinalizationException + { + public RuntimeShutdownException(IntPtr disposable) + : base("Python runtime was shut down after this object was created." + + " It is an error to attempt to dispose or to continue using it even after restarting the runtime.", disposable) + { + } } } diff --git a/src/runtime/pybuffer.cs b/src/runtime/pybuffer.cs index 21ec0d889..94741b19b 100644 --- a/src/runtime/pybuffer.cs +++ b/src/runtime/pybuffer.cs @@ -238,9 +238,10 @@ private void Dispose(bool disposing) { Debug.Assert(!disposedValue); - _exporter.CheckRun(); - - Finalizer.Instance.AddFinalizedObject(ref _view.obj); + if (_view.obj != IntPtr.Zero) + { + Finalizer.Instance.AddFinalizedObject(ref _view.obj, _exporter.run); + } } /// diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index c53220347..5e9d5da97 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -1,10 +1,10 @@ using System; -using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.Dynamic; using System.Linq; using System.Linq.Expressions; +using System.Threading; namespace Python.Runtime { @@ -27,7 +27,7 @@ public partial class PyObject : DynamicObject, IDisposable #endif protected internal IntPtr obj = IntPtr.Zero; - readonly int run = Runtime.GetRun(); + internal readonly int run = Runtime.GetRun(); public static PyObject None => new PyObject(new BorrowedReference(Runtime.PyNone)); internal BorrowedReference Reference => new BorrowedReference(this.obj); @@ -96,13 +96,19 @@ internal PyObject(in StolenReference reference) // when the managed wrapper is garbage-collected. ~PyObject() { - Debug.Assert(obj != IntPtr.Zero); + Debug.Assert(obj != IntPtr.Zero || this.GetType() != typeof(PyObject)); + + if (obj != IntPtr.Zero) + { #if TRACE_ALLOC - CheckRun(); + CheckRun(); #endif - Finalizer.Instance.AddFinalizedObject(ref obj); + Interlocked.Increment(ref Runtime._collected); + + Finalizer.Instance.AddFinalizedObject(ref obj, run); + } Dispose(false); } @@ -231,12 +237,18 @@ public void Dispose() Dispose(true); } + [Obsolete("Test use only")] + internal void Leak() + { + Debug.Assert(obj != IntPtr.Zero); + GC.SuppressFinalize(this); + obj = IntPtr.Zero; + } + internal void CheckRun() { if (run != Runtime.GetRun()) - throw new InvalidOperationException( - "PythonEngine was shut down after this object was created." + - " It is an error to attempt to dispose or to continue using it."); + throw new RuntimeShutdownException(obj); } internal BorrowedReference GetPythonTypeReference() diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index a72e0cdec..d8099f6bb 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -155,6 +155,7 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd MainManagedThreadId = Thread.CurrentThread.ManagedThreadId; IsFinalizing = false; + Finalizer.Initialize(); InternString.Initialize(); InitPyMembers(); @@ -378,6 +379,8 @@ internal static void Shutdown(ShutdownMode mode) DisposeLazyModule(inspect); PyObjectConversions.Reset(); + bool everythingSeemsCollected = TryCollectingGarbage(); + Debug.Assert(everythingSeemsCollected); Finalizer.Shutdown(); InternString.Shutdown(); @@ -423,6 +426,26 @@ internal static void Shutdown(ShutdownMode mode) } } + const int MaxCollectRetriesOnShutdown = 20; + internal static int _collected; + static bool TryCollectingGarbage() + { + for (int attempt = 0; attempt < MaxCollectRetriesOnShutdown; attempt++) + { + Interlocked.Exchange(ref _collected, 0); + nint pyCollected = 0; + for (int i = 0; i < 2; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + pyCollected += PyGC_Collect(); + } + if (Volatile.Read(ref _collected) == 0 && pyCollected == 0) + return true; + } + return false; + } + internal static void Shutdown() { var mode = ShutdownMode; @@ -818,6 +841,10 @@ internal static unsafe long Refcount(IntPtr op) return *p; } + [Pure] + internal static long Refcount(BorrowedReference op) + => Refcount(op.DangerousGetAddress()); + /// /// Call specified function, and handle PythonDLL-related failures. /// @@ -2212,7 +2239,7 @@ internal static int PyException_SetTraceback(BorrowedReference ex, BorrowedRefer - internal static IntPtr PyGC_Collect() => Delegates.PyGC_Collect(); + internal static nint PyGC_Collect() => Delegates.PyGC_Collect(); internal static IntPtr _Py_AS_GC(BorrowedReference ob) { @@ -2592,7 +2619,7 @@ static Delegates() PyErr_Print = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Print), GetUnmanagedDll(_PythonDll)); PyCell_Get = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyCell_Get), GetUnmanagedDll(_PythonDll)); PyCell_Set = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyCell_Set), GetUnmanagedDll(_PythonDll)); - PyGC_Collect = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyGC_Collect), GetUnmanagedDll(_PythonDll)); + PyGC_Collect = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyGC_Collect), GetUnmanagedDll(_PythonDll)); PyCapsule_New = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyCapsule_New), GetUnmanagedDll(_PythonDll)); PyCapsule_GetPointer = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyCapsule_GetPointer), GetUnmanagedDll(_PythonDll)); PyCapsule_SetPointer = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyCapsule_SetPointer), GetUnmanagedDll(_PythonDll)); @@ -2871,7 +2898,7 @@ static Delegates() internal static delegate* unmanaged[Cdecl] PyErr_Print { get; } internal static delegate* unmanaged[Cdecl] PyCell_Get { get; } internal static delegate* unmanaged[Cdecl] PyCell_Set { get; } - internal static delegate* unmanaged[Cdecl] PyGC_Collect { get; } + internal static delegate* unmanaged[Cdecl] PyGC_Collect { get; } internal static delegate* unmanaged[Cdecl] PyCapsule_New { get; } internal static delegate* unmanaged[Cdecl] PyCapsule_GetPointer { get; } internal static delegate* unmanaged[Cdecl] PyCapsule_SetPointer { get; } From 3909639ce1b0c17b66707208592a0d7c991645b6 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 11 Nov 2021 20:53:52 -0800 Subject: [PATCH 4/9] allow tests to pass when objects are leaking due to being GCed after Python runtime is shut down --- src/embed_tests/GlobalTestsSetup.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/embed_tests/GlobalTestsSetup.cs b/src/embed_tests/GlobalTestsSetup.cs index 9a832cb0c..dff58b978 100644 --- 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() { From 60d90c6d6fa5c1fc365487aaff526e38b040475a Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 11 Nov 2021 21:09:49 -0800 Subject: [PATCH 5/9] removed code testing possiblity to dispose objects after domain restart --- src/embed_tests/TestDomainReload.cs | 110 ---------------------------- 1 file changed, 110 deletions(-) diff --git a/src/embed_tests/TestDomainReload.cs b/src/embed_tests/TestDomainReload.cs index e4479da18..e7b330d12 100644 --- 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(); - 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(); - 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 - /// /// This is a magic incantation required to run code in an application /// domain other than the current one. From 9e815a62ebcc52c5c958db4b63e4ca987b8c5f38 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 11 Nov 2021 22:08:28 -0800 Subject: [PATCH 6/9] allow leaking PyObject instances when CLR is stared from Python --- src/runtime/pythonengine.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs index 35ca1ce1e..6e0057036 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -317,6 +317,8 @@ public static IntPtr InitExt() { Initialize(setSysArgv: false, mode: ShutdownMode.Extension); + Finalizer.Instance.ErrorHandler += AllowLeaksDuringShutdown; + // Trickery - when the import hook is installed into an already // running Python, the standard import machinery is still in // control for the duration of the import that caused bootstrap. @@ -358,6 +360,14 @@ public static IntPtr InitExt() .DangerousMoveToPointerOrNull(); } + private static void AllowLeaksDuringShutdown(object sender, Finalizer.ErrorArgs e) + { + if (e.Error is RuntimeShutdownException) + { + e.Handled = true; + } + } + /// /// Shutdown Method /// From 8611dde56deb1c35ce042d4dff56804945612182 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 11 Nov 2021 22:48:23 -0800 Subject: [PATCH 7/9] WaitForFullGCComplete was never needed, and was used incorrectly --- src/runtime/runtime.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index d8099f6bb..3255988e8 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -393,14 +393,6 @@ internal static void Shutdown(ShutdownMode mode) } ResetPyMembers(); GC.Collect(); - try - { - GC.WaitForFullGCComplete(); - } - catch (NotImplementedException) - { - // Some clr runtime didn't implement GC.WaitForFullGCComplete yet. - } GC.WaitForPendingFinalizers(); PyGILState_Release(state); // Then release the GIL for good, if there is somehting to release From 6383a28b55e2b96f93cc4b4cd0b7ee466d67b75a Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Mon, 22 Nov 2021 12:04:39 -0800 Subject: [PATCH 8/9] remove finalizer assert for raw pointer value; skip collection assert on shutdown when loaded from Python --- src/runtime/pyobject.cs | 2 -- src/runtime/runtime.cs | 10 +++++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index 5e9d5da97..747b4ecdf 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -96,8 +96,6 @@ internal PyObject(in StolenReference reference) // when the managed wrapper is garbage-collected. ~PyObject() { - Debug.Assert(obj != IntPtr.Zero || this.GetType() != typeof(PyObject)); - if (obj != IntPtr.Zero) { diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 3255988e8..68322dff8 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -379,14 +379,18 @@ internal static void Shutdown(ShutdownMode mode) DisposeLazyModule(inspect); PyObjectConversions.Reset(); - bool everythingSeemsCollected = TryCollectingGarbage(); - Debug.Assert(everythingSeemsCollected); + if (mode != ShutdownMode.Extension) + { + PyGC_Collect(); + bool everythingSeemsCollected = TryCollectingGarbage(); + Debug.Assert(everythingSeemsCollected); + } + Finalizer.Shutdown(); InternString.Shutdown(); if (mode != ShutdownMode.Normal && mode != ShutdownMode.Extension) { - PyGC_Collect(); if (mode == ShutdownMode.Soft) { RuntimeState.Restore(); From 47b3913843f4d6899277bfed5d31e5080ea4ac97 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Tue, 23 Nov 2021 09:32:37 -0800 Subject: [PATCH 9/9] renamed run system property to __pythonnet_run__ to be consistent with other PythonNET properties --- src/runtime/runtime.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 68322dff8..217075494 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -85,6 +85,7 @@ internal static Version PyVersion } } + const string RunSysPropName = "__pythonnet_run__"; static int run = 0; internal static int GetRun() @@ -142,7 +143,7 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd PyGILState_Ensure(); } - BorrowedReference pyRun = PySys_GetObject("__pynet_run__"); + BorrowedReference pyRun = PySys_GetObject(RunSysPropName); if (pyRun != null) { run = checked((int)PyLong_AsSignedSize_t(pyRun)); @@ -201,7 +202,7 @@ static void NewRun() { run++; using var pyRun = PyLong_FromLongLong(run); - PySys_SetObject("__pynet_run__", pyRun); + PySys_SetObject(RunSysPropName, pyRun); } private static void InitPyMembers()