diff --git a/.travis.yml b/.travis.yml index 025229d04..4869b81de 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,6 @@ matrix: - python: 2.7 env: &xplat-env - BUILD_OPTS=--xplat - - NUNIT_PATH=~/.nuget/packages/nunit.consolerunner/3.*/tools/nunit3-console.exe addons: &xplat-addons apt: sources: @@ -45,7 +44,6 @@ matrix: - python: 2.7 env: &classic-env - BUILD_OPTS= - - NUNIT_PATH=./packages/NUnit.*/tools/nunit3-console.exe - python: 3.3 env: *classic-env @@ -97,7 +95,7 @@ install: script: - python -m pytest - - mono $NUNIT_PATH src/embed_tests/bin/Python.EmbeddingTest.dll + - mono src/embed_tests/bin/Python.EmbeddingTest.exe - if [[ $BUILD_OPTS == --xplat ]]; then dotnet src/embed_tests/bin/netcoreapp2.0_publish/Python.EmbeddingTest.dll; fi after_script: diff --git a/CHANGELOG.md b/CHANGELOG.md index 380c6f6d3..aad78e95e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,14 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ## [unreleased][] ### Added +- Added tool for debugging floating bugs. Stable tests are executed in the loop. ~100 cycles is enough to pop up any bugs. + Usage: Python.EmbeddingTest.exe --loop --where="cat != Unstable" - Added support for embedding python into dotnet core 2.0 (NetStandard 2.0) - Added new build system (pythonnet.15.sln) based on dotnetcore-sdk/xplat(crossplatform msbuild). Currently there two side-by-side build systems that produces the same output (net40) from the same sources. After a some transition time, current (mono/ msbuild 14.0) build system will be removed. -- NUnit upgraded to 3.7 (eliminates travis-ci random bug) +- Python.EmbeddingTest (net40) now tested through built-in NUnitLite in Travis-CI. (Solves NUnit vs Mono stability problems.) +- NUnit upgraded to 3.8.1, Python.EmbeddingTest now executable with the NUnitLite self-tester. - Added `clr.GetClrType` (#432, #433) - Allowed passing `None` for nullable args (#460) - Added keyword arguments based on C# syntax for calling CPython methods (#461) @@ -20,7 +23,11 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ### Changed ### Fixed - +- Fixed memory leaks, caused by non-working PyObject, PythonException, DelecateManager->Dispatcher finalizers. + Set environment variable PYTHONNET_GC_DEBUG=1 to force full GC on each PythonEngine.Shutdown, this allows + to esily detect GC related bugs. +- Fixed secondary PythonEngine.Initialize call, all sensitive static variables now reseted. + This is a hidden bug. Once python cleaning up enough memory, objects from previous engine run becomes corrupted. - Fixed Visual Studio 2017 compat (#434) for setup.py - Fixed crash on exit of the Python interpreter if a python class derived from a .NET class has a `__namespace__` or `__assembly__` diff --git a/NuGet.config b/NuGet.config index 719fbc83c..f270fb328 100644 --- a/NuGet.config +++ b/NuGet.config @@ -1,7 +1,7 @@  - + \ No newline at end of file diff --git a/ci/appveyor_run_tests.ps1 b/ci/appveyor_run_tests.ps1 index b45440fbe..2811c5bed 100644 --- a/ci/appveyor_run_tests.ps1 +++ b/ci/appveyor_run_tests.ps1 @@ -20,7 +20,7 @@ else{ $PY = Get-Command python # Can't use ".\build\*\Python.EmbeddingTest.dll". Missing framework files. -$CS_TESTS = ".\src\embed_tests\bin\Python.EmbeddingTest.dll" +$CS_TESTS = ".\src\embed_tests\bin\Python.EmbeddingTest.exe" $RUNTIME_DIR = ".\src\runtime\bin\" # Run python tests with C# coverage diff --git a/src/embed_tests/Program.cs b/src/embed_tests/Program.cs index b4439e3e4..a18894367 100644 --- a/src/embed_tests/Program.cs +++ b/src/embed_tests/Program.cs @@ -1,8 +1,12 @@ using System; - +using System.Collections; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; using NUnit.Common; using NUnitLite; +using Python.Runtime; namespace Python.EmbeddingTest { @@ -10,6 +14,36 @@ public class Program { public static int Main(string[] args) { + if (args.Contains("--loop")) + { + args = args.Where(x => x != "--loop").ToArray(); + int result; + int runNumber = 0; + string pathEnv = Environment.GetEnvironmentVariable("PATH"); + do + { + Console.ForegroundColor = ConsoleColor.Green; + Console.Error.WriteLine($"----- Run = {++runNumber}, MemUsage = {Process.GetCurrentProcess().WorkingSet64 / 1024 / 1024} Mb -----"); + Console.ForegroundColor = ConsoleColor.Gray; + + result = new AutoRun(typeof(Program).Assembly).Execute( + args, + new ExtendedTextWrapper(Console.Out), + Console.In); + + // Python does not see Environment.SetEnvironmentVariable changes. + // So we needs restore PATH variable in a pythonic way. + using (new PythonEngine()) + { + dynamic os = PythonEngine.ImportModule("os"); + os.environ["PATH"] = new PyString(pathEnv); + } + } + while (!Console.KeyAvailable); + + return result; + } + return new AutoRun(typeof(Program).Assembly).Execute( args, new ExtendedTextWrapper(Console.Out), diff --git a/src/embed_tests/Python.EmbeddingTest.15.csproj b/src/embed_tests/Python.EmbeddingTest.15.csproj index 92d55a7e0..24bead01f 100644 --- a/src/embed_tests/Python.EmbeddingTest.15.csproj +++ b/src/embed_tests/Python.EmbeddingTest.15.csproj @@ -5,7 +5,7 @@ net40;netcoreapp2.0 x64;x86 DebugMono;DebugMonoPY3;ReleaseMono;ReleaseMonoPY3;DebugWin;DebugWinPY3;ReleaseWin;ReleaseWinPY3 - Exe + Exe false Python.EmbeddingTest Python.EmbeddingTest @@ -76,10 +76,10 @@ - + - + diff --git a/src/embed_tests/Python.EmbeddingTest.csproj b/src/embed_tests/Python.EmbeddingTest.csproj index fe02b0526..bbb35a8e0 100644 --- a/src/embed_tests/Python.EmbeddingTest.csproj +++ b/src/embed_tests/Python.EmbeddingTest.csproj @@ -4,7 +4,7 @@ Debug AnyCPU {4165C59D-2822-499F-A6DB-EACA4C331EB5} - Library + Exe Python.EmbeddingTest Python.EmbeddingTest bin\Python.EmbeddingTest.xml @@ -70,8 +70,11 @@ - - ..\..\packages\NUnit.3.7.1\lib\net40\nunit.framework.dll + + ..\..\packages\NUnit.3.8.1\lib\net40\nunit.framework.dll + + + ..\..\packages\NUnitLite.3.8.1\lib\net40\nunitlite.dll @@ -84,6 +87,7 @@ + @@ -102,6 +106,8 @@ + + diff --git a/src/embed_tests/TestExample.cs b/src/embed_tests/TestExample.cs index 671f9e33d..7265ffa44 100644 --- a/src/embed_tests/TestExample.cs +++ b/src/embed_tests/TestExample.cs @@ -5,6 +5,7 @@ namespace Python.EmbeddingTest { + [Category("Unstable")] public class TestExample { [OneTimeSetUp] diff --git a/src/embed_tests/TestFinalizer.cs b/src/embed_tests/TestFinalizer.cs new file mode 100644 index 000000000..a7b6432a5 --- /dev/null +++ b/src/embed_tests/TestFinalizer.cs @@ -0,0 +1,130 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; +using Python.Runtime; + +namespace Python.EmbeddingTest +{ + public class TestFinalizer + { + [OneTimeSetUp] + public void SetUp() + { + PythonEngine.Initialize(); + } + + [OneTimeTearDown] + public void Dispose() + { + PythonEngine.Shutdown(); + } + + [Test] + public void TestClrObjectFullRelease() + { + var gs = PythonEngine.BeginAllowThreads(); + + WeakReference weakRef; + try + { + var weakRefCreateTask = Task.Factory.StartNew(() => + { + using (Py.GIL()) + { + byte[] testObject = new byte[100]; + var testObjectWeakReference = new WeakReference(testObject); + + dynamic pyList = new PyList(); + pyList.append(testObject); + return testObjectWeakReference; + } + }); + + weakRef = weakRefCreateTask.Result; + } + finally + { + PythonEngine.EndAllowThreads(gs); + } + + // Triggering C# finalizer (PyList ref should be scheduled to decref). + GC.Collect(); + GC.WaitForPendingFinalizers(); + + // Forcing dec reference thread to wakeup and decref PyList. + PythonEngine.EndAllowThreads(PythonEngine.BeginAllowThreads()); + Thread.Sleep(200); + PythonEngine.CurrentRefDecrementer.WaitForPendingDecReferences(); + + // Now python free up GCHandle on CLRObject and subsequent GC should fully remove testObject. + GC.Collect(); + GC.WaitForPendingFinalizers(); + + Assert.IsFalse(weakRef.IsAlive, "Clr object should be collected."); + } + + [Test] + [Ignore("For debug only")] + public void TestExceptionMemoryLeak() + { + dynamic pymodule; + PyObject gc; + dynamic testmethod; + + var ts = PythonEngine.BeginAllowThreads(); + IntPtr pylock = PythonEngine.AcquireLock(); + +#if NETCOREAPP + const string s = "../../fixtures"; +#else + const string s = "../fixtures"; +#endif + string testPath = Path.Combine(TestContext.CurrentContext.TestDirectory, s); + + IntPtr str = Runtime.Runtime.PyString_FromString(testPath); + IntPtr path = Runtime.Runtime.PySys_GetObject("path"); + Runtime.Runtime.PyList_Append(path, str); + + { + PyObject sys = PythonEngine.ImportModule("sys"); + gc = PythonEngine.ImportModule("gc"); + + pymodule = PythonEngine.ImportModule("MemoryLeakTest.pyraise"); + testmethod = pymodule.test_raise_exception; + } + + PythonEngine.ReleaseLock(pylock); + + double floatarg1 = 5.1f; + dynamic res = null; + { + for (int i = 1; i <= 10000000; i++) + { + using (Py.GIL()) + { + try + { + res = testmethod(Py.kw("number", floatarg1), Py.kw("astring", "bbc")); + } + catch (Exception e) + { + if (i % 10000 == 0) + { + TestContext.Progress.WriteLine(e.Message); + } + } + } + + if (i % 10000 == 0) + { + GC.Collect(); + } + } + } + + PythonEngine.EndAllowThreads(ts); + } + } +} 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/TestPyLong.cs b/src/embed_tests/TestPyLong.cs index fe3e13ef5..903256f12 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()); } 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/embed_tests/TestPythonEngineProperties.cs b/src/embed_tests/TestPythonEngineProperties.cs index 01c6ae7e3..d95942577 100644 --- a/src/embed_tests/TestPythonEngineProperties.cs +++ b/src/embed_tests/TestPythonEngineProperties.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using NUnit.Framework; using Python.Runtime; @@ -109,18 +110,41 @@ public static void GetPythonHomeDefault() [Test] public void SetPythonHome() { + // We needs to ensure that engine was started and shutdown at least once before setting dummy home. + // Otherwise engine will not run with dummy path with random problem. + if (!PythonEngine.IsInitialized) + { + PythonEngine.Initialize(); + } + + PythonEngine.Shutdown(); + + var pythonHomeBackup = PythonEngine.PythonHome; + var pythonHome = "/dummypath/"; PythonEngine.PythonHome = pythonHome; PythonEngine.Initialize(); - Assert.AreEqual(pythonHome, PythonEngine.PythonHome); PythonEngine.Shutdown(); + + // Restoring valid pythonhome. + PythonEngine.PythonHome = pythonHomeBackup; } [Test] public void SetPythonHomeTwice() { + // We needs to ensure that engine was started and shutdown at least once before setting dummy home. + // Otherwise engine will not run with dummy path with random problem. + if (!PythonEngine.IsInitialized) + { + PythonEngine.Initialize(); + } + PythonEngine.Shutdown(); + + var pythonHomeBackup = PythonEngine.PythonHome; + var pythonHome = "/dummypath/"; PythonEngine.PythonHome = "/dummypath2/"; @@ -129,11 +153,20 @@ public void SetPythonHomeTwice() Assert.AreEqual(pythonHome, PythonEngine.PythonHome); PythonEngine.Shutdown(); + + PythonEngine.PythonHome = pythonHomeBackup; } [Test] public void SetProgramName() { + if (PythonEngine.IsInitialized) + { + PythonEngine.Shutdown(); + } + + var programNameBackup = PythonEngine.ProgramName; + var programName = "FooBar"; PythonEngine.ProgramName = programName; @@ -141,6 +174,8 @@ public void SetProgramName() Assert.AreEqual(programName, PythonEngine.ProgramName); PythonEngine.Shutdown(); + + PythonEngine.ProgramName = programNameBackup; } [Test] @@ -156,7 +191,7 @@ public void SetPythonPath() string path = PythonEngine.PythonPath; PythonEngine.Shutdown(); - PythonEngine.ProgramName = path; + PythonEngine.PythonPath = path; PythonEngine.Initialize(); Assert.AreEqual(path, PythonEngine.PythonPath); @@ -171,7 +206,6 @@ public void SetPythonPathExceptionOn27() Assert.Pass(); } - // Get previous path to avoid crashing Python PythonEngine.Initialize(); string path = PythonEngine.PythonPath; PythonEngine.Shutdown(); diff --git a/src/embed_tests/TestRuntime.cs b/src/embed_tests/TestRuntime.cs index 2e0598da7..9dfb04c9c 100644 --- a/src/embed_tests/TestRuntime.cs +++ b/src/embed_tests/TestRuntime.cs @@ -1,4 +1,4 @@ -using System; +using System; using NUnit.Framework; using Python.Runtime; @@ -6,10 +6,21 @@ namespace Python.EmbeddingTest { public class TestRuntime { + [OneTimeSetUp] + public void SetUp() + { + // We needs to ensure that no any engines are running. + if (PythonEngine.IsInitialized) + { + PythonEngine.Shutdown(); + } + } + [Test] public static void Py_IsInitializedValue() { - Runtime.Runtime.Py_Finalize(); // In case another test left it on. + // We know for sure that all engines are shut down. + // Runtime.Runtime.Py_Finalize(); Assert.AreEqual(0, Runtime.Runtime.Py_IsInitialized()); Runtime.Runtime.Py_Initialize(); Assert.AreEqual(1, Runtime.Runtime.Py_IsInitialized()); diff --git a/src/embed_tests/TestsSuite.cs b/src/embed_tests/TestsSuite.cs new file mode 100644 index 000000000..44ce2d4b8 --- /dev/null +++ b/src/embed_tests/TestsSuite.cs @@ -0,0 +1,18 @@ +using NUnit.Framework; +using Python.Runtime; + +namespace Python.EmbeddingTest +{ + [SetUpFixture] + public class TestsSuite + { + [OneTimeTearDown] + public void FinalCleanup() + { + if (PythonEngine.IsInitialized) + { + PythonEngine.Shutdown(); + } + } + } +} diff --git a/src/embed_tests/fixtures/MemoryLeakTest/pyraise.py b/src/embed_tests/fixtures/MemoryLeakTest/pyraise.py new file mode 100644 index 000000000..444594283 --- /dev/null +++ b/src/embed_tests/fixtures/MemoryLeakTest/pyraise.py @@ -0,0 +1,8 @@ +def test_raise_exception(number=3, astring='abc'): + raise ValueError("testing for memory leak") + return astring * int(number) + +def test_raise_exception2(number, astring): + #raise ValueError("testing for memory leak") + #astring * int(number) + return "test" diff --git a/src/embed_tests/packages.config b/src/embed_tests/packages.config index 8c175f441..aa03916a2 100644 --- a/src/embed_tests/packages.config +++ b/src/embed_tests/packages.config @@ -1,5 +1,6 @@ - + + diff --git a/src/runtime/Python.Runtime.csproj b/src/runtime/Python.Runtime.csproj index 82825a626..b36f228b2 100644 --- a/src/runtime/Python.Runtime.csproj +++ b/src/runtime/Python.Runtime.csproj @@ -129,6 +129,7 @@ + diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs index 06a4449a2..34d08f0ec 100644 --- a/src/runtime/assemblymanager.cs +++ b/src/runtime/assemblymanager.cs @@ -17,13 +17,14 @@ internal class AssemblyManager { // modified from event handlers below, potentially triggered from different .NET threads // therefore this should be a ConcurrentDictionary - private static ConcurrentDictionary> namespaces; + private static ConcurrentDictionary> namespaces = + new ConcurrentDictionary>(); //private static Dictionary> generics; private static AssemblyLoadEventHandler lhandler; private static ResolveEventHandler rhandler; // updated only under GIL? - private static Dictionary probed; + private static Dictionary probed = new Dictionary(32); // modified from event handlers below, potentially triggered from different .NET threads private static AssemblyList assemblies; @@ -40,9 +41,6 @@ private AssemblyManager() /// internal static void Initialize() { - namespaces = new ConcurrentDictionary>(); - probed = new Dictionary(32); - //generics = new Dictionary>(); assemblies = new AssemblyList(16); pypath = new List(16); diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs index 16d3b99db..ec3734ea5 100644 --- a/src/runtime/classderived.cs +++ b/src/runtime/classderived.cs @@ -1,8 +1,9 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Reflection; using System.Reflection.Emit; +using System.Resources; using System.Runtime.InteropServices; using System.Threading.Tasks; @@ -32,6 +33,12 @@ static ClassDerivedObject() moduleBuilders = new Dictionary, ModuleBuilder>(); } + public static void Reset() + { + assemblyBuilders = new Dictionary(); + moduleBuilders = new Dictionary, ModuleBuilder>(); + } + internal ClassDerivedObject(Type tp) : base(tp) { } diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index 6a9d40ebd..0b084a49d 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -34,6 +34,11 @@ static ClassManager() dtype = typeof(MulticastDelegate); } + public static void Reset() + { + cache = new Dictionary(128); + } + /// /// Return the ClassBase-derived instance that implements a particular /// reflected managed type, creating it if it doesn't yet exist. diff --git a/src/runtime/debughelper.cs b/src/runtime/debughelper.cs index 2a91a74b4..3fe9ee5bb 100644 --- a/src/runtime/debughelper.cs +++ b/src/runtime/debughelper.cs @@ -116,10 +116,10 @@ internal static void debug(string msg) Console.WriteLine(" {0}", msg); } - /// + /// /// Helper function to inspect/compare managed to native conversions. - /// Especially useful when debugging CustomMarshaler. - /// + /// Especially useful when debugging CustomMarshaler. + /// /// [Conditional("DEBUG")] public static void PrintHexBytes(byte[] bytes) diff --git a/src/runtime/delegatemanager.cs b/src/runtime/delegatemanager.cs index 7632816d1..54256ac17 100644 --- a/src/runtime/delegatemanager.cs +++ b/src/runtime/delegatemanager.cs @@ -185,9 +185,12 @@ public class Dispatcher { public IntPtr target; public Type dtype; + private readonly PyReferenceDecrementer _referenceDecrementer; public Dispatcher(IntPtr target, Type dtype) { + _referenceDecrementer = PythonEngine.CurrentRefDecrementer; + Runtime.XIncref(target); this.target = target; this.dtype = dtype; @@ -195,18 +198,7 @@ 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; - - // 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) - { - IntPtr gs = PythonEngine.AcquireLock(); - Runtime.XDecref(target); - PythonEngine.ReleaseLock(gs); - } + _referenceDecrementer?.ScheduleDecRef(target); } public object Dispatch(ArrayList args) diff --git a/src/runtime/genericutil.cs b/src/runtime/genericutil.cs index 9772d082f..3a230e12c 100644 --- a/src/runtime/genericutil.cs +++ b/src/runtime/genericutil.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Resources; namespace Python.Runtime { @@ -20,6 +21,11 @@ static GenericUtil() mapping = new Dictionary>>(); } + public static void Reset() + { + mapping = new Dictionary>>(); + } + /// /// Register a generic type that appears in a given namespace. /// diff --git a/src/runtime/interop.cs b/src/runtime/interop.cs index 4ae4b61e0..872d5fc2c 100644 --- a/src/runtime/interop.cs +++ b/src/runtime/interop.cs @@ -352,7 +352,6 @@ static Interop() } keepAlive = new ArrayList(); - Marshal.AllocHGlobal(IntPtr.Size); pmap = new Hashtable(); pmap["tp_dealloc"] = p["DestructorFunc"]; diff --git a/src/runtime/methodbinding.cs b/src/runtime/methodbinding.cs index 07090a92c..b58c092de 100644 --- a/src/runtime/methodbinding.cs +++ b/src/runtime/methodbinding.cs @@ -21,11 +21,12 @@ public MethodBinding(MethodObject m, IntPtr target, IntPtr targetType) Runtime.XIncref(target); this.target = target; - Runtime.XIncref(targetType); if (targetType == IntPtr.Zero) { targetType = Runtime.PyObject_Type(target); } + + Runtime.XIncref(targetType); this.targetType = targetType; this.info = null; diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index e683026f9..0ec9900dd 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -328,6 +328,17 @@ public CLRModule() : base("clr") } } + public static void Reset() + { + hacked = false; + interactive_preload = true; + preload = false; + + // XXX Test performance of new features // + _SuppressDocs = false; + _SuppressOverloads = false; + } + /// /// The initializing of the preload hook has to happen as late as /// possible since sys.ps1 is created after the CLR module is diff --git a/src/runtime/pyiter.cs b/src/runtime/pyiter.cs index ee07bcecf..5fa6ec70c 100644 --- a/src/runtime/pyiter.cs +++ b/src/runtime/pyiter.cs @@ -40,14 +40,20 @@ public PyIter(PyObject iterable) } } - protected override void Dispose(bool disposing) + protected internal override void Dispose(bool disposing) { - if (null != _current) + try { - _current.Dispose(); - _current = null; + if (null != _current) + { + _current.Dispose(disposing); + _current = null; + } + } + finally + { + base.Dispose(disposing); } - base.Dispose(disposing); } public bool MoveNext() diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index 0d186bf4e..843325c5b 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -16,6 +16,7 @@ public class PyObject : DynamicObject, IEnumerable, IDisposable { protected internal IntPtr obj = IntPtr.Zero; private bool disposed = false; + private readonly PyReferenceDecrementer _referenceDecrementer; /// /// PyObject Constructor @@ -26,7 +27,7 @@ public class PyObject : DynamicObject, IEnumerable, IDisposable /// and the reference will be DECREFed when the PyObject is garbage /// collected or explicitly disposed. /// - public PyObject(IntPtr ptr) + public PyObject(IntPtr ptr): this() { obj = ptr; } @@ -36,6 +37,7 @@ public PyObject(IntPtr ptr) protected PyObject() { + _referenceDecrementer = PythonEngine.CurrentRefDecrementer; } // Ensure that encapsulated Python object is decref'ed appropriately @@ -43,11 +45,7 @@ protected PyObject() ~PyObject() { - // We needs to disable Finalizers until it's valid implementation. - // Current implementation can produce low probability floating bugs. - return; - - Dispose(); + Dispose(false); } @@ -134,25 +132,40 @@ public T As() /// to Python objects will not be released until a managed garbage /// collection occurs. /// - protected virtual void Dispose(bool disposing) + protected internal virtual void Dispose(bool disposing) { if (!disposed) { - if (Runtime.Py_IsInitialized() > 0 && !Runtime.IsFinalizing) + disposed = true; + IntPtr objToDispose = obj; + obj = IntPtr.Zero; + + if (disposing) { - IntPtr gs = PythonEngine.AcquireLock(); - Runtime.XDecref(obj); - obj = IntPtr.Zero; - PythonEngine.ReleaseLock(gs); + if (Runtime.Py_IsInitialized() > 0 && !Runtime.IsFinalizing) + { + IntPtr gs = PythonEngine.AcquireLock(); + try + { + Runtime.XDecref(objToDispose); + } + finally + { + PythonEngine.ReleaseLock(gs); + } + } + } + else + { + _referenceDecrementer?.ScheduleDecRef(objToDispose); } - disposed = true; } } public void Dispose() { - Dispose(true); GC.SuppressFinalize(this); + Dispose(true); } diff --git a/src/runtime/pyreferencedecrementer.cs b/src/runtime/pyreferencedecrementer.cs new file mode 100644 index 000000000..674ebb5f9 --- /dev/null +++ b/src/runtime/pyreferencedecrementer.cs @@ -0,0 +1,179 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; + +namespace Python.Runtime +{ + /// + /// Background python objects reference decrementer. + /// !!! By some unknown reasons python references decrements have been performed in the GC thread - affects GC. !!! + /// This component contains background thread with the queue of IntPtr python object references. + /// Finalizers should schedule reference decrement operation in this component to avoid problem with GC. + /// Each PythonEngine Init/Shutdown have it's own PyReferenceDecrementer, so PyObject should also have a field with the + /// reference to the correct PyReferenceDecrementer. + /// + internal class PyReferenceDecrementer : IDisposable + { + private static readonly DedicatedThreadTaskScheduler DedicatedThreadTaskScheduler = new DedicatedThreadTaskScheduler(); + + private readonly BlockingCollection _asyncDecRefQueue = new BlockingCollection(); + + private CancellationTokenSource _cts; + private CancellationToken _ct; + private Task _backgroundWorkerTask; + + public PyReferenceDecrementer() + { + InitDecRefThread(); + } + + public void ScheduleDecRef(IntPtr pyRef) + { + // ReSharper disable once MethodSupportsCancellation + _asyncDecRefQueue.Add(pyRef); + } + + internal void WaitForPendingDecReferences() + { + ShutdownDecRefThread(); + InitDecRefThread(); + } + + private void ShutdownDecRefThread() + { + _cts?.Cancel(); + try + { + IntPtr ts = IntPtr.Zero; + if (Runtime.PyGILState_GetThisThreadState() != IntPtr.Zero) + { + ts = Runtime.PyEval_SaveThread(); + } + try + { + // ReSharper disable once MethodSupportsCancellation + _backgroundWorkerTask.Wait(); + } + catch (AggregateException ex) + { + if (!(ex.InnerException is OperationCanceledException)) + { + throw; + } + } + finally + { + if (ts != IntPtr.Zero) + { + Runtime.PyEval_RestoreThread(ts); + } + } + } + catch + { + // Just stopping background thread. + } + + _cts = null; + _ct = default(CancellationToken); + + _backgroundWorkerTask = null; + } + + private void InitDecRefThread() + { + _cts = new CancellationTokenSource(); + _ct = _cts.Token; + + _backgroundWorkerTask = Task.Factory.StartNew(WorkerThread, _ct, TaskCreationOptions.LongRunning, + DedicatedThreadTaskScheduler); + } + + private void WorkerThread() + { + while (true) + { + IntPtr refToDecrease = _asyncDecRefQueue.Take(_ct); ; + + try + { + + IntPtr gs = PythonEngine.AcquireLock(); + try + { + do + { + Runtime.XDecref(refToDecrease); + } while (_asyncDecRefQueue.TryTake(out refToDecrease)); + } + finally + { + PythonEngine.ReleaseLock(gs); + } + } + catch + { + // Nothing to do in this case. + } + } + } + + public void Dispose() + { + ShutdownDecRefThread(); + } + } + + + /// + /// Scheduler that uses only one thread for all scheduled task. + /// + internal class DedicatedThreadTaskScheduler : TaskScheduler + { + private readonly BlockingCollection _queuedTasks = new BlockingCollection(); + + + /// + /// Initializes a new instance of the class. + /// + public DedicatedThreadTaskScheduler() + { + var thread = new Thread(WorkerThreadProc); + thread.IsBackground = true; + thread.Start(); + } + + /// + protected override IEnumerable GetScheduledTasks() + { + return _queuedTasks; + } + + /// + protected override void QueueTask(Task task) + { + _queuedTasks.Add(task); + } + + /// + protected override bool TryExecuteTaskInline(Task task, bool taskWasPreviouslyQueued) + { + return false; + } + + private void WorkerThreadProc() + { + for (;;) + { + Task dequeuedTask = _queuedTasks.Take(); + + // This is synchronous execution. + bool taskExecuted = TryExecuteTask(dequeuedTask); + Debug.Assert(taskExecuted, "DedicatedThread task have some problem."); + } + } + } +} diff --git a/src/runtime/pyscope.cs b/src/runtime/pyscope.cs index 67f93c6e2..88161e677 100644 --- a/src/runtime/pyscope.cs +++ b/src/runtime/pyscope.cs @@ -24,12 +24,14 @@ public class PyGILAttribute : Attribute [PyGIL] public class PyScope : DynamicObject, IDisposable { + private readonly PyReferenceDecrementer _referenceDecrementer; + public readonly string Name; /// /// the python Module object the scope associated with. /// - internal readonly IntPtr obj; + internal IntPtr obj; /// /// the variable dict of the scope. @@ -57,6 +59,8 @@ public class PyScope : DynamicObject, IDisposable /// internal PyScope(IntPtr ptr, PyScopeManager manager) { + _referenceDecrementer = PythonEngine.CurrentRefDecrementer; + if (!Runtime.PyType_IsSubtype(Runtime.PyObject_TYPE(ptr), Runtime.PyModuleType)) { throw new PyScopeException("object is not a module"); @@ -514,33 +518,68 @@ private void Check() } } - public void Dispose() + protected virtual void Dispose(bool disposing) { - if (_isDisposed) + if (!_isDisposed) { - return; + IntPtr objToDispose = obj; + if (disposing) + { + try + { + OnDispose?.Invoke(this); + } + finally + { + _isDisposed = true; + obj = IntPtr.Zero; + } + + if (Runtime.Py_IsInitialized() > 0 && !Runtime.IsFinalizing) + { + IntPtr gs = PythonEngine.AcquireLock(); + try + { + Runtime.XDecref(objToDispose); + } + finally + { + PythonEngine.ReleaseLock(gs); + } + } + } + else + { + _isDisposed = true; + obj = IntPtr.Zero; + _referenceDecrementer?.ScheduleDecRef(objToDispose); + } } - _isDisposed = true; - Runtime.XDecref(obj); - this.OnDispose?.Invoke(this); } - ~PyScope() + public void Dispose() { - // We needs to disable Finalizers until it's valid implementation. - // Current implementation can produce low probability floating bugs. - return; + GC.SuppressFinalize(this); + Dispose(true); + } - Dispose(); + ~PyScope() + { + Dispose(false); } } public class PyScopeManager { - public readonly static PyScopeManager Global = new PyScopeManager(); + public static PyScopeManager Global; private Dictionary NamedScopes = new Dictionary(); + internal static void Reset() + { + Global = new PyScopeManager(); + } + internal PyScope NewScope(string name) { if (name == null) diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs index a23c7ac79..e0593140c 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -1,9 +1,10 @@ -using System; +using System; using System.Collections.Generic; using System.IO; using System.Linq; using System.Reflection; using System.Runtime.InteropServices; +using System.Threading; namespace Python.Runtime { @@ -17,6 +18,12 @@ public class PythonEngine : IDisposable private static IntPtr _pythonHome = IntPtr.Zero; private static IntPtr _programName = IntPtr.Zero; private static IntPtr _pythonPath = IntPtr.Zero; + private static bool _isGcDebugEnabled; + + static PythonEngine() + { + _isGcDebugEnabled = !String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("PYTHONNET_GC_DEBUG")); + } public PythonEngine() { @@ -130,6 +137,8 @@ public static string Compiler get { return Marshal.PtrToStringAnsi(Runtime.Py_GetCompiler()); } } + internal static PyReferenceDecrementer CurrentRefDecrementer { get; private set; } + public static int RunSimpleString(string code) { return Runtime.PyRun_SimpleString(code); @@ -215,6 +224,8 @@ public static void Initialize(IEnumerable args, bool setSysArgv = true) } key.Dispose(); } + + CurrentRefDecrementer = new PyReferenceDecrementer(); } finally { @@ -294,13 +305,36 @@ public static void Shutdown() { if (initialized) { + + if (_isGcDebugEnabled) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + Thread.Sleep(100); + } + + CurrentRefDecrementer?.Dispose(); + CurrentRefDecrementer = null; + + if (_isGcDebugEnabled) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + Thread.Sleep(100); + } + + PyScopeManager.Global.Clear(); - Marshal.FreeHGlobal(_pythonHome); - _pythonHome = IntPtr.Zero; - Marshal.FreeHGlobal(_programName); - _programName = IntPtr.Zero; - Marshal.FreeHGlobal(_pythonPath); - _pythonPath = IntPtr.Zero; + // We should not release memory for variables that can be used without initialized python engine. + // It's assumed that Py_GetPythonHome returns valid string without engine initialize. Py_GetPythonHome will always return the + // same pointer that was passed before to Py_SetPythonHome and stored in the _pythonHome. + + ////Marshal.FreeHGlobal(_pythonHome); + ////_pythonHome = IntPtr.Zero; + ////Marshal.FreeHGlobal(_programName); + ////_programName = IntPtr.Zero; + ////Marshal.FreeHGlobal(_pythonPath); + ////_pythonPath = IntPtr.Zero; Runtime.Shutdown(); initialized = false; @@ -562,15 +596,24 @@ internal GILState() state = PythonEngine.AcquireLock(); } + protected virtual void Dispose(bool disposing) + { + //ReleaseLock is thread bound and if it's called in finalizer thread it can wrongly release lock. + if (disposing) + { + PythonEngine.ReleaseLock(state); + } + } + public void Dispose() { - PythonEngine.ReleaseLock(state); GC.SuppressFinalize(this); + Dispose(true); } ~GILState() { - Dispose(); + Dispose(false); } } diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index ded7fbeb5..dcba80bd6 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -8,6 +8,7 @@ namespace Python.Runtime /// public class PythonException : System.Exception { + private readonly PyReferenceDecrementer _referenceDecrementer; private IntPtr _pyType = IntPtr.Zero; private IntPtr _pyValue = IntPtr.Zero; private IntPtr _pyTB = IntPtr.Zero; @@ -18,11 +19,14 @@ public class PythonException : System.Exception public PythonException() { + _referenceDecrementer = PythonEngine.CurrentRefDecrementer; IntPtr gs = PythonEngine.AcquireLock(); Runtime.PyErr_Fetch(ref _pyType, ref _pyValue, ref _pyTB); - Runtime.XIncref(_pyType); - Runtime.XIncref(_pyValue); - Runtime.XIncref(_pyTB); + + // Those references already owned by the caller + ////Runtime.XIncref(_pyType); + ////Runtime.XIncref(_pyValue); + ////Runtime.XIncref(_pyTB); if (_pyType != IntPtr.Zero && _pyValue != IntPtr.Zero) { string type; @@ -45,7 +49,13 @@ public PythonException() } if (_pyTB != IntPtr.Zero) { - PyObject tb_module = PythonEngine.ImportModule("traceback"); + + if (_tbModule == null) + { + _tbModule = PythonEngine.ImportModule("traceback"); + } + PyObject tb_module = _tbModule; + Runtime.XIncref(_pyTB); using (var pyTB = new PyObject(_pyTB)) { @@ -55,16 +65,15 @@ public PythonException() PythonEngine.ReleaseLock(gs); } + private static PyObject _tbModule; + // Ensure that encapsulated Python objects are decref'ed appropriately // when the managed exception wrapper is garbage-collected. ~PythonException() { - // We needs to disable Finalizers until it's valid implementation. - // Current implementation can produce low probability floating bugs. - return; - Dispose(); + Dispose(false); } /// @@ -153,23 +162,64 @@ public string PythonTypeName /// See GH#397 and GH#400. /// public void Dispose() + { + GC.SuppressFinalize(this); + Dispose(true); + } + + protected void Dispose(bool disposing) { if (!disposed) { - if (Runtime.Py_IsInitialized() > 0 && !Runtime.IsFinalizing) + disposed = true; + + IntPtr pyTypeToDispose = _pyType; + _pyType = IntPtr.Zero; + + IntPtr pyValueToDispose = _pyValue; + _pyValue = IntPtr.Zero; + + IntPtr pyTBToDispose = _pyTB; + _pyTB = IntPtr.Zero; + + if (disposing) { - IntPtr gs = PythonEngine.AcquireLock(); - Runtime.XDecref(_pyType); - Runtime.XDecref(_pyValue); - // XXX Do we ever get TraceBack? // - if (_pyTB != IntPtr.Zero) + if (Runtime.Py_IsInitialized() > 0 && !Runtime.IsFinalizing) { - Runtime.XDecref(_pyTB); + IntPtr gs = PythonEngine.AcquireLock(); + try + { + Runtime.XDecref(pyTypeToDispose); + Runtime.XDecref(pyValueToDispose); + // XXX Do we ever get TraceBack? // + if (pyTBToDispose != IntPtr.Zero) + { + Runtime.XDecref(pyTBToDispose); + } + } + finally + { + PythonEngine.ReleaseLock(gs); + } + } + } + else + { + if (pyTypeToDispose != IntPtr.Zero) + { + _referenceDecrementer?.ScheduleDecRef(pyTypeToDispose); + } + + if (pyValueToDispose != IntPtr.Zero) + { + _referenceDecrementer?.ScheduleDecRef(pyValueToDispose); + } + + if (pyTBToDispose != IntPtr.Zero) + { + _referenceDecrementer?.ScheduleDecRef(pyTBToDispose); } - PythonEngine.ReleaseLock(gs); } - GC.SuppressFinalize(this); - disposed = true; } } diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index abd0661a4..164b1bfaa 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -221,6 +221,15 @@ internal static void Initialize() PyEval_InitThreads(); } + IsFinalizing = false; + + CLRModule.Reset(); + GenericUtil.Reset(); + PyScopeManager.Reset(); + ClassManager.Reset(); + ClassDerivedObject.Reset(); + TypeManager.Reset(); + IntPtr op; IntPtr dict; if (IsPython3) diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index 6570ee083..ececcc79c 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections; using System.Collections.Generic; using System.Reflection; @@ -21,6 +21,10 @@ static TypeManager() cache = new Dictionary(128); } + public static void Reset() + { + cache = new Dictionary(128); + } /// /// Given a managed Type derived from ExtensionType, get the handle to