Skip to content

Fixes to integrate pythonnet into Unity #745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
410ac15
UNI-63112: unit test for the domain reload crash
Sep 14, 2018
c07fff0
UNI-62864: shutdown on domain reload.
Sep 14, 2018
d016b24
Drive-by improve a hashtable to hashset.
Sep 26, 2018
9ae91ba
UNI-63112: implement platform-aware native code for tp_traverse et al
Sep 26, 2018
bb76ff3
Domain reload work: port previous commit to Windows
benoithudson Sep 28, 2018
0c5b78f
Fixed a bogus comment.
benoithudson Sep 28, 2018
9ffb705
Make prettier
benoithudson Sep 28, 2018
2e03eb8
Added author and changelog information.
Sep 28, 2018
156f554
Fix for linux mmap requiring MAP_PRIVATE
benoithudson Oct 2, 2018
84f5087
Doc and typo fixes.
benoithudson Oct 2, 2018
b9f6c2c
Merge branch 'uni-63112-hotreload-crash-test' of https://github.com/U…
benoithudson Oct 2, 2018
e585bdc
Merge pull request #7 from Unity-Technologies/uni-63112-hotreload-cra…
benoithudson Oct 2, 2018
9ab7b13
Disable app domain test case on .NET Standard
Oct 3, 2018
13f8b53
Fix for CI: define NETSTANDARD in src/embed_tests
Oct 3, 2018
c0b52fa
Fix compile error on OSX that came from an upstream merge.
Oct 3, 2018
4096a95
Fix in dotnet code path: find mmap in libc rather than __Internal
Oct 3, 2018
9b74cce
Fix TypeManager test for running on .NET Core under linux/OSX
Oct 3, 2018
5e15b2c
Fix for python3 tests crashing: it's about test order
Oct 4, 2018
211155e
WIP - debug: turn off "quiet" so that I get an error message
Oct 11, 2018
14bc2e2
Use msbuild v14 for linux/darwin.
Oct 11, 2018
8a80fd5
Upgrade setuptools on appveyor
Oct 11, 2018
dd77fa5
Flush the console on every WriteLine so messages are in order.
Oct 11, 2018
f947e3b
Grasping at straws: try using conda to set up environment
Oct 11, 2018
1fda82b
Revert "Grasping at straws: try using conda to set up environment"
Oct 11, 2018
ec6f6c5
Give up on python 3.4 for appveyor.
Oct 11, 2018
4bac40e
Travis: don't print from domain-reload test
Oct 11, 2018
386a034
Fixed appveyor "allow_failures" syntax.
Oct 11, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions AUTHORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

- Alexandre Catarino([@AlexCatarino](https://github.com/AlexCatarino))
- Arvid JB ([@ArvidJB](https://github.com/ArvidJB))
- Benoît Hudson ([@benoithudson](https://github.com/benoithudson))
- Bradley Friedman ([@leith-bartrich](https://github.com/leith-bartrich))
- Callum Noble ([@callumnoble](https://github.com/callumnoble))
- Christian Heimes ([@tiran](https://github.com/tiran))
Expand All @@ -22,6 +23,7 @@
- Daniel Fernandez ([@fdanny](https://github.com/fdanny))
- Daniel Santana ([@dgsantana](https://github.com/dgsantana))
- Dave Hirschfeld ([@dhirschfeld](https://github.com/dhirschfeld))
- David Lassonde ([@lassond](https://github.com/lassond))
- David Lechner ([@dlech](https://github.com/dlech))
- Dmitriy Se ([@dmitriyse](https://github.com/dmitriyse))
- He-chien Tsai ([@t3476](https://github.com/t3476))
Expand All @@ -39,6 +41,7 @@
- Sam Winstanley ([@swinstanley](https://github.com/swinstanley))
- Sean Freitag ([@cowboygneox](https://github.com/cowboygneox))
- Serge Weinstock ([@sweinst](https://github.com/sweinst))
- Viktoria Kovescses ([@vkovec](https://github.com/vkovec))
- Ville M. Vainio ([@vivainio](https://github.com/vivainio))
- Virgil Dupras ([@hsoft](https://github.com/hsoft))
- Wenguang Yang ([@yagweb](https://github.com/yagweb))
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
### Fixed

- 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
- Fixed crash on exit of the Python interpreter if a python class
derived from a .NET class has a `__namespace__` or `__assembly__`
attribute ([#481][i481])
Expand Down
12 changes: 10 additions & 2 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ environment:
- PYTHON_VERSION: 3.5
- PYTHON_VERSION: 3.6

matrix:
allow_failures:
- PYTHON_VERSION: 3.4
BUILD_OPTS: --xplat
- PYTHON_VERSION: 3.4

init:
# Update Environment Variables based on matrix/platform
- set PY_VER=%PYTHON_VERSION:.=%
Expand All @@ -38,11 +44,13 @@ init:
- set PATH=%PYTHON%;%PYTHON%\Scripts;%PATH%

install:
# Upgrade setuptools to find MSVC. Otherwise you get "error: Microsoft Visual C++ 10.0 is required (Unable to find vcvarsall.bat)."
- python -m pip install -U pip
- pip install --upgrade -r requirements.txt --quiet
- pip install --upgrade setuptools
- pip install --upgrade -r requirements.txt

# Install OpenCover. Can't put on `packages.config`, not Mono compatible
- .\tools\nuget\nuget.exe install OpenCover -OutputDirectory packages -Verbosity quiet
- .\tools\nuget\nuget.exe install OpenCover -OutputDirectory packages

build_script:
# Create clean `sdist`. Only used for releases
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ def _install_packages(self):
self.debug_print("Updating NuGet: {0}".format(cmd))
subprocess.check_call(cmd, shell=use_shell)

cmd = "{0} restore pythonnet.sln -o packages".format(nuget)
cmd = "{0} restore pythonnet.sln -MSBuildVersion 14 -o packages".format(nuget)
self.debug_print("Installing packages: {0}".format(cmd))
subprocess.check_call(cmd, shell=use_shell)

Expand Down
1 change: 1 addition & 0 deletions src/embed_tests/Python.EmbeddingTest.15.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<BaseDefineConstants>XPLAT</BaseDefineConstants>
<DefineConstants>$(DefineConstants);$(CustomDefineConstants);$(BaseDefineConstants);</DefineConstants>
<DefineConstants Condition="'$(TargetFramework)'=='netcoreapp2.0'">$(DefineConstants);NETCOREAPP</DefineConstants>
<DefineConstants Condition="'$(TargetFramework)'=='netstandard2.0'">$(DefineConstants);NETSTANDARD</DefineConstants>
<DefineConstants Condition="'$(BuildingInsideVisualStudio)' == 'true' AND '$(CustomDefineConstants)' != '' AND $(Configuration.Contains('Debug'))">$(DefineConstants);TRACE;DEBUG</DefineConstants>
<FrameworkPathOverride Condition="'$(TargetFramework)'=='net40' AND $(Configuration.Contains('Mono'))">$(NuGetPackageRoot)\microsoft.targetingpack.netframework.v4.5\1.0.1\lib\net45\</FrameworkPathOverride>
</PropertyGroup>
Expand Down
4 changes: 3 additions & 1 deletion src/embed_tests/Python.EmbeddingTest.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
<Compile Include="pyrunstring.cs" />
<Compile Include="TestConverter.cs" />
<Compile Include="TestCustomMarshal.cs" />
<Compile Include="TestDomainReload.cs" />
<Compile Include="TestExample.cs" />
<Compile Include="TestPyAnsiString.cs" />
<Compile Include="TestPyFloat.cs" />
Expand All @@ -103,6 +104,7 @@
<Compile Include="TestPyWith.cs" />
<Compile Include="TestRuntime.cs" />
<Compile Include="TestPyScope.cs" />
<Compile Include="TestTypeManager.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\runtime\Python.Runtime.csproj">
Expand All @@ -122,4 +124,4 @@
<Copy SourceFiles="$(TargetAssembly)" DestinationFolder="$(PythonBuildDir)" />
<!--Copy SourceFiles="$(TargetAssemblyPdb)" Condition="Exists('$(TargetAssemblyPdb)')" DestinationFolder="$(PythonBuildDir)" /-->
</Target>
</Project>
</Project>
270 changes: 270 additions & 0 deletions src/embed_tests/TestDomainReload.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,270 @@
using System;
using System.CodeDom.Compiler;
using System.Reflection;
using NUnit.Framework;
using Python.Runtime;

//
// This test case is disabled on .NET Standard because it doesn't have all the
// APIs we use. We could work around that, but .NET Core doesn't implement
// domain creation, so it's not worth it.
//
// Unfortunately this means no continuous integration testing for this case.
//
#if !NETSTANDARD && !NETCOREAPP
namespace Python.EmbeddingTest
{
class TestDomainReload
{
/// <summary>
/// Test that the python runtime can survive a C# domain reload without crashing.
///
/// At the time this test was written, there was a very annoying
/// seemingly random crash bug when integrating pythonnet into Unity.
///
/// The repro steps that David Lassonde, Viktoria Kovecses and
/// Benoit Hudson eventually worked out:
/// 1. Write a HelloWorld.cs script that uses Python.Runtime to access
/// some C# data from python: C# calls python, which calls C#.
/// 2. Execute the script (e.g. make it a MenuItem and click it).
/// 3. Touch HelloWorld.cs on disk, forcing Unity to recompile scripts.
/// 4. Wait several seconds for Unity to be done recompiling and
/// reloading the C# domain.
/// 5. Make python run the gc (e.g. by calling gc.collect()).
///
/// The reason:
/// A. In step 2, Python.Runtime registers a bunch of new types with
/// their tp_traverse slot pointing to managed code, and allocates
/// some objects of those types.
/// B. In step 4, Unity unloads the C# domain. That frees the managed
/// code. But at the time of the crash investigation, pythonnet
/// leaked the python side of the objects allocated in step 1.
/// C. In step 5, python sees some pythonnet objects in its gc list of
/// potentially-leaked objects. It calls tp_traverse on those objects.
/// But tp_traverse was freed in step 3 => CRASH.
///
/// This test distills what's going on without needing Unity around (we'd see
/// similar behaviour if we were using pythonnet on a .NET web server that did
/// a hot reload).
/// </summary>
[Test]
public static void DomainReloadAndGC()
{
// We're set up to run in the directory that includes the bin directory.
System.IO.Directory.SetCurrentDirectory(AppDomain.CurrentDomain.BaseDirectory);

Assembly pythonRunner1 = BuildAssembly("test1");
RunAssemblyAndUnload(pythonRunner1, "test1");

// Verify that python is not initialized even though we ran it.
Assert.That(Runtime.Runtime.Py_IsInitialized(), Is.Zero);

// This caused a crash because objects allocated in pythonRunner1
// still existed in memory, but the code to do python GC on those
// objects is gone.
Assembly pythonRunner2 = BuildAssembly("test2");
RunAssemblyAndUnload(pythonRunner2, "test2");
}

/// <summary>
/// On travis, printing to stdout causes a race condition (which
/// becomes visible as either EPIPE or a deadlock). Set Verbose to true
/// to print-debug the example on your laptop, but keep it off for CI.
///
/// Verbose is protected to quash a warning that it's never set.
/// </summary>
protected static bool Verbose = false;

/// <summary>
/// On travis, printing to stdout causes a race condition (which
/// becomes visible as either EPIPE or a deadlock). Set Verbose to true
/// to print-debug the example on your laptop, but keep it off for CI.
/// </summary>
static void WriteLine(string line) {
if (!Verbose) { return; }
Console.WriteLine(line);
Console.Out.Flush();
}

/// <summary>
/// The code we'll test. All that really matters is
/// using GIL { Python.Exec(pyScript); }
/// but the rest is useful for debugging.
///
/// What matters in the python code is gc.collect and clr.AddReference.
///
/// On Windows, the language version is 2.0, so no $"foo{bar}" syntax.
///
/// On travis, printing to stdout causes a race condition (which
/// becomes visible as either EPIPE or a deadlock). Set Verbose to true
/// to print-debug the example on your laptop, but keep it off for CI.
/// </summary>
const string TestCode = @"
using Python.Runtime;
using System;
class PythonRunner {
protected static bool Verbose = false;
static void WriteLine(string line) {
if (!Verbose) { return; }
Console.WriteLine(line);
Console.Out.Flush();
}
public static void RunPython() {
AppDomain.CurrentDomain.DomainUnload += OnDomainUnload;
string name = AppDomain.CurrentDomain.FriendlyName;
WriteLine(string.Format(""[{0} in .NET] In PythonRunner.RunPython"", name));

using (Py.GIL()) {
try {
var pyScript = string.Format(""import clr\n""
+ ""print('[{0} in python] imported clr')\n""
+ ""clr.AddReference('System')\n""
+ ""print('[{0} in python] allocated a clr object')\n""
+ ""import gc\n""
+ ""gc.collect()\n""
+ ""print('[{0} in python] collected garbage')\n"",
name);
PythonEngine.Exec(pyScript);
} catch(Exception e) {
WriteLine(string.Format(""[{0} in .NET] Caught exception: {1}"", name, e));
}
}
}
static void OnDomainUnload(object sender, EventArgs e) {
WriteLine(string.Format(""[{0} in .NET] unloading"", AppDomain.CurrentDomain.FriendlyName));
}
}";


/// <summary>
/// Build an assembly out of the source code above.
///
/// This creates a file <paramref name="assemblyName"/>.dll in order
/// to support the statement "proxy.theAssembly = assembly" below.
/// That statement needs a file, can't run via memory.
/// </summary>
static Assembly BuildAssembly(string assemblyName)
{
var provider = CodeDomProvider.CreateProvider("CSharp");

var compilerparams = new CompilerParameters();
compilerparams.ReferencedAssemblies.Add("Python.Runtime.dll");
compilerparams.GenerateExecutable = false;
compilerparams.GenerateInMemory = false;
compilerparams.IncludeDebugInformation = false;
compilerparams.OutputAssembly = assemblyName;

var results = provider.CompileAssemblyFromSource(compilerparams, TestCode);
if (results.Errors.HasErrors)
{
var errors = new System.Text.StringBuilder("Compiler Errors:\n");
foreach (CompilerError error in results.Errors)
{
errors.AppendFormat("Line {0},{1}\t: {2}\n",
error.Line, error.Column, error.ErrorText);
}
throw new Exception(errors.ToString());
}
else
{
return results.CompiledAssembly;
}
}

/// <summary>
/// This is a magic incantation required to run code in an application
/// domain other than the current one.
/// </summary>
class Proxy : MarshalByRefObject
{
Assembly theAssembly = null;

public void InitAssembly(string assemblyPath)
{
theAssembly = Assembly.LoadFile(System.IO.Path.GetFullPath(assemblyPath));
}

public void RunPython()
{
WriteLine("[Proxy] Entering RunPython");

// Call into the new assembly. Will execute Python code
var pythonrunner = theAssembly.GetType("PythonRunner");
var runPythonMethod = pythonrunner.GetMethod("RunPython");
runPythonMethod.Invoke(null, new object[] { });

WriteLine("[Proxy] Leaving RunPython");
}
}

/// <summary>
/// Create a domain, run the assembly in it (the RunPython function),
/// and unload the domain.
/// </summary>
static void RunAssemblyAndUnload(Assembly assembly, string assemblyName)
{
WriteLine($"[Program.Main] === creating domain for assembly {assembly.FullName}");

// Create the domain. Make sure to set PrivateBinPath to a relative
// path from the CWD (namely, 'bin').
// See https://stackoverflow.com/questions/24760543/createinstanceandunwrap-in-another-domain
var currentDomain = AppDomain.CurrentDomain;
var domainsetup = new AppDomainSetup()
{
ApplicationBase = currentDomain.SetupInformation.ApplicationBase,
ConfigurationFile = currentDomain.SetupInformation.ConfigurationFile,
LoaderOptimization = LoaderOptimization.SingleDomain,
PrivateBinPath = "."
};
var domain = AppDomain.CreateDomain(
$"My Domain {assemblyName}",
currentDomain.Evidence,
domainsetup);

// Create a Proxy object in the new domain, where we want the
// assembly (and Python .NET) to reside
Type type = typeof(Proxy);
System.IO.Directory.SetCurrentDirectory(AppDomain.CurrentDomain.BaseDirectory);
var theProxy = (Proxy)domain.CreateInstanceAndUnwrap(
type.Assembly.FullName,
type.FullName);

// From now on use the Proxy to call into the new assembly
theProxy.InitAssembly(assemblyName);
theProxy.RunPython();

WriteLine($"[Program.Main] Before Domain Unload on {assembly.FullName}");
AppDomain.Unload(domain);
WriteLine($"[Program.Main] After Domain Unload on {assembly.FullName}");

// Validate that the assembly does not exist anymore
try
{
WriteLine($"[Program.Main] The Proxy object is valid ({theProxy}). Unexpected domain unload behavior");
}
catch (Exception)
{
WriteLine("[Program.Main] The Proxy object is not valid anymore, domain unload complete.");
}
}

/// <summary>
/// Resolves the assembly. Why doesn't this just work normally?
/// </summary>
static Assembly ResolveAssembly(object sender, ResolveEventArgs args)
{
var loadedAssemblies = AppDomain.CurrentDomain.GetAssemblies();

foreach (var assembly in loadedAssemblies)
{
if (assembly.FullName == args.Name)
{
return assembly;
}
}

return null;
}
}
}
#endif
20 changes: 20 additions & 0 deletions src/embed_tests/TestRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,26 @@ namespace Python.EmbeddingTest
{
public class TestRuntime
{
/// <summary>
/// Test the cache of the information from the platform module.
///
/// Test fails on platforms we haven't implemented yet.
/// </summary>
[Test]
public static void PlatformCache()
{
Runtime.Runtime.Initialize();

Assert.That(Runtime.Runtime.Machine, Is.Not.EqualTo(Runtime.Runtime.MachineType.Other));
Assert.That(!string.IsNullOrEmpty(Runtime.Runtime.MachineName));

Assert.That(Runtime.Runtime.OperatingSystem, Is.Not.EqualTo(Runtime.Runtime.OperatingSystemType.Other));
Assert.That(!string.IsNullOrEmpty(Runtime.Runtime.OperatingSystemName));

// 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()
{
Expand Down
Loading