From b0c68e6a662ebb4dd866c00a8684dedbc479b53c Mon Sep 17 00:00:00 2001 From: dse Date: Thu, 31 Aug 2017 19:34:31 +0400 Subject: [PATCH 1/8] Runtime/Shutdown loop stores old caches and static variables. It's produces bugs when CPython freeing up enough objects. --- CHANGELOG.md | 2 ++ src/runtime/assemblymanager.cs | 11 ++++------- src/runtime/classderived.cs | 9 ++++++++- src/runtime/classmanager.cs | 5 +++++ src/runtime/genericutil.cs | 6 ++++++ src/runtime/moduleobject.cs | 11 +++++++++++ src/runtime/pyscope.cs | 7 ++++++- src/runtime/runtime.cs | 7 +++++++ src/runtime/typemanager.cs | 4 ++++ 9 files changed, 53 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e131c327b..0c0957e2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ### Fixed +- 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][i434]) for setup.py - Fixed crashes when integrating pythonnet in Unity3d ([#714][i714]), related to unloading the Application Domain diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs index 2df7ad2f5..05761dd1e 100644 --- a/src/runtime/assemblymanager.cs +++ b/src/runtime/assemblymanager.cs @@ -24,14 +24,14 @@ internal class AssemblyManager // than it can end up referring to assemblies that are already unloaded (default behavior after unload appDomain - // unless LoaderOptimization.MultiDomain is used); // So for multidomain support it is better to have the dict. recreated for each app-domain initialization - 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 ConcurrentQueue assemblies; @@ -48,10 +48,7 @@ private AssemblyManager() /// internal static void Initialize() { - namespaces = new ConcurrentDictionary>(); - probed = new Dictionary(32); - //generics = new Dictionary>(); - assemblies = new ConcurrentQueue(); + assemblies = new AssemblyList(16); pypath = new List(16); AppDomain domain = AppDomain.CurrentDomain; 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/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/moduleobject.cs b/src/runtime/moduleobject.cs index ea1e8e8d0..8af722d29 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -335,6 +335,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/pyscope.cs b/src/runtime/pyscope.cs index 67f93c6e2..8e6957855 100644 --- a/src/runtime/pyscope.cs +++ b/src/runtime/pyscope.cs @@ -537,10 +537,15 @@ public void Dispose() 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/runtime.cs b/src/runtime/runtime.cs index 831864ef9..e66b28b72 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -281,6 +281,13 @@ internal static void Initialize() PyEval_InitThreads(); } + 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 df2e71be0..d19c8737f 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -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 From 1dbf38399af0d53dbedaa7275ebdca848a067b39 Mon Sep 17 00:00:00 2001 From: dse Date: Sat, 2 Sep 2017 22:51:58 +0400 Subject: [PATCH 2/8] Added all tests finalizer routine. --- src/embed_tests/Python.EmbeddingTest.csproj | 1 + src/embed_tests/TestsSuite.cs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 src/embed_tests/TestsSuite.cs diff --git a/src/embed_tests/Python.EmbeddingTest.csproj b/src/embed_tests/Python.EmbeddingTest.csproj index e50053f07..fcd15d9b2 100644 --- a/src/embed_tests/Python.EmbeddingTest.csproj +++ b/src/embed_tests/Python.EmbeddingTest.csproj @@ -105,6 +105,7 @@ + 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(); + } + } + } +} From ccd60034d6b7c065b49d9a4ad68d9eda96e97150 Mon Sep 17 00:00:00 2001 From: dse Date: Sat, 2 Sep 2017 23:01:47 +0400 Subject: [PATCH 3/8] Bug: Py_Initialize/Py_Finalize calls during alive PythonEngine. Fixed. --- src/embed_tests/TestRuntime.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/embed_tests/TestRuntime.cs b/src/embed_tests/TestRuntime.cs index f26a1e4b4..ac1fa1ac0 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,6 +6,16 @@ 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 the cache of the information from the platform module. /// @@ -24,12 +34,12 @@ public static void PlatformCache() // Don't shut down the runtime: if the python engine was initialized // but not shut down by another test, we'd end up in a bad state. - } + } [Test] public static void Py_IsInitializedValue() { - Runtime.Runtime.Py_Finalize(); // In case another test left it on. + Runtime.Runtime.Py_Finalize(); Assert.AreEqual(0, Runtime.Runtime.Py_IsInitialized()); Runtime.Runtime.Py_Initialize(); Assert.AreEqual(1, Runtime.Runtime.Py_IsInitialized()); From 6a6414dc20864478c872f957e63862abb302a50d Mon Sep 17 00:00:00 2001 From: dse Date: Sat, 2 Sep 2017 22:48:05 +0400 Subject: [PATCH 4/8] Init/Shutdown state variable (IsFinalizing) fix. --- src/runtime/runtime.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index e66b28b72..ef492a7b8 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -281,6 +281,8 @@ internal static void Initialize() PyEval_InitThreads(); } + IsFinalizing = false; + CLRModule.Reset(); GenericUtil.Reset(); PyScopeManager.Reset(); From d3315d490682374901be435065511ac6550313c6 Mon Sep 17 00:00:00 2001 From: dse Date: Wed, 6 Sep 2017 00:45:44 +0400 Subject: [PATCH 5/8] TestPythonEngineProperties fixes. --- src/embed_tests/TestPythonEngineProperties.cs | 40 +++++++++++++++++-- src/runtime/pythonengine.cs | 9 +---- 2 files changed, 39 insertions(+), 10 deletions(-) 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/runtime/pythonengine.cs b/src/runtime/pythonengine.cs index 38bd33099..4c51224f2 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -309,19 +309,14 @@ public static void Shutdown() { if (initialized) { + PyScopeManager.Global.Clear(); + // If the shutdown handlers trigger a domain unload, // don't call shutdown again. AppDomain.CurrentDomain.DomainUnload -= OnDomainUnload; ExecuteShutdownHandlers(); - Marshal.FreeHGlobal(_pythonHome); - _pythonHome = IntPtr.Zero; - Marshal.FreeHGlobal(_programName); - _programName = IntPtr.Zero; - Marshal.FreeHGlobal(_pythonPath); - _pythonPath = IntPtr.Zero; - initialized = false; } } From b2b48388c94bf4da0b7599a5cc97ab0a3f1d4b07 Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Tue, 16 Oct 2018 11:36:57 +0200 Subject: [PATCH 6/8] Update CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c0957e2f..ef9358793 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ### Fixed - 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. + This is a hidden bug. Once python cleaning up enough memory, objects from previous engine run becomes corrupted. ([#534][p534]) - Fixed Visual Studio 2017 compat ([#434][i434]) for setup.py - Fixed crashes when integrating pythonnet in Unity3d ([#714][i714]), related to unloading the Application Domain @@ -697,3 +697,4 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. [i131]: https://github.com/pythonnet/pythonnet/issues/131 [p531]: https://github.com/pythonnet/pythonnet/pull/531 [i755]: https://github.com/pythonnet/pythonnet/pull/755 +[p534]: https://github.com/pythonnet/pythonnet/pull/534 \ No newline at end of file From d7f2bc849c311d7a9d17975bc583bbf2f6654bc9 Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Tue, 16 Oct 2018 12:04:03 +0200 Subject: [PATCH 7/8] Rename TestsSuite and add comment, remove unneeded using --- src/embed_tests/{TestsSuite.cs => GlobalTestsSetup.cs} | 5 ++++- src/embed_tests/Python.EmbeddingTest.csproj | 2 +- src/embed_tests/TestPythonEngineProperties.cs | 1 - 3 files changed, 5 insertions(+), 3 deletions(-) rename src/embed_tests/{TestsSuite.cs => GlobalTestsSetup.cs} (68%) diff --git a/src/embed_tests/TestsSuite.cs b/src/embed_tests/GlobalTestsSetup.cs similarity index 68% rename from src/embed_tests/TestsSuite.cs rename to src/embed_tests/GlobalTestsSetup.cs index 44ce2d4b8..458ab6a99 100644 --- a/src/embed_tests/TestsSuite.cs +++ b/src/embed_tests/GlobalTestsSetup.cs @@ -3,8 +3,11 @@ namespace Python.EmbeddingTest { + + // As the SetUpFixture, the OneTimeTearDown of this class is executed after + // all tests have run. [SetUpFixture] - public class TestsSuite + public class GlobalTestsSetup { [OneTimeTearDown] public void FinalCleanup() diff --git a/src/embed_tests/Python.EmbeddingTest.csproj b/src/embed_tests/Python.EmbeddingTest.csproj index fcd15d9b2..1033fbb20 100644 --- a/src/embed_tests/Python.EmbeddingTest.csproj +++ b/src/embed_tests/Python.EmbeddingTest.csproj @@ -105,7 +105,7 @@ - + diff --git a/src/embed_tests/TestPythonEngineProperties.cs b/src/embed_tests/TestPythonEngineProperties.cs index d95942577..243349b82 100644 --- a/src/embed_tests/TestPythonEngineProperties.cs +++ b/src/embed_tests/TestPythonEngineProperties.cs @@ -1,5 +1,4 @@ using System; -using System.Diagnostics; using NUnit.Framework; using Python.Runtime; From dbd5fdd6837b2f7f2d18f449fdd9533da6ca8ffd Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Mon, 29 Oct 2018 13:22:12 +0100 Subject: [PATCH 8/8] Update assemblymanager.cs --- src/runtime/assemblymanager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs index 05761dd1e..0709eedad 100644 --- a/src/runtime/assemblymanager.cs +++ b/src/runtime/assemblymanager.cs @@ -48,7 +48,7 @@ private AssemblyManager() /// internal static void Initialize() { - assemblies = new AssemblyList(16); + assemblies = new ConcurrentQueue(); pypath = new List(16); AppDomain domain = AppDomain.CurrentDomain;