FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

MSTests failing when importing a module with imports · Issue #262 · pythonnet/pythonnet · GitHub

Repository navigation

MSTests failing when importing a module with imports #262

Closed

Description

Very simple to reproduce:

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");
        }
    }
}

Activity

  1. den-run-ai commented on Sep 9, 2016

    Contributor

    @gatapia good finding, let me link this here:

    #224

    @BartonCline originally implemented this test, not sure if he recalls any issues like that?

  2. vmuriart commented on Jan 9, 2017

    Contributor

    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
    
  3. filmor commented on Jan 10, 2017

    Member

    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.

  4. vmuriart commented on Jan 10, 2017

    Contributor
  5. vmuriart commented on Jan 13, 2017

    Contributor

    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

  6. filmor commented on Jan 16, 2017

    Member

    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.

  7. den-run-ai commented on Jan 22, 2017

    Contributor

    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.

  8. vmuriart commented on Jan 22, 2017

    Contributor

    @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.

  9. filmor commented on Jan 26, 2017

    Member

    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.

  10. vmuriart commented on Jan 26, 2017

    Contributor

    @filmor awesome! I already have a test for it in #329

  11. vmuriart commented on Feb 1, 2017

    Contributor

    Fixed on PY27, still buggy on py3+

  12. added this to the 2.3.0 milestone on Feb 2, 2017
  13. vmuriart commented on Feb 8, 2017

    Contributor

    @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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions


      Back | FazBrowse Home | New Git URL