diff --git a/CHANGELOG.md b/CHANGELOG.md index 17c1780ee..d611b2656 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,10 +21,12 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. - Catches exceptions thrown in C# iterators (yield returns) and rethrows them in python ([#475][i475])([#693][p693]) - Implemented GetDynamicMemberNames() for PyObject to allow dynamic object members to be visible in the debugger ([#443][i443])([#690][p690]) - Incorporated reference-style links to issues and pull requests in the CHANGELOG ([#608][i608]) +- Added PyObject finalizer support, Python objects referred by C# can be auto collect now ([#692][p692]). - Added detailed comments about aproaches and dangers to handle multi-app-domains ([#625][p625]) - Python 3.7 support, builds and testing added. Defaults changed from Python 3.6 to 3.7 ([#698][p698]) ### Changed +- PythonException included C# call stack - Reattach python exception traceback information (#545) - PythonEngine.Intialize will now call `Py_InitializeEx` with a default value of 0, so signals will not be configured by default on embedding. This is different from the previous behaviour, where `Py_Initialize` was called instead, which sets initSigs to 1. ([#449][i449]) diff --git a/src/embed_tests/Python.EmbeddingTest.csproj b/src/embed_tests/Python.EmbeddingTest.csproj index 6aa48becc..faa55fa27 100644 --- a/src/embed_tests/Python.EmbeddingTest.csproj +++ b/src/embed_tests/Python.EmbeddingTest.csproj @@ -88,6 +88,7 @@ + diff --git a/src/embed_tests/TestFinalizer.cs b/src/embed_tests/TestFinalizer.cs new file mode 100644 index 000000000..bb90c92cf --- /dev/null +++ b/src/embed_tests/TestFinalizer.cs @@ -0,0 +1,239 @@ +using NUnit.Framework; +using Python.Runtime; +using System; +using System.Linq; +using System.Threading; + +namespace Python.EmbeddingTest +{ + public class TestFinalizer + { + private int _oldThreshold; + + [SetUp] + public void SetUp() + { + _oldThreshold = Finalizer.Instance.Threshold; + PythonEngine.Initialize(); + Exceptions.Clear(); + } + + [TearDown] + public void TearDown() + { + Finalizer.Instance.Threshold = _oldThreshold; + PythonEngine.Shutdown(); + } + + private static void FullGCCollect() + { + GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced); + GC.WaitForPendingFinalizers(); + } + + [Test] + public void CollectBasicObject() + { + Assert.IsTrue(Finalizer.Instance.Enable); + + int thId = Thread.CurrentThread.ManagedThreadId; + Finalizer.Instance.Threshold = 1; + bool called = false; + EventHandler handler = (s, e) => + { + Assert.AreEqual(thId, Thread.CurrentThread.ManagedThreadId); + Assert.GreaterOrEqual(e.ObjectCount, 1); + called = true; + }; + + WeakReference shortWeak; + WeakReference longWeak; + { + MakeAGarbage(out shortWeak, out longWeak); + } + FullGCCollect(); + // The object has been resurrected + Assert.IsFalse(shortWeak.IsAlive); + Assert.IsTrue(longWeak.IsAlive); + + { + var garbage = Finalizer.Instance.GetCollectedObjects(); + Assert.NotZero(garbage.Count); + Assert.IsTrue(garbage.Any(T => ReferenceEquals(T.Target, longWeak.Target))); + } + + Assert.IsFalse(called); + Finalizer.Instance.CollectOnce += handler; + try + { + Finalizer.Instance.CallPendingFinalizers(); + } + finally + { + Finalizer.Instance.CollectOnce -= handler; + } + Assert.IsTrue(called); + } + + private static void MakeAGarbage(out WeakReference shortWeak, out WeakReference longWeak) + { + PyLong obj = new PyLong(1024); + shortWeak = new WeakReference(obj); + longWeak = new WeakReference(obj, true); + obj = null; + } + + private static long CompareWithFinalizerOn(PyObject pyCollect, bool enbale) + { + // Must larger than 512 bytes make sure Python use + string str = new string('1', 1024); + Finalizer.Instance.Enable = true; + FullGCCollect(); + FullGCCollect(); + pyCollect.Invoke(); + Finalizer.Instance.Collect(); + Finalizer.Instance.Enable = enbale; + + // Estimate unmanaged memory size + long before = Environment.WorkingSet - GC.GetTotalMemory(true); + for (int i = 0; i < 10000; i++) + { + // Memory will leak when disable Finalizer + new PyString(str); + } + FullGCCollect(); + FullGCCollect(); + pyCollect.Invoke(); + if (enbale) + { + Finalizer.Instance.Collect(); + } + + FullGCCollect(); + FullGCCollect(); + long after = Environment.WorkingSet - GC.GetTotalMemory(true); + return after - before; + + } + + /// + /// Because of two vms both have their memory manager, + /// this test only prove the finalizer has take effect. + /// + [Test] + [Ignore("Too many uncertainties, only manual on when debugging")] + public void SimpleTestMemory() + { + bool oldState = Finalizer.Instance.Enable; + try + { + using (PyObject gcModule = PythonEngine.ImportModule("gc")) + using (PyObject pyCollect = gcModule.GetAttr("collect")) + { + long span1 = CompareWithFinalizerOn(pyCollect, false); + long span2 = CompareWithFinalizerOn(pyCollect, true); + Assert.Less(span2, span1); + } + } + finally + { + Finalizer.Instance.Enable = oldState; + } + } + + class MyPyObject : PyObject + { + public MyPyObject(IntPtr op) : base(op) + { + } + + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + GC.SuppressFinalize(this); + throw new Exception("MyPyObject"); + } + internal static void CreateMyPyObject(IntPtr op) + { + Runtime.Runtime.XIncref(op); + new MyPyObject(op); + } + } + + [Test] + public void ErrorHandling() + { + bool called = false; + EventHandler handleFunc = (sender, args) => + { + called = true; + Assert.AreEqual(args.Error.Message, "MyPyObject"); + }; + Finalizer.Instance.Threshold = 1; + Finalizer.Instance.ErrorHandler += handleFunc; + try + { + WeakReference shortWeak; + WeakReference longWeak; + { + MakeAGarbage(out shortWeak, out longWeak); + var obj = (PyLong)longWeak.Target; + IntPtr handle = obj.Handle; + shortWeak = null; + longWeak = null; + MyPyObject.CreateMyPyObject(handle); + obj.Dispose(); + obj = null; + } + FullGCCollect(); + Finalizer.Instance.Collect(); + Assert.IsTrue(called); + } + finally + { + Finalizer.Instance.ErrorHandler -= handleFunc; + } + } + + [Test] + public void ValidateRefCount() + { + if (!Finalizer.Instance.RefCountValidationEnabled) + { + Assert.Pass("Only run with FINALIZER_CHECK"); + } + IntPtr ptr = IntPtr.Zero; + bool called = false; + Finalizer.IncorrectRefCntHandler handler = (s, e) => + { + called = true; + Assert.AreEqual(ptr, e.Handle); + Assert.AreEqual(2, e.ImpactedObjects.Count); + // Fix for this test, don't do this on general environment + Runtime.Runtime.XIncref(e.Handle); + return false; + }; + Finalizer.Instance.IncorrectRefCntResolver += handler; + try + { + ptr = CreateStringGarbage(); + FullGCCollect(); + Assert.Throws(() => Finalizer.Instance.Collect()); + Assert.IsTrue(called); + } + finally + { + Finalizer.Instance.IncorrectRefCntResolver -= handler; + } + } + + private static IntPtr CreateStringGarbage() + { + PyString s1 = new PyString("test_string"); + // s2 steal a reference from s1 + PyString s2 = new PyString(s1.Handle); + return s1.Handle; + } + + } +} diff --git a/src/embed_tests/TestPyAnsiString.cs b/src/embed_tests/TestPyAnsiString.cs index 9ba7d6cc6..b4a965ff7 100644 --- a/src/embed_tests/TestPyAnsiString.cs +++ b/src/embed_tests/TestPyAnsiString.cs @@ -63,6 +63,7 @@ public void TestCtorPtr() const string expected = "foo"; var t = new PyAnsiString(expected); + Runtime.Runtime.XIncref(t.Handle); var actual = new PyAnsiString(t.Handle); Assert.AreEqual(expected, actual.ToString()); diff --git a/src/embed_tests/TestPyFloat.cs b/src/embed_tests/TestPyFloat.cs index f2c85a77f..94e7026c7 100644 --- a/src/embed_tests/TestPyFloat.cs +++ b/src/embed_tests/TestPyFloat.cs @@ -25,6 +25,7 @@ public void Dispose() public void IntPtrCtor() { var i = new PyFloat(1); + Runtime.Runtime.XIncref(i.Handle); var ii = new PyFloat(i.Handle); Assert.AreEqual(i.Handle, ii.Handle); } diff --git a/src/embed_tests/TestPyInt.cs b/src/embed_tests/TestPyInt.cs index 4117336d8..005ab466d 100644 --- a/src/embed_tests/TestPyInt.cs +++ b/src/embed_tests/TestPyInt.cs @@ -86,6 +86,7 @@ public void TestCtorSByte() public void TestCtorPtr() { var i = new PyInt(5); + Runtime.Runtime.XIncref(i.Handle); var a = new PyInt(i.Handle); Assert.AreEqual(5, a.ToInt32()); } @@ -94,6 +95,7 @@ public void TestCtorPtr() public void TestCtorPyObject() { var i = new PyInt(5); + Runtime.Runtime.XIncref(i.Handle); var a = new PyInt(i); Assert.AreEqual(5, a.ToInt32()); } diff --git a/src/embed_tests/TestPyLong.cs b/src/embed_tests/TestPyLong.cs index fe3e13ef5..3c155f315 100644 --- a/src/embed_tests/TestPyLong.cs +++ b/src/embed_tests/TestPyLong.cs @@ -102,6 +102,7 @@ public void TestCtorDouble() public void TestCtorPtr() { var i = new PyLong(5); + Runtime.Runtime.XIncref(i.Handle); var a = new PyLong(i.Handle); Assert.AreEqual(5, a.ToInt32()); } @@ -110,6 +111,7 @@ public void TestCtorPtr() public void TestCtorPyObject() { var i = new PyLong(5); + Runtime.Runtime.XIncref(i.Handle); var a = new PyLong(i); Assert.AreEqual(5, a.ToInt32()); } diff --git a/src/embed_tests/TestPyScope.cs b/src/embed_tests/TestPyScope.cs index 49c15a3a1..21c0d2b3f 100644 --- a/src/embed_tests/TestPyScope.cs +++ b/src/embed_tests/TestPyScope.cs @@ -293,24 +293,27 @@ public void TestImportScopeByName() [Test] public void TestVariables() { - (ps.Variables() as dynamic)["ee"] = new PyInt(200); - var a0 = ps.Get("ee"); - Assert.AreEqual(200, a0); + using (Py.GIL()) + { + (ps.Variables() as dynamic)["ee"] = new PyInt(200); + var a0 = ps.Get("ee"); + Assert.AreEqual(200, a0); - ps.Exec("locals()['ee'] = 210"); - var a1 = ps.Get("ee"); - Assert.AreEqual(210, a1); + ps.Exec("locals()['ee'] = 210"); + var a1 = ps.Get("ee"); + Assert.AreEqual(210, a1); - ps.Exec("globals()['ee'] = 220"); - var a2 = ps.Get("ee"); - Assert.AreEqual(220, a2); + ps.Exec("globals()['ee'] = 220"); + var a2 = ps.Get("ee"); + Assert.AreEqual(220, a2); - using (var item = ps.Variables()) - { - item["ee"] = new PyInt(230); + using (var item = ps.Variables()) + { + item["ee"] = new PyInt(230); + } + var a3 = ps.Get("ee"); + Assert.AreEqual(230, a3); } - var a3 = ps.Get("ee"); - Assert.AreEqual(230, a3); } /// @@ -324,49 +327,55 @@ public void TestThread() //should be removed. dynamic _ps = ps; var ts = PythonEngine.BeginAllowThreads(); - using (Py.GIL()) - { - _ps.res = 0; - _ps.bb = 100; - _ps.th_cnt = 0; - //add function to the scope - //can be call many times, more efficient than ast - ps.Exec( - "def update():\n" + - " global res, th_cnt\n" + - " res += bb + 1\n" + - " th_cnt += 1\n" - ); - } - int th_cnt = 3; - for (int i =0; i< th_cnt; i++) + try { - System.Threading.Thread th = new System.Threading.Thread(()=> + using (Py.GIL()) + { + _ps.res = 0; + _ps.bb = 100; + _ps.th_cnt = 0; + //add function to the scope + //can be call many times, more efficient than ast + ps.Exec( + "def update():\n" + + " global res, th_cnt\n" + + " res += bb + 1\n" + + " th_cnt += 1\n" + ); + } + int th_cnt = 3; + for (int i = 0; i < th_cnt; i++) + { + System.Threading.Thread th = new System.Threading.Thread(() => + { + using (Py.GIL()) + { + //ps.GetVariable("update")(); //call the scope function dynamicly + _ps.update(); + } + }); + th.Start(); + } + //equivalent to Thread.Join, make the main thread join the GIL competition + int cnt = 0; + while (cnt != th_cnt) { using (Py.GIL()) { - //ps.GetVariable("update")(); //call the scope function dynamicly - _ps.update(); + cnt = ps.Get("th_cnt"); } - }); - th.Start(); - } - //equivalent to Thread.Join, make the main thread join the GIL competition - int cnt = 0; - while(cnt != th_cnt) - { + System.Threading.Thread.Sleep(10); + } using (Py.GIL()) { - cnt = ps.Get("th_cnt"); + var result = ps.Get("res"); + Assert.AreEqual(101 * th_cnt, result); } - System.Threading.Thread.Sleep(10); } - using (Py.GIL()) + finally { - var result = ps.Get("res"); - Assert.AreEqual(101* th_cnt, result); + PythonEngine.EndAllowThreads(ts); } - PythonEngine.EndAllowThreads(ts); } } } diff --git a/src/embed_tests/TestPyString.cs b/src/embed_tests/TestPyString.cs index 9d1cdb0e9..0de436e35 100644 --- a/src/embed_tests/TestPyString.cs +++ b/src/embed_tests/TestPyString.cs @@ -64,6 +64,7 @@ public void TestCtorPtr() const string expected = "foo"; var t = new PyString(expected); + Runtime.Runtime.XIncref(t.Handle); var actual = new PyString(t.Handle); Assert.AreEqual(expected, actual.ToString()); diff --git a/src/runtime/Python.Runtime.15.csproj b/src/runtime/Python.Runtime.15.csproj index 794645994..29177b78c 100644 --- a/src/runtime/Python.Runtime.15.csproj +++ b/src/runtime/Python.Runtime.15.csproj @@ -68,10 +68,10 @@ $(DefineConstants);PYTHON3;$(Python3Version);$(PythonMonoDefineConstants) - $(DefineConstants);PYTHON2;$(Python2Version);$(PythonMonoDefineConstants);TRACE;DEBUG + $(DefineConstants);PYTHON2;$(Python2Version);$(PythonMonoDefineConstants);FINALIZER_CHECK;TRACE;DEBUG - $(DefineConstants);PYTHON3;$(Python3Version);$(PythonMonoDefineConstants);TRACE;DEBUG + $(DefineConstants);PYTHON3;$(Python3Version);$(PythonMonoDefineConstants);FINALIZER_CHECK;TRACE;DEBUG $(DefineConstants);PYTHON2;$(Python2Version);$(PythonWinDefineConstants) @@ -80,10 +80,10 @@ $(DefineConstants);PYTHON3;$(Python3Version);$(PythonWinDefineConstants) - $(DefineConstants);PYTHON2;$(Python2Version);$(PythonWinDefineConstants);TRACE;DEBUG + $(DefineConstants);PYTHON2;$(Python2Version);$(PythonWinDefineConstants);FINALIZER_CHECK;TRACE;DEBUG - $(DefineConstants);PYTHON3;$(Python3Version);$(PythonWinDefineConstants);TRACE;DEBUG + $(DefineConstants);PYTHON3;$(Python3Version);$(PythonWinDefineConstants);FINALIZER_CHECK;TRACE;DEBUG diff --git a/src/runtime/Python.Runtime.csproj b/src/runtime/Python.Runtime.csproj index fc155ca91..19f776c77 100644 --- a/src/runtime/Python.Runtime.csproj +++ b/src/runtime/Python.Runtime.csproj @@ -1,170 +1,171 @@ - - - - Debug - AnyCPU - {097B4AC0-74E9-4C58-BCF8-C69746EC8271} - Library - Python.Runtime - Python.Runtime - bin\Python.Runtime.xml - bin\ - v4.0 - - 1591 - ..\..\ - $(SolutionDir)\bin\ - Properties - 6 - true - false - ..\pythonnet.snk - - - + + + + Debug + AnyCPU + {097B4AC0-74E9-4C58-BCF8-C69746EC8271} + Library + Python.Runtime + Python.Runtime + bin\Python.Runtime.xml + bin\ + v4.0 + + 1591 + ..\..\ + $(SolutionDir)\bin\ + Properties + 6 + true + false + ..\pythonnet.snk + + + - - PYTHON2;PYTHON27;UCS4 - true - pdbonly - - + --> + + PYTHON2;PYTHON27;UCS4 + true + pdbonly + + PYTHON3;PYTHON37;UCS4 - true - pdbonly - - - true - PYTHON2;PYTHON27;UCS4;TRACE;DEBUG - false - full - - - true + true + pdbonly + + + true + PYTHON2;PYTHON27;UCS4;TRACE;DEBUG + false + full + + + true PYTHON3;PYTHON37;UCS4;TRACE;DEBUG - false - full - - - PYTHON2;PYTHON27;UCS2 - true - pdbonly - - + false + full + + + PYTHON2;PYTHON27;UCS2 + true + pdbonly + + PYTHON3;PYTHON37;UCS2 - true - pdbonly - - - true - PYTHON2;PYTHON27;UCS2;TRACE;DEBUG - false - full - - - true + true + pdbonly + + + true + PYTHON2;PYTHON27;UCS2;TRACE;DEBUG + false + full + + + true PYTHON3;PYTHON37;UCS2;TRACE;DEBUG - false - full - - - - - - - - Properties\SharedAssemblyInfo.cs - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + false + full + + + + + + + + + Properties\SharedAssemblyInfo.cs + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - clr.py - - - - - $(TargetPath) - $(TargetDir)$(TargetName).pdb - - - - - + + + + + + + clr.py + + + + + $(TargetPath) + $(TargetDir)$(TargetName).pdb + + + + + diff --git a/src/runtime/delegatemanager.cs b/src/runtime/delegatemanager.cs index 7632816d1..bd8f1ee4c 100644 --- a/src/runtime/delegatemanager.cs +++ b/src/runtime/delegatemanager.cs @@ -181,10 +181,12 @@ A possible alternate strategy would be to create custom subclasses too "special" for this to work. It would be more work, so for now the 80/20 rule applies :) */ - public class Dispatcher + public class Dispatcher : IPyDisposable { public IntPtr target; public Type dtype; + private bool _disposed = false; + private bool _finalized = false; public Dispatcher(IntPtr target, Type dtype) { @@ -195,18 +197,25 @@ public Dispatcher(IntPtr target, Type dtype) ~Dispatcher() { - // We needs to disable Finalizers until it's valid implementation. - // Current implementation can produce low probability floating bugs. - return; + if (_finalized || _disposed) + { + return; + } + _finalized = true; + Finalizer.Instance.AddFinalizedObject(this); + } - // Note: the managed GC thread can run and try to free one of - // these *after* the Python runtime has been finalized! - if (Runtime.Py_IsInitialized() > 0) + public void Dispose() + { + if (_disposed) { - IntPtr gs = PythonEngine.AcquireLock(); - Runtime.XDecref(target); - PythonEngine.ReleaseLock(gs); + return; } + _disposed = true; + Runtime.XDecref(target); + target = IntPtr.Zero; + dtype = null; + GC.SuppressFinalize(this); } public object Dispatch(ArrayList args) @@ -267,6 +276,11 @@ public object TrueDispatch(ArrayList args) Runtime.XDecref(op); return result; } + + public IntPtr[] GetTrackedHandles() + { + return new IntPtr[] { target }; + } } diff --git a/src/runtime/finalizer.cs b/src/runtime/finalizer.cs new file mode 100644 index 000000000..a94f7be31 --- /dev/null +++ b/src/runtime/finalizer.cs @@ -0,0 +1,331 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Runtime.InteropServices; +using System.Threading; + +namespace Python.Runtime +{ + public class Finalizer + { + public class CollectArgs : EventArgs + { + public int ObjectCount { get; set; } + } + + public class ErrorArgs : EventArgs + { + public Exception Error { get; set; } + } + + public static readonly Finalizer Instance = new Finalizer(); + + public event EventHandler CollectOnce; + public event EventHandler ErrorHandler; + + public int Threshold { get; set; } + public bool Enable { get; set; } + + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] + struct PendingArgs + { + public bool cancelled; + } + + [UnmanagedFunctionPointer(CallingConvention.Cdecl)] + private delegate int PendingCall(IntPtr arg); + private readonly PendingCall _collectAction; + + private ConcurrentQueue _objQueue = new ConcurrentQueue(); + private bool _pending = false; + private readonly object _collectingLock = new object(); + private IntPtr _pendingArgs; + + #region FINALIZER_CHECK + +#if FINALIZER_CHECK + private readonly object _queueLock = new object(); + public bool RefCountValidationEnabled { get; set; } = true; +#else + public readonly bool RefCountValidationEnabled = false; +#endif + // Keep these declarations for compat even no FINALIZER_CHECK + public class IncorrectFinalizeArgs : EventArgs + { + public IntPtr Handle { get; internal set; } + public ICollection ImpactedObjects { get; internal set; } + } + + public class IncorrectRefCountException : Exception + { + public IntPtr PyPtr { get; internal set; } + private string _message; + public override string Message => _message; + + public IncorrectRefCountException(IntPtr ptr) + { + PyPtr = ptr; + IntPtr pyname = Runtime.PyObject_Unicode(PyPtr); + string name = Runtime.GetManagedString(pyname); + Runtime.XDecref(pyname); + _message = $"{name} may has a incorrect ref count"; + } + } + + public delegate bool IncorrectRefCntHandler(object sender, IncorrectFinalizeArgs e); + public event IncorrectRefCntHandler IncorrectRefCntResolver; + public bool ThrowIfUnhandleIncorrectRefCount { get; set; } = true; + + #endregion + + private Finalizer() + { + Enable = true; + Threshold = 200; + _collectAction = OnPendingCollect; + } + + public void CallPendingFinalizers() + { + if (Thread.CurrentThread.ManagedThreadId != Runtime.MainManagedThreadId) + { + throw new Exception("PendingCall should execute in main Python thread"); + } + Runtime.Py_MakePendingCalls(); + } + + public void Collect() + { + using (var gilState = new Py.GILState()) + { + DisposeAll(); + } + } + + public List GetCollectedObjects() + { + return _objQueue.Select(T => new WeakReference(T)).ToList(); + } + + internal void AddFinalizedObject(IPyDisposable obj) + { + if (!Enable) + { + return; + } + if (Runtime.Py_IsInitialized() == 0) + { + // XXX: Memory will leak if a PyObject finalized after Python shutdown, + // for avoiding that case, user should call GC.Collect manual before shutdown. + return; + } +#if FINALIZER_CHECK + lock (_queueLock) +#endif + { + _objQueue.Enqueue(obj); + } + GC.ReRegisterForFinalize(obj); + if (_objQueue.Count >= Threshold) + { + AddPendingCollect(); + } + } + + internal static void Shutdown() + { + if (Runtime.Py_IsInitialized() == 0) + { + Instance._objQueue = new ConcurrentQueue(); + return; + } + Instance.DisposeAll(); + if (Thread.CurrentThread.ManagedThreadId != Runtime.MainManagedThreadId) + { + if (Instance._pendingArgs == IntPtr.Zero) + { + Instance.ResetPending(); + return; + } + // Not in main thread just cancel the pending operation to avoid error in different domain + // It will make a memory leak + unsafe + { + PendingArgs* args = (PendingArgs*)Instance._pendingArgs; + args->cancelled = true; + } + Instance.ResetPending(); + return; + } + Instance.CallPendingFinalizers(); + } + + private void AddPendingCollect() + { + if (_pending) + { + return; + } + lock (_collectingLock) + { + if (_pending) + { + return; + } + _pending = true; + var args = new PendingArgs() { cancelled = false }; + IntPtr p = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(PendingArgs))); + Marshal.StructureToPtr(args, p, false); + _pendingArgs = p; + IntPtr func = Marshal.GetFunctionPointerForDelegate(_collectAction); + if (Runtime.Py_AddPendingCall(func, p) != 0) + { + // Full queue, append next time + _pending = false; + } + } + } + + private static int OnPendingCollect(IntPtr arg) + { + Debug.Assert(arg == Instance._pendingArgs); + try + { + unsafe + { + PendingArgs* pendingArgs = (PendingArgs*)arg; + if (pendingArgs->cancelled) + { + return 0; + } + } + Instance.DisposeAll(); + } + finally + { + Instance.ResetPending(); + Marshal.FreeHGlobal(arg); + } + return 0; + } + + private void DisposeAll() + { + CollectOnce?.Invoke(this, new CollectArgs() + { + ObjectCount = _objQueue.Count + }); +#if FINALIZER_CHECK + lock (_queueLock) +#endif + { +#if FINALIZER_CHECK + ValidateRefCount(); +#endif + IPyDisposable obj; + while (_objQueue.TryDequeue(out obj)) + { + try + { + obj.Dispose(); + Runtime.CheckExceptionOccurred(); + } + catch (Exception e) + { + // We should not bother the main thread + ErrorHandler?.Invoke(this, new ErrorArgs() + { + Error = e + }); + } + } + } + } + + private void ResetPending() + { + lock (_collectingLock) + { + _pending = false; + _pendingArgs = IntPtr.Zero; + } + } + +#if FINALIZER_CHECK + private void ValidateRefCount() + { + if (!RefCountValidationEnabled) + { + return; + } + var counter = new Dictionary(); + var holdRefs = new Dictionary(); + var indexer = new Dictionary>(); + foreach (var obj in _objQueue) + { + IntPtr[] handles = obj.GetTrackedHandles(); + foreach (var handle in handles) + { + if (handle == IntPtr.Zero) + { + continue; + } + if (!counter.ContainsKey(handle)) + { + counter[handle] = 0; + } + counter[handle]++; + if (!holdRefs.ContainsKey(handle)) + { + holdRefs[handle] = Runtime.Refcount(handle); + } + List objs; + if (!indexer.TryGetValue(handle, out objs)) + { + objs = new List(); + indexer.Add(handle, objs); + } + objs.Add(obj); + } + } + foreach (var pair in counter) + { + IntPtr handle = pair.Key; + long cnt = pair.Value; + // Tracked handle's ref count is larger than the object's holds + // it may take an unspecified behaviour if it decref in Dispose + if (cnt > holdRefs[handle]) + { + var args = new IncorrectFinalizeArgs() + { + Handle = handle, + ImpactedObjects = indexer[handle] + }; + bool handled = false; + if (IncorrectRefCntResolver != null) + { + var funcList = IncorrectRefCntResolver.GetInvocationList(); + foreach (IncorrectRefCntHandler func in funcList) + { + if (func(this, args)) + { + handled = true; + break; + } + } + } + if (!handled && ThrowIfUnhandleIncorrectRefCount) + { + throw new IncorrectRefCountException(handle); + } + } + // Make sure no other references for PyObjects after this method + indexer[handle].Clear(); + } + indexer.Clear(); + } +#endif + } +} diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index 7dca85545..c86504802 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -1,11 +1,17 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics; using System.Dynamic; using System.Linq.Expressions; namespace Python.Runtime { + public interface IPyDisposable : IDisposable + { + IntPtr[] GetTrackedHandles(); + } + /// /// Represents a generic Python object. The methods of this class are /// generally equivalent to the Python "abstract object API". See @@ -13,10 +19,18 @@ namespace Python.Runtime /// PY3: https://docs.python.org/3/c-api/object.html /// for details. /// - public class PyObject : DynamicObject, IEnumerable, IDisposable + public class PyObject : DynamicObject, IEnumerable, IPyDisposable { +#if TRACE_ALLOC + /// + /// Trace stack for PyObject's construction + /// + public StackTrace Traceback { get; private set; } +#endif + protected internal IntPtr obj = IntPtr.Zero; private bool disposed = false; + private bool _finalized = false; /// /// PyObject Constructor @@ -30,6 +44,9 @@ public class PyObject : DynamicObject, IEnumerable, IDisposable public PyObject(IntPtr ptr) { obj = ptr; +#if TRACE_ALLOC + Traceback = new StackTrace(1); +#endif } // Protected default constructor to allow subclasses to manage @@ -37,18 +54,26 @@ public PyObject(IntPtr ptr) protected PyObject() { +#if TRACE_ALLOC + Traceback = new StackTrace(1); +#endif } // Ensure that encapsulated Python object is decref'ed appropriately // when the managed wrapper is garbage-collected. - ~PyObject() { - // We needs to disable Finalizers until it's valid implementation. - // Current implementation can produce low probability floating bugs. - return; - - Dispose(); + if (obj == IntPtr.Zero) + { + return; + } + if (_finalized || disposed) + { + return; + } + // Prevent a infinity loop by calling GC.WaitForPendingFinalizers + _finalized = true; + Finalizer.Instance.AddFinalizedObject(this); } @@ -156,6 +181,10 @@ public void Dispose() GC.SuppressFinalize(this); } + public IntPtr[] GetTrackedHandles() + { + return new IntPtr[] { obj }; + } /// /// GetPythonType Method diff --git a/src/runtime/pyscope.cs b/src/runtime/pyscope.cs index 8e6957855..4008ce29a 100644 --- a/src/runtime/pyscope.cs +++ b/src/runtime/pyscope.cs @@ -22,7 +22,7 @@ public class PyGILAttribute : Attribute } [PyGIL] - public class PyScope : DynamicObject, IDisposable + public class PyScope : DynamicObject, IPyDisposable { public readonly string Name; @@ -37,6 +37,7 @@ public class PyScope : DynamicObject, IDisposable internal readonly IntPtr variables; private bool _isDisposed; + private bool _finalized = false; /// /// The Manager this scope associated with. @@ -525,13 +526,19 @@ public void Dispose() this.OnDispose?.Invoke(this); } - ~PyScope() + public IntPtr[] GetTrackedHandles() { - // We needs to disable Finalizers until it's valid implementation. - // Current implementation can produce low probability floating bugs. - return; + return new IntPtr[] { obj }; + } - Dispose(); + ~PyScope() + { + if (_finalized || _isDisposed) + { + return; + } + _finalized = true; + Finalizer.Instance.AddFinalizedObject(this); } } diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index ded7fbeb5..295a63b3d 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -6,7 +6,7 @@ namespace Python.Runtime /// Provides a managed interface to exceptions thrown by the Python /// runtime. /// - public class PythonException : System.Exception + public class PythonException : System.Exception, IPyDisposable { private IntPtr _pyType = IntPtr.Zero; private IntPtr _pyValue = IntPtr.Zero; @@ -15,14 +15,12 @@ public class PythonException : System.Exception private string _message = ""; private string _pythonTypeName = ""; private bool disposed = false; + private bool _finalized = false; public PythonException() { IntPtr gs = PythonEngine.AcquireLock(); Runtime.PyErr_Fetch(ref _pyType, ref _pyValue, ref _pyTB); - Runtime.XIncref(_pyType); - Runtime.XIncref(_pyValue); - Runtime.XIncref(_pyTB); if (_pyType != IntPtr.Zero && _pyValue != IntPtr.Zero) { string type; @@ -45,11 +43,13 @@ public PythonException() } if (_pyTB != IntPtr.Zero) { - PyObject tb_module = PythonEngine.ImportModule("traceback"); - Runtime.XIncref(_pyTB); - using (var pyTB = new PyObject(_pyTB)) + using (PyObject tb_module = PythonEngine.ImportModule("traceback")) { - _tb = tb_module.InvokeMethod("format_tb", pyTB).ToString(); + Runtime.XIncref(_pyTB); + using (var pyTB = new PyObject(_pyTB)) + { + _tb = tb_module.InvokeMethod("format_tb", pyTB).ToString(); + } } } PythonEngine.ReleaseLock(gs); @@ -60,11 +60,12 @@ public PythonException() ~PythonException() { - // We needs to disable Finalizers until it's valid implementation. - // Current implementation can produce low probability floating bugs. - return; - - Dispose(); + if (_finalized || disposed) + { + return; + } + _finalized = true; + Finalizer.Instance.AddFinalizedObject(this); } /// @@ -132,7 +133,7 @@ public override string Message /// public override string StackTrace { - get { return _tb; } + get { return _tb + base.StackTrace; } } /// @@ -173,6 +174,11 @@ public void Dispose() } } + public IntPtr[] GetTrackedHandles() + { + return new IntPtr[] { _pyType, _pyValue, _pyTB }; + } + /// /// Matches Method /// diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index d4cb85583..7623200e0 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -2,6 +2,7 @@ using System.Runtime.InteropServices; using System.Security; using System.Text; +using System.Threading; using System.Collections.Generic; namespace Python.Runtime @@ -261,6 +262,8 @@ public enum MachineType internal static bool IsPython2 = pyversionnumber < 30; internal static bool IsPython3 = pyversionnumber >= 30; + public static int MainManagedThreadId { get; private set; } + /// /// Encoding to use to convert Unicode to/from Managed to Native /// @@ -274,6 +277,7 @@ internal static void Initialize(bool initSigs = false) if (Py_IsInitialized() == 0) { Py_InitializeEx(initSigs ? 1 : 0); + MainManagedThreadId = Thread.CurrentThread.ManagedThreadId; } if (PyEval_ThreadsInitialized() == 0) @@ -476,6 +480,7 @@ internal static void Shutdown() AssemblyManager.Shutdown(); Exceptions.Shutdown(); ImportHook.Shutdown(); + Finalizer.Shutdown(); Py_Finalize(); } @@ -1953,5 +1958,11 @@ internal static IntPtr PyMem_Realloc(IntPtr ptr, long size) [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern IntPtr PyMethod_Function(IntPtr ob); + + [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] + internal static extern int Py_AddPendingCall(IntPtr func, IntPtr arg); + + [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] + internal static extern int Py_MakePendingCalls(); } }