In the Python.EmbeddingTests project (I am using master branch):
Add reference to MSTest (Microsoft.VisualStudio.QualityTools.UnitTestFramework.dll 10.0.0.0)
Convert pyimport from NUnit to MSTest:
remove NUnit Import, add using Microsoft.VisualStudio.TestTools.UnitTesting
replace attributes with MStest attributes (TestClass, TestInitialise, TestCleanup, TestMethod)
Running TestDottedName works as expected.
Edit one.py and add import numpy as np as first line
Run TestDottedName should work first time
Run TestDottedName again, now it fails with:
Test method Python.EmbeddingTest.PyImportTest.TestDottedName threw exception:
System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
This error is also being shown in my ASP.Net Web API application on restart so I hope a solution will solve both MSTest and ASP.Net.
Further Information: Full version of pyimport.cs after edits is:
using System;
using System.Reflection;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Python.Runtime;
namespace Python.EmbeddingTest
{
[TestClass]
public class PyImportTest
{
private IntPtr gs;
[TestInitialize]
public void SetUp()
{
string path = @"c:\dev\libs\Anaconda3\envs\python2;" + Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.Machine);
Environment.SetEnvironmentVariable("PATH", path, EnvironmentVariableTarget.Process);
Environment.SetEnvironmentVariable("PYTHONHOME", @"c:\dev\libs\Anaconda3\envs\python2", EnvironmentVariableTarget.Process);
Environment.SetEnvironmentVariable("PYTHONPATH ", @"c:\dev\libs\Anaconda3\envs\python2\Lib", EnvironmentVariableTarget.Process);
PythonEngine.Initialize();
gs = PythonEngine.AcquireLock();
//string here = Environment.CurrentDirectory;
//trunk\pythonnet\src\embed_tests\bin\x86\DebugWin
/*
* Append the tests directory to sys.path
* using reflection to circumvent the private modifires placed on most Runtime methods.
*/
const string s = @"../../../../tests";
Type RTClass = typeof(Runtime.Runtime);
/* pyStrPtr = PyString_FromString(s); */
MethodInfo PyString_FromString = RTClass.GetMethod("PyString_FromString",
BindingFlags.NonPublic | BindingFlags.Static);
object[] funcArgs = new object[1];
funcArgs[0] = s;
IntPtr pyStrPtr = (IntPtr)PyString_FromString.Invoke(null, funcArgs);
/* SysDotPath = sys.path */
MethodInfo PySys_GetObject = RTClass.GetMethod("PySys_GetObject",
BindingFlags.NonPublic | BindingFlags.Static);
funcArgs[0] = "path";
IntPtr SysDotPath = (IntPtr)PySys_GetObject.Invoke(null, funcArgs);
/* SysDotPath.append(*pyStrPtr) */
MethodInfo PyList_Append = RTClass.GetMethod("PyList_Append", BindingFlags.NonPublic | BindingFlags.Static);
funcArgs = new object[] { SysDotPath, pyStrPtr };
int r = (int)PyList_Append.Invoke(null, funcArgs);
}
[TestCleanup]
public void TearDown()
{
PythonEngine.ReleaseLock(gs);
PythonEngine.Shutdown();
}
[TestMethod]
public void TestDottedName()
{
PyObject module = PythonEngine.ImportModule("PyImportTest.test.one");
Assert.IsNotNull(module, ">>> import PyImportTest.test.one # FAILED");
}
}
}
This occasionally also shows up when using nunit. If all tests run at once, the first test will pass, and every test after fails, regardless of which tests it is (ie, not just pyimport.cs).
If I run each test separately they run with no problems.
The issue isn't present on v2.0.0 e5c4e74, but it is on the commit right after 138bda7. That commit introduced python3 code and changed 5.5k lines though.
SetUp : System.AccessViolationException : Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
TearDown : System.AccessViolationException : Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
at Python.Runtime.Runtime.PyModule_GetDict(IntPtr module)
at Python.Runtime.PythonEngine.Initialize() in .\pythonnet\src\runtime\pythonengine.cs:line 147
at Python.EmbeddingTest.PyIterTest.SetUp() in .\pythonnet\src\embed_tests\pyiter.cs:line 16
--TearDown
at Python.Runtime.Runtime.PyObject_HasAttrString(IntPtr pointer, String name)
at Python.Runtime.Exceptions.Shutdown() in .\pythonnet\src\runtime\exceptions.cs:line 134
at Python.Runtime.Runtime.Shutdown() in .\pythonnet\src\runtime\runtime.cs:line 371
at Python.Runtime.PythonEngine.Shutdown() in .\pythonnet\src\runtime\pythonengine.cs:line 261
I'm actually pretty certain that this always happens if we do PythonEngine.Initialize() after a previous PythonEngine.Shutdown(). I've experienced the same issue while running unit tests with NUnit, gaven't come far debugging this, though.
@filmor absolutely right. Just wrote a test on it and got it to fail by just doing
PythonEngine.Initialize()
PythonEngine.Shutdown()
PythonEngine.Initialize() << fails here
PythonEngine.Shutdown()
Almost got it, its a messy one... this section creates an exception that gets taken care of by moduleobject the first time around when interactive_preload is true. The second time around the interactive_preload flag isn't reset, so the exception goes unhandled and is what causes the bug.
dirty fix would be to clear exception and move on... looking for a better fix for now though
Still don't fully understand the issue, however, I think the whole preload logic is superfluous. We can get the same behaviour by simply loading all names as soon as __dir__ is called.
until we find a way for clean initialize and shutdown, can we get rid of setup and teardown of nunit tests in the meantime? so just have one initialize and one shutdown for all embedding tests.
@denfromufa That may work since I'm adding a test explicitly for this. I want to understand this issue a bit more before doing that change. @filmor I don't think this issue should block 2.2.0. The issue is going to take a while to properly figure out.
I think I've got it, working on minimising the change now: At least one of the problems is the call ModuleDefOffset.FreeModuleDef(module_def); in ImportHook.Shutdown, as the module def is still used in the actual finalisation (Py_Finalize()) as part of garbage collecting the modules, so we get a use-after-free situation here. However, just fixing this doesn't seem to be enough, I did a bunch of other changes before so I'm now working on boiling this down to a small set of commits.
@gatapia can you verify your issue has been resolved? Between @filmor and my recent changes, this should have been solved.
We had a similar behavior in NUNIT and we know its solved there, but haven't gone back to test against mstests.
Activity
den-run-ai commented on Sep 9, 2016
@gatapia good finding, let me link this here:
#224
@BartonCline originally implemented this test, not sure if he recalls any issues like that?
vmuriart commented on Jan 9, 2017
This occasionally also shows up when using nunit. If all tests run at once, the first test will pass, and every test after fails, regardless of which tests it is (ie, not just pyimport.cs).
If I run each test separately they run with no problems.
The issue isn't present on v2.0.0 e5c4e74, but it is on the commit right after 138bda7. That commit introduced python3 code and changed 5.5k lines though.
filmor commented on Jan 10, 2017
I'm actually pretty certain that this always happens if we do PythonEngine.Initialize() after a previous PythonEngine.Shutdown(). I've experienced the same issue while running unit tests with NUnit, gaven't come far debugging this, though.
vmuriart commented on Jan 10, 2017
@filmor absolutely right. Just wrote a test on it and got it to fail by just doing
PythonEngine.Initialize()
PythonEngine.Shutdown()
PythonEngine.Initialize() << fails here
PythonEngine.Shutdown()
and verified that it did work on v2.0.0
Good reads related:
Second time this line runs it returns an invalid pointer.
vmuriart commented on Jan 13, 2017
Almost got it, its a messy one... this section creates an exception that gets taken care of by moduleobject the first time around when interactive_preload is true. The second time around the interactive_preload flag isn't reset, so the exception goes unhandled and is what causes the bug.
dirty fix would be to clear exception and move on... looking for a better fix for now though
filmor commented on Jan 16, 2017
Still don't fully understand the issue, however, I think the whole preload logic is superfluous. We can get the same behaviour by simply loading all names as soon as __dir__ is called.
den-run-ai commented on Jan 22, 2017
until we find a way for clean initialize and shutdown, can we get rid of setup and teardown of nunit tests in the meantime? so just have one initialize and one shutdown for all embedding tests.
vmuriart commented on Jan 22, 2017
@denfromufa That may work since I'm adding a test explicitly for this. I want to understand this issue a bit more before doing that change.
@filmor I don't think this issue should block 2.2.0. The issue is going to take a while to properly figure out.
filmor commented on Jan 26, 2017
I think I've got it, working on minimising the change now: At least one of the problems is the call ModuleDefOffset.FreeModuleDef(module_def); in ImportHook.Shutdown, as the module def is still used in the actual finalisation (Py_Finalize()) as part of garbage collecting the modules, so we get a use-after-free situation here. However, just fixing this doesn't seem to be enough, I did a bunch of other changes before so I'm now working on boiling this down to a small set of commits.
vmuriart commented on Jan 26, 2017
@filmor awesome! I already have a test for it in #329
Fix the shutdown issue by keeping PyMethodDef.
Fix the shutdown issue by keeping PyMethodDef.
Fix the shutdown issue by keeping PyMethodDef.
Fix the shutdown issue by keeping PyMethodDef.
Fix the shutdown issue by keeping PyMethodDef.
Fix the shutdown issue by keeping PyMethodDef.
Fix the shutdown issue by keeping PyMethodDef.
Fix the shutdown issue by keeping PyMethodDef.
Fix the shutdown issue by keeping PyMethodDef.
Fix the shutdown issue by keeping PyMethodDef.
Fix the shutdown issue by keeping PyMethodDef.
vmuriart commented on Feb 1, 2017
Fixed on PY27, still buggy on py3+
vmuriart commented on Feb 8, 2017
@gatapia can you verify your issue has been resolved? Between @filmor and my recent changes, this should have been solved.
We had a similar behavior in NUNIT and we know its solved there, but haven't gone back to test against mstests.
Thanks!