Unity-Technologies · benoithudson · Oct 2, 2018 · Sep 14, 2018 · Sep 14, 2018 · Sep 26, 2018
diff --git a/AUTHORS.md b/AUTHORS.md
@@ -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))
@@ -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))
@@ -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))

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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])

diff --git a/src/embed_tests/Python.EmbeddingTest.csproj b/src/embed_tests/Python.EmbeddingTest.csproj
@@ -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" />
@@ -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">
@@ -122,4 +124,4 @@
     <Copy SourceFiles="$(TargetAssembly)" DestinationFolder="$(PythonBuildDir)" />
     <!--Copy SourceFiles="$(TargetAssemblyPdb)" Condition="Exists('$(TargetAssemblyPdb)')" DestinationFolder="$(PythonBuildDir)" /-->
   </Target>
-</Project>
+</Project>
diff --git a/src/embed_tests/TestDomainReload.cs b/src/embed_tests/TestDomainReload.cs
@@ -0,0 +1,230 @@
+using System;
+using System.CodeDom.Compiler;
+using System.Reflection;
+using NUnit.Framework;
+using Python.Runtime;
+
+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");
+        }
+
+        //
+        // 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.
+        //
+        // Note that the language version is 2.0, so no $"foo{bar}" syntax.
+        //
+        const string TestCode = @"
+            using Python.Runtime;
+            using System;
+            class PythonRunner {
+                public static void RunPython() {
+                    AppDomain.CurrentDomain.DomainUnload += OnDomainUnload;
+                    string name = AppDomain.CurrentDomain.FriendlyName;
+                    Console.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) {
+                            Console.WriteLine(string.Format(""[{0} in .NET] Caught exception: {1}"", name, e));
+                        }
+                    }
+                }
+                static void OnDomainUnload(object sender, EventArgs e) {
+                    System.Console.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()
+            {
+                Console.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[] { });
+
+                Console.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)
+        {
+            Console.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();
+
+            Console.WriteLine($"[Program.Main] Before Domain Unload on {assembly.FullName}");
+            AppDomain.Unload(domain);
+            Console.WriteLine($"[Program.Main] After Domain Unload on {assembly.FullName}");
+
+            // Validate that the assembly does not exist anymore
+            try
+            {
+                Console.WriteLine($"[Program.Main] The Proxy object is valid ({theProxy}). Unexpected domain unload behavior");
+            }
+            catch (Exception)
+            {
+                Console.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;
+        }
+    }
+}
diff --git a/src/embed_tests/TestRuntime.cs b/src/embed_tests/TestRuntime.cs
@@ -6,6 +6,25 @@ 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));
+
+            Runtime.Runtime.Shutdown();
+        }
+
         [Test]
         public static void Py_IsInitializedValue()
         {

diff --git a/src/embed_tests/TestTypeManager.cs b/src/embed_tests/TestTypeManager.cs
@@ -0,0 +1,52 @@
+using NUnit.Framework;
+using Python.Runtime;
+using System.Runtime.InteropServices;
+
+namespace Python.EmbeddingTest
+{
+    class TestTypeManager
+    {
+        [Test]
+        public static void TestNativeCode()
+        {
+            Runtime.Runtime.Initialize();
+
+            Assert.That(() => { var _ = TypeManager.NativeCode.Active; }, Throws.Nothing);
+            Assert.That(TypeManager.NativeCode.Active.Code.Length, Is.GreaterThan(0));
+
+            Runtime.Runtime.Shutdown();
+        }
+
+        [Test]
+        public static void TestMemoryMapping()
+        {
+            Runtime.Runtime.Initialize();
+
+            Assert.That(() => { var _ = TypeManager.CreateMemoryMapper(); }, Throws.Nothing);
+            var mapper = TypeManager.CreateMemoryMapper();
+
+            // Allocate a read-write page.
+            int len = 12;
+            var page = mapper.MapWriteable(len);
+            Assert.That(() => { Marshal.WriteInt64(page, 17); }, Throws.Nothing);
+            Assert.That(Marshal.ReadInt64(page), Is.EqualTo(17));
+
+            // Mark it read-execute, now we can't write anymore.
+            //
+            // We can't actually test access protection under Windows, because
+            // AccessViolationException is assumed to mean we're in a corrupted
+            // state:
+            //   https://stackoverflow.com/questions/3469368/how-to-handle-accessviolationexception
+            mapper.SetReadExec(page, len);
+            Assert.That(Marshal.ReadInt64(page), Is.EqualTo(17));
+            if (Runtime.Runtime.OperatingSystem != Runtime.Runtime.OperatingSystemType.Windows)
+            {
+                // Mono throws NRE instead of AccessViolationException for some reason.
+                Assert.That(() => { Marshal.WriteInt64(page, 73); }, Throws.TypeOf<System.NullReferenceException>());
+                Assert.That(Marshal.ReadInt64(page), Is.EqualTo(17));
+            }
+
+            Runtime.Runtime.Shutdown();
+        }
+    }
+}
diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs
@@ -247,24 +247,6 @@ public static IntPtr tp_str(IntPtr ob)
         }
 
 
-        /// <summary>
-        /// Default implementations for required Python GC support.
-        /// </summary>
-        public static int tp_traverse(IntPtr ob, IntPtr func, IntPtr args)
-        {
-            return 0;
-        }
-
-        public static int tp_clear(IntPtr ob)
-        {
-            return 0;
-        }
-
-        public static int tp_is_gc(IntPtr type)
-        {
-            return 1;
-        }
-
         /// <summary>
         /// Standard dealloc implementation for instances of reflected types.
         /// </summary>