8000 Merge pull request #7 from Unity-Technologies/uni-63112-hotreload-cra… · Unity-Technologies/pythonnet@e585bdc · GitHub
[go: up one dir, main page]
Skip to content

Commit e585bdc

Browse files
benoithudsonbenoithudson
authored
Merge pull request #7 from Unity-Technologies/uni-63112-hotreload-crash-test
Uni 63112 hotreload crash test
2 parents b6417ca + b9f6c2c commit e585bdc

File tree

11 files changed

+688
-50
lines changed

11 files changed

+688
-50
lines changed

‎AUTHORS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
- Alexandre Catarino([@AlexCatarino](https://github.com/AlexCatarino))
1616
- Arvid JB ([@ArvidJB](https://github.com/ArvidJB))
17+
- Benoît Hudson ([@benoithudson](https://github.com/benoithudson))
1718
- Bradley Friedman ([@leith-bartrich](https://github.com/leith-bartrich))
1819
- Callum Noble ([@callumnoble](https://github.com/callumnoble))
1920
- Christian Heimes ([@tiran](https://github.com/tiran))
@@ -22,6 +23,7 @@
2223
- Daniel Fernandez ([@fdanny](https://github.com/fdanny))
2324
- Daniel Santana ([@dgsantana](https://github.com/dgsantana))
2425
- Dave Hirschfeld ([@dhirschfeld](https://github.com/dhirschfeld))
26+
- David Lassonde ([@lassond](https://github.com/lassond))
2527
- David Lechner ([@dlech](https://github.com/dlech))
2628
- Dmitriy Se ([@dmitriyse](https://github.com/dmitriyse))
2729
- He-chien Tsai ([@t3476](https://github.com/t3476))
@@ -39,6 +41,7 @@
3941
- Sam Winstanley ([@swinstanley](https://github.com/swinstanley))
4042
- Sean Freitag ([@cowboygneox](https://github.com/cowboygneox))
4143
- Serge Weinstock ([@sweinst](https://github.com/sweinst))
44+
- Viktoria Kovescses ([@vkovec](https://github.com/vkovec))
4245
- Ville M. Vainio ([@vivainio](https://github.com/vivainio))
4346
- Virgil Dupras ([@hsoft](https://github.com/hsoft))
4447
- Wenguang Yang ([@yagweb](https://github.com/yagweb))

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
2525
### Fixed
2626

2727
- Fixed Visual Studio 2017 compat ([#434][i434]) for setup.py
28+
- Fixed crashes when integrating pythonnet in Unity3d ([#714][i714]),
29+
related to unloading the Application Domain
2830
- Fixed crash on exit of the Python interpreter if a python class
2931
derived from a .NET class has a `__namespace__` or `__assembly__`
3032
attribute ([#481][i481])

‎src/embed_tests/Python.EmbeddingTest.csproj

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@
8686
<Compile Include="pyrunstring.cs" />
8787
<Compile Include="TestConverter.cs" />
8888
<Compile Include="TestCustomMarshal.cs" />
89+
<Compile Include="TestDomainReload.cs" />
8990
<Compile Include="TestExample.cs" />
9091
<Compile Include="TestPyAnsiString.cs" />
9192
<Compile Include="TestPyFloat.cs" />
@@ -103,6 +104,7 @@
103104
<Compile Include="TestPyWith.cs" />
104105
<Compile Include="TestRuntime.cs" />
105106
<Compile Include="TestPyScope.cs" />
107+
<Compile Include="TestTypeManager.cs" />
106108
</ItemGroup>
107109
<ItemGroup>
108110
<ProjectReference Include="..\runtime\Python.Runtime.csproj">
@@ -122,4 +124,4 @@
122124
<Copy SourceFiles="$(TargetAssembly)" DestinationFolder="$(PythonBuildDir)" />
123125
<!--Copy SourceFiles="$(TargetAssemblyPdb)" Condition="Exists('$(TargetAssemblyPdb)')" DestinationFolder="$(PythonBuildDir)" /-->
124126
</Target>
125-
</Project>
127+
</Project>

‎src/embed_tests/TestDomainReload.cs

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
using System;
2+
using System.CodeDom.Compiler;
3+
using System.Reflection;
4+
using NUnit.Framework;
5+
using Python.Runtime;
6+
7+
namespace Python.EmbeddingTest
8+
{
9+
class TestDomainReload
10+
{
11+
/// <summary>
12+
/// Test that the python runtime can survive a C# domain reload without crashing.
13+
///
14+
/// At the time this test was written, there was a very annoying
15+
/// seemingly random crash bug when integrating pythonnet into Unity.
16+
///
17+
/// The repro steps that David Lassonde, Viktoria Kovecses and
18+
/// Benoit Hudson eventually worked out:
19+
/// 1. Write a HelloWorld.cs script that uses Python.Runtime to access
20+
/// some C# data from python: C# calls python, which calls C#.
21+
/// 2. Execute the script (e.g. make it a MenuItem and click it).
22+
/// 3. Touch HelloWorld.cs on disk, forcing Unity to recompile scripts.
23+
/// 4. Wait several seconds for Unity to be done recompiling and
24+
/// reloading the C# domain.
25+
/// 5. Make python run the gc (e.g. by calling gc.collect()).
26+
///
27+
/// The reason:
28+
/// A. In step 2, Python.Runtime registers a bunch of new types with
29+
/// their tp_traverse slot pointing to managed code, and allocates
30+
/// some objects of those types.
31+
/// B. In step 4, Unity unloads the C# domain. That frees the managed
32+
/// code. But at the time of the crash investigation, pythonnet
33+
/// leaked the python side of the objects allocated in step 1.
34+
/// C. In step 5, python sees some pythonnet objects in its gc list of
35+
/// potentially-leaked objects. It calls tp_traverse on those objects.
36+
/// But tp_traverse was freed in step 3 => CRASH.
37+
///
38+
/// This test distills what's going on without needing Unity around (we'd see
39+
/// similar behaviour if we were using pythonnet on a .NET web server that did
40+
/// a hot reload).
41+
/// </summary>
42+
[Test]
43+
public static void DomainReloadAndGC()
44+
{
45+
// We're set up to run in the directory that includes the bin directory.
46+
System.IO.Directory.SetCurrentDirectory(AppDomain.CurrentDomain.BaseDirectory);
47+
48+
Assembly pythonRunner1 = BuildAssembly("test1");
49+
RunAssemblyAndUnload(pythonRunner1, "test1");
50+
51+
// Verify that python is not initialized even though we ran it.
52+
Assert.That(Runtime.Runtime.Py_IsInitialized(), Is.Zero);
53+
54+
// This caused a crash because objects allocated in pythonRunner1
55+
// still existed in memory, but the code to do python GC on those
56+
// objects is gone.
57+
Assembly pythonRunner2 = BuildAssembly("test2");
58+
RunAssemblyAndUnload(pythonRunner2, "test2");
59+
}
60+
61+
//
62+
// The code we'll test. All that really matters is
63+
// using GIL { Python.Exec(pyScript); }
64+
// but the rest is useful for debugging.
65+
//
66+
// What matters in the python code is gc.collect and clr.AddReference.
67+
//
68+
// Note that the language version is 2.0, so no $"foo{bar}" syntax.
69+
//
70+
const string TestCode = @"
71+
using Python.Runtime;
72+
using System;
73+
class PythonRunner {
74+
public static void RunPython() {
75+
AppDomain.CurrentDomain.DomainUnload += OnDomainUnload;
76+
string name = AppDomain.CurrentDomain.FriendlyName;
77+
Console.WriteLine(string.Format(""[{0} in .NET] In PythonRunner.RunPython"", name));
78+
using (Py.GIL()) {
79+
try {
80+
var pyScript = string.Format(""import clr\n""
81+
+ ""print('[{0} in python] imported clr')\n""
82+
+ ""clr.AddReference('System')\n""
83+
+ ""print('[{0} in python] allocated a clr object')\n""
84+
+ ""import gc\n""
85+
+ ""gc.collect()\n""
86+
+ ""print('[{0} in python] collected garbage')\n"",
87+
name);
88+
PythonEngine.Exec(pyScript);
89+
} catch(Exception e) {
90+
Console.WriteLine(string.Format(""[{0} in .NET] Caught exception: {1}"", name, e));
91+
}
92+
}
93+
}
94+
static void OnDomainUnload(object sender, EventArgs e) {
95+
System.Console.WriteLine(string.Format(""[{0} in .NET] unloading"", AppDomain.CurrentDomain.FriendlyName));
96+
}
97+
}";
98+
99+
100+
/// <summary>
101+
/// Build an assembly out of the source code above.
102+
///
103+
/// This creates a file <paramref name="assemblyName"/>.dll in order
104+
/// to support the statement "proxy.theAssembly = assembly" below.
105+
/// That statement needs a file, can't run via memory.
106+
/// </summary>
107+
static Assembly BuildAssembly(string assemblyName)
108+
{
109+
var provider = CodeDomProvider.CreateProvider("CSharp");
110+
111+
var compilerparams = new CompilerParameters();
112+
compilerparams.ReferencedAssemblies.Add("Python.Runtime.dll");
113+
compilerparams.GenerateExecutable = false;
114+
compilerparams.GenerateInMemory = false;
115+
compilerparams.IncludeDebugInformation = false;
116+
compilerparams.OutputAssembly = assemblyName;
117+
118+
var results = provider.CompileAssemblyFromSource(compilerparams, TestCode);
119+
if (results.Errors.HasErrors)
120+
{
121+
var errors = new System.Text.StringBuilder("Compiler Errors:\n");
122+
foreach (CompilerError error in results.Errors)
123+
{
124+
errors.AppendFormat("Line {0},{1}\t: {2}\n",
125+
error.Line, error.Column, error.ErrorText);
126+
}
127+
throw new Exception(errors.ToString());
128+
}
129+
else
130+
{
131+
return results.CompiledAssembly;
132+
}
133+
}
134+
135+
/// <summary>
136+
/// This is a magic incantation required to run code in an application
137+
/// domain other than the current one.
138+
/// </summary>
139+
class Proxy : MarshalByRefObject
140+
{
141+
Assembly theAssembly = null;
142+
143+
public void InitAssembly(string assemblyPath)
144+
{
145+
theAssembly = Assembly.LoadFile(System.IO.Path.GetFullPath(assemblyPath));
146+
}
147+
148+
public void RunPython()
149+
{
150+
Console.WriteLine("[Proxy] Entering RunPython");
151+
152+
// Call into the new assembly. Will execute Python code
153+
var pythonrunner = theAssembly.GetType("PythonRunner");
154+
var runPythonMethod = pythonrunner.GetMethod("RunPython");
155+
runPythonMethod.Invoke(null, new object[] { });
156+
157+
Console.WriteLine("[Proxy] Leaving RunPython");
158+
}
159+
}
160+
161+
/// <summary>
162+
/// Create a domain, run the assembly in it (the RunPython function),
163+
/// and unload the domain.
164+
/// </summary>
165+
static void RunAssemblyAndUnload(Assembly assembly, string assemblyName)
166+
{
167+
Console.WriteLine($"[Program.Main] === creating domain for assembly {assembly.FullName}");
168+
169+
// Create the domain. Make sure to set PrivateBinPath to a relative
170+
// path from the CWD (namely, 'bin').
171+
// See https://stackoverflow.com/questions/24760543/createinstanceandunwrap-in-another-domain
172+
var currentDomain = AppDomain.CurrentDomain;
173+
var domainsetup = new AppDomainSetup()
174+
{
175+
ApplicationBase = currentDomain.SetupInformation.ApplicationBase,
176+
ConfigurationFile = currentDomain.SetupInformation.ConfigurationFile,
177+
LoaderOptimization = LoaderOptimization.SingleDomain,
178+
PrivateBinPath = "."
179+
};
180+
var domain = AppDomain.CreateDomain(
181+
$"My Domain {assemblyName}",
182+
currentDomain.Evidence,
183+
domainsetup);
184+
185+
// Create a Proxy object in the new domain, where we want the
186+
// assembly (and Python .NET) to reside
187+
Type type = typeof(Proxy);
188+
System.IO.Directory.SetCurrentDirectory(AppDomain.CurrentDomain.BaseDirectory);
189+
var theProxy = (Proxy)domain.CreateInstanceAndUnwrap(
190+
type.Assembly.FullName,
191+
type.FullName);
192+
193+
// From now on use the Proxy to call into the new assembly
194+
theProxy.InitAssembly(assemblyName);
195+
theProxy.RunPython();
196+
197+
Console.WriteLine($"[Program.Main] Before Domain Unload on {assembly.FullName}");
198+
AppDomain.Unload(domain);
199+
Console.WriteLine($"[Program.Main] After Domain Unload on {assembly.FullName}");
2 10000 00+
201+
// Validate that the assembly does not exist anymore
202+
try
203+
{
204+
Console.WriteLine($"[Program.Main] The Proxy object is valid ({theProxy}). Unexpected domain unload behavior");
205+
}
206+
catch (Exception)
207+
{
208+
Console.WriteLine("[Program.Main] The Proxy object is not valid anymore, domain unload complete.");
209+
}
210+
}
211+
212+
/// <summary>
213+
/// Resolves the assembly. Why doesn't this just work normally?
214+
/// </summary>
215+
static Assembly ResolveAssembly(object sender, ResolveEventArgs args)
216+
{
217+
var loadedAssemblies = AppDomain.CurrentDomain.GetAssemblies();
218+
219+
foreach (var assembly in loadedAssemblies)
220+
{
221+
if (assembly.FullName == args.Name)
222+
{
223+
return assembly;
224+
}
225+
}
226+
227+
return null;
228+
}
229+
}
230+
}

‎src/embed_tests/TestRuntime.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,25 @@ namespace Python.EmbeddingTest
66
{
77
public class TestRuntime
88
{
9+
/// <summary>
10+
/// Test the cache of the information from the platform module.
11+
///
12+
/// Test fails on platforms we haven't implemented yet.
13+
/// </summary>
14+
[Test]
15+
public static void PlatformCache()
16+
{
17+
Runtime.Runtime.Initialize();
18+
19+
Assert.That(Runtime.Runtime.Machine, Is.Not.EqualTo(Runtime.Runtime.MachineType.Other));
20+
Assert.That(!string.IsNullOrEmpty(Runtime.Runtime.MachineName));
21+
22+
Assert.That(Runtime.Runtime.OperatingSystem, Is.Not.EqualTo(Runtime.Runtime.OperatingSystemType.Other));
23+
Assert.That(!string.IsNullOrEmpty(Runtime.Runtime.OperatingSystemName));
24+
25+
Runtime.Runtime.Shutdown();
26+
}
27+
928
[Test]
1029
public static void Py_IsInitializedValue()
1130
{

‎src/embed_tests/TestTypeManager.cs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
using NUnit.Framework;
2+
using Python.Runtime;
3+
using System.Runtime.InteropServices;
4+
5+
namespace Python.EmbeddingTest
6+
{
7+
class TestTypeManager
8+
{
9+
[Test]
10+
public static void TestNativeCode()
11+
{
12+
Runtime.Runtime.Initialize();
13+
14+
Assert.That(() => { var _ = TypeManager.NativeCode.Active; }, Throws.Nothing);
15+
Assert.That(TypeManager.NativeCode.Active.Code.Length, Is.GreaterThan(0));
16+
17+
Runtime.Runtime.Shutdown();
18+
}
19+
20+
[Test]
21+
public static void TestMemoryMapping()
22+
{
23+
Runtime.Runtime.Initialize();
24+
25+
Assert.That(() => { var _ = TypeManager.CreateMemoryMapper(); }, Throws.Nothing);
26+
var mapper = TypeManager.CreateMemoryMapper();
27+
28+
// Allocate a read-write page.
29+
int len = 12;
30+
var page = mapper.MapWriteable(len);
31+
Assert.That(() => { Marshal.WriteInt64(page, 17); }, Throws.Nothing);
32+
Assert.That(Marshal.ReadInt64(page), Is.EqualTo(17));
33+
34+
// Mark it read-execute, now we can't write anymore.
35+
//
36+
// We can't actually test access protection under Windows, because
37+
// AccessViolationException is assumed to mean we're in a corrupted
38+
// state:
39+
// https://stackoverflow.com/questions/3469368/how-to-handle-accessviolationexception
40+
mapper.SetReadExec(page, len);
41+
Assert.That(Marshal.ReadInt64(page), Is.EqualTo(17));
42+
if (Runtime.Runtime.OperatingSystem != Runtime.Runtime.OperatingSystemType.Windows)
43+
{
44+
// Mono throws NRE instead of AccessViolationException for some reason.
45+
Assert.That(() => { Marshal.WriteInt64(page, 73); }, Throws.TypeOf<System.NullReferenceException>());
46+
Assert.That(Marshal.ReadInt64(page), Is.EqualTo(17));
47+
}
48+
49+
Runtime.Runtime.Shutdown();
50+
}
51+
}
52+
}

‎src/runtime/classbase.cs

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -247,24 +247,6 @@ public static IntPtr tp_str(IntPtr ob)
247247
}
248248

249249

250-
/// <summary>
251-
/// Default implementations for required Python GC support.
252-
/// </summary>
253-
public static int tp_traverse(IntPtr ob, IntPtr func, IntPtr args)
254-
{
255-
return 0;
256-
}
257-
258-
public static int tp_clear(IntPtr ob)
259-
{
260-
return 0;
261-
}
262-
263-
public static int tp_is_gc(IntPtr type)
264-
{
265-
return 1;
266-
}
267-
268250
/// <summary>
269251
/// Standard dealloc implementation for instances of reflected types.
270252
/// </summary>

0 commit comments

Comments
 (0)
0