Create a clr.loader module

* Return ModuleObject.pyHandle, do not convert. * Write domain tests generated code to file.
pythonnet · filmor · Jun 18, 2021 · Jan 15, 2021 · Jan 15, 2021 · Jan 15, 2021
commit 31ea87698811ac382243f7328bc0885bfb4e5c75
diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs
@@ -202,6 +202,14 @@ internal static IntPtr ToPython(object value, Type type)
                     return ClassDerivedObject.ToPython(pyderived);
             }
 
+            // ModuleObjects are created in a way that their wrapping them as
+            // a CLRObject fails, the ClassObject has no tpHandle. Return the
+            // pyHandle as is, do not convert.
+            if (value is ModuleObject modobj)
+            {
+                return modobj.pyHandle;
+            }
+
             // hmm - from Python, we almost never care what the declared
             // type is. we'd rather have the object bound to the actual
             // implementing class.

diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Runtime.InteropServices;
 
 namespace Python.Runtime
@@ -18,9 +19,6 @@ import sys
 
 class DotNetLoader(importlib.abc.Loader):
 
-    def __init__(self):
-        super().__init__()
-
     @classmethod
     def exec_module(klass, mod):
         # This method needs to exist.
@@ -32,13 +30,13 @@ import clr
         return clr._load_clr_module(spec)
 
 class DotNetFinder(importlib.abc.MetaPathFinder):
-
-    def __init__(self):
-        super().__init__()
-
+
     @classmethod
     def find_spec(klass, fullname, paths=None, target=None): 
-        import clr
+        # Don't import, we might call ourselves recursively!
+        if 'clr' not in sys.modules:
+            return None
+        clr = sys.modules['clr']
         if clr._available_namespaces and fullname in clr._available_namespaces:
             return importlib.machinery.ModuleSpec(fullname, DotNetLoader(), is_package=True)
         return None
@@ -64,13 +62,10 @@ internal static unsafe void Initialize()
             BorrowedReference dict = Runtime.PyImport_GetModuleDict();
             Runtime.PyDict_SetItemString(dict, "CLR", ClrModuleReference);
             Runtime.PyDict_SetItemString(dict, "clr", ClrModuleReference);
-
-            // Add/create the MetaPathLoader
             SetupNamespaceTracking();
             SetupImportHook();
         }
 
-
         /// <summary>
         /// Cleanup resources upon shutdown of the Python runtime.
         /// </summary>
@@ -81,11 +76,10 @@ internal static void Shutdown()
                 return;
             }
 
-            bool shouldFreeDef = Runtime.Refcount(py_clr_module) == 1;
+            TeardownNameSpaceTracking();
             Runtime.XDecref(py_clr_module);
             py_clr_module = IntPtr.Zero;
 
-            TeardownNameSpaceTracking();
             Runtime.XDecref(root.pyHandle);
             root = null;
             CLRModule.Reset();
@@ -107,62 +101,42 @@ internal static void RestoreRuntimeData(RuntimeDataStorage storage)
             var rootHandle = storage.GetValue<IntPtr>("root");
             root = (CLRModule)ManagedType.GetManagedObject(rootHandle);
             BorrowedReference dict = Runtime.PyImport_GetModuleDict();
-            Runtime.PyDict_SetItemString(dict.DangerousGetAddress(), "CLR", py_clr_module);
             Runtime.PyDict_SetItemString(dict.DangerousGetAddress(), "clr", py_clr_module);
             SetupNamespaceTracking();
         }
 
         static void SetupImportHook()
         {
             // Create the import hook module
-            var import_hook_module_def = ModuleDefOffset.AllocModuleDef("clr.loader");
-            var import_hook_module = Runtime.PyModule_Create2(import_hook_module_def, 3);
+            var import_hook_module = Runtime.PyModule_New("clr.loader");
 
             // Run the python code to create the module's classes.
-            var mod_dict = Runtime.PyModule_GetDict(new BorrowedReference(import_hook_module));
             var builtins = Runtime.PyEval_GetBuiltins();
             var exec = Runtime.PyDict_GetItemString(builtins, "exec");
             using var args = NewReference.DangerousFromPointer(Runtime.PyTuple_New(2));
 
-            var codeStr = Runtime.PyString_FromString(LoaderCode);
+            IntPtr codeStr = Runtime.PyString_FromString(LoaderCode);
             Runtime.PyTuple_SetItem(args.DangerousGetAddress(), 0, codeStr);
             // PyTuple_SetItem steals a reference, mod_dict is borrowed.
+            var mod_dict = Runtime.PyModule_GetDict(import_hook_module);
             Runtime.XIncref(mod_dict.DangerousGetAddress());
             Runtime.PyTuple_SetItem(args.DangerousGetAddress(), 1, mod_dict.DangerousGetAddress());
             Runtime.PyObject_Call(exec.DangerousGetAddress(), args.DangerousGetAddress(), IntPtr.Zero);
 internal static IntPtr PyObject_Type(IntPtr op) 
 internal static IntPtr PyObject_Type(IntPtr op) 
 
-            var loader = Runtime.PyDict_GetItemString(mod_dict, "DotNetLoader").DangerousGetAddressOrNull();
-            Runtime.XIncref(loader);
-
-            // Add the classes to the module
-            // PyModule_AddObject steals a reference only on success
-            if (Runtime.PyModule_AddObject(import_hook_module, "DotNetLoader", loader) != 0)
-            {
-                Runtime.XDecref(loader);
-                throw new PythonException();
-            }
-
-            var finder = Runtime.PyDict_GetItemString(mod_dict, "DotNetFinder").DangerousGetAddressOrNull();
-            Runtime.XIncref(finder);
-            if (Runtime.PyModule_AddObject(import_hook_module, "DotNetFinder", finder) != 0)
-            {
-                Runtime.XDecref(finder);
-                throw new PythonException();
-            }
-
             // Set as a sub-module of clr.
-            Runtime.XIncref(import_hook_module);
-            if(Runtime.PyModule_AddObject(py_clr_module, "loader", import_hook_module) != 0)
+            if(Runtime.PyModule_AddObject(ClrModuleReference, "loader", import_hook_module) != 0)
             {
-                Runtime.XDecref(import_hook_module);
+                Runtime.XDecref(import_hook_module.DangerousGetAddress());
                 throw new PythonException();
             }
 
             // Finally, add the hook to the meta path
-            var finder_inst = Runtime.PyDict_GetItemString(mod_dict, "finder_inst").DangerousGetAddressOrNull();
-            Runtime.XIncref(finder);
+            var findercls = Runtime.PyDict_GetItemString(mod_dict, "DotNetFinder");
+            var finderCtorArgs = Runtime.PyTuple_New(0);
+            var finder_inst = Runtime.PyObject_CallObject(findercls.DangerousGetAddress(), finderCtorArgs);
+            Runtime.XDecref(finderCtorArgs);
             var metapath = Runtime.PySys_GetObject("meta_path");
-            Runtime.PyList_Append(metapath, finder);
+            Runtime.PyList_Append(metapath, finder_inst);
         }
 
         /// <summary>
@@ -268,7 +242,8 @@ public static unsafe NewReference GetCLRModule()
         }
 
         /// <summary>
-        /// The hook to import a CLR module into Python
+        /// The hook to import a CLR module into Python. Returns a new reference
+        /// to the module.
         /// </summary>
         public static ModuleObject Import(string modname)
         {
@@ -305,7 +280,7 @@ public static ModuleObject Import(string modname)
                     tail.LoadNames();
                 }
             }
-
+            tail.IncrRefCount();
             return tail;
         }
 

diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs
@@ -53,7 +53,7 @@ public ModuleObject(string name)
             var dictRef = Runtime.PyObject_GenericGetDict(ObjectReference);
             PythonException.ThrowIfIsNull(dictRef);
             dict = dictRef.DangerousMoveToPointer();
-
+            __all__ = Runtime.PyList_New(0);
             using var pyname = NewReference.DangerousFromPointer(Runtime.PyString_FromString(moduleName));
             using var pyfilename = NewReference.DangerousFromPointer(Runtime.PyString_FromString(filename));
             using var pydocstring = NewReference.DangerousFromPointer(Runtime.PyString_FromString(docstring));
@@ -576,13 +576,6 @@ public static string[] ListAssemblies(bool verbose)
             return names;
         }
 
-        [ModuleFunction]
-        public static int _AtExit()
-        {
-            return Runtime.AtExit();
-        }
-
-
         /// <summary>
         /// Note: This should *not* be called directly.
         /// The function that get/import a CLR assembly as a python module.
@@ -593,16 +586,14 @@ public static int _AtExit()
         /// <returns>A new reference to the imported module, as a PyObject.</returns>
         [ModuleFunction]
         [ForbidPythonThreads]
-        public static PyObject _load_clr_module(PyObject spec)
+        public static ModuleObject _load_clr_module(PyObject spec)
         {
             ModuleObject mod = null;
             using (var modname = spec.GetAttr("name"))
             {
                 mod = ImportHook.Import(modname.ToString());
             }
-            // We can't return directly a ModuleObject, because the tpHandle is
-            // not set, but we can return a PyObject.
-            return new PyObject(mod.pyHandle);
+            return mod;
         }
     }
 }
diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs
@@ -1944,7 +1944,7 @@ internal static string PyModule_GetFilename(IntPtr module)
 
 
         internal static IntPtr PyImport_Import(IntPtr name) => Delegates.PyImport_Import(name);
-        internal static int PyModule_AddObject(IntPtr module, string name, IntPtr stolenObject)
+        internal static int PyModule_AddObject(BorrowedReference module, string name, BorrowedReference stolenObject)
         {
             using var namePtr = new StrPtr(name, Encoding.UTF8);
             return Delegates.PyModule_AddObject(module, namePtr, stolenObject);
@@ -2507,7 +2507,7 @@ static Delegates()
                 {
                     PyModule_Create2 = (delegate* unmanaged[Cdecl]<IntPtr, int, IntPtr>)GetFunctionByName("PyModule_Create2TraceRefs", GetUnmanagedDll(_PythonDll));
                 }
-                PyModule_AddObject = (delegate* unmanaged[Cdecl]<IntPtr, StrPtr, IntPtr, int>)GetFunctionByName(nameof(PyModule_AddObject), GetUnmanagedDll(_PythonDll));
+                PyModule_AddObject = (delegate* unmanaged[Cdecl]<BorrowedReference, StrPtr, BorrowedReference, int>)GetFunctionByName(nameof(PyModule_AddObject), GetUnmanagedDll(_PythonDll));
                 PyImport_Import = (delegate* unmanaged[Cdecl]<IntPtr, IntPtr>)GetFunctionByName(nameof(PyImport_Import), GetUnmanagedDll(_PythonDll));
                 PyImport_ImportModule = (delegate* unmanaged[Cdecl]<StrPtr, NewReference>)GetFunctionByName(nameof(PyImport_ImportModule), GetUnmanagedDll(_PythonDll));
                 PyImport_ReloadModule = (delegate* unmanaged[Cdecl]<BorrowedReference, NewReference>)GetFunctionByName(nameof(PyImport_ReloadModule), GetUnmanagedDll(_PythonDll));
@@ -2797,7 +2797,7 @@ static Delegates()
             internal static delegate* unmanaged[Cdecl]<BorrowedReference, BorrowedReference> PyModule_GetDict { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, StrPtr> PyModule_GetFilename { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, int, IntPtr> PyModule_Create2 { get; }
-            internal static delegate* unmanaged[Cdecl]<IntPtr, StrPtr, IntPtr, int> PyModule_AddObject { get; }
+            internal static delegate* unmanaged[Cdecl]<BorrowedReference, StrPtr, BorrowedReference, int> PyModule_AddObject { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, IntPtr> PyImport_Import { get; }
             internal static delegate* unmanaged[Cdecl]<StrPtr, NewReference> PyImport_ImportModule { get; }
             internal static delegate* unmanaged[Cdecl]<BorrowedReference, NewReference> PyImport_ReloadModule { get; }

diff --git a/tests/domain_tests/TestRunner.cs b/tests/domain_tests/TestRunner.cs
@@ -30,6 +30,11 @@ namespace Python.DomainReloadTests
     /// which test case to run. That's because pytest assumes we'll run
     /// everything in one process, but we really want a clean process on each
     /// test case to test the init/reload/teardown parts of the domain reload.
+    /// 
+    /// ### Debugging tips: ###
+    /// * Running pytest with the `-s` argument prevents stdout capture by pytest
+    /// * Add a sleep into the python test case before the crash/failure, then while 
+    ///   sleeping, attach the debugger to the Python.TestDomainReload.exe process.
     /// </summary>
     /// 
     class TestRunner
@@ -1287,7 +1292,13 @@ static string CreateAssembly(string name, string code, bool exe = false)
             }
             parameters.ReferencedAssemblies.Add(netstandard);
             parameters.ReferencedAssemblies.Add(PythonDllLocation);
-            CompilerResults results = provider.CompileAssemblyFromSource(parameters, code);
+            // Write code to file so it can debugged.
+            var sourcePath = Path.Combine(TestPath, name+"_source.cs");
+            using(var file = new StreamWriter(sourcePath))
+            {
+                file.Write(code);
+            }
+            CompilerResults results = provider.CompileAssemblyFromFile(parameters, sourcePath);
             if (results.NativeCompilerReturnValue != 0)
             {
                 var stderr = System.Console.Error;

diff --git a/tests/domain_tests/test_domain_reload.py b/tests/domain_tests/test_domain_reload.py
@@ -89,6 +89,5 @@ def test_in_to_ref_param():
 def test_nested_type():
     _run_test("nested_type")
 
-@pytest.mark.skipif(platform.system() == 'Darwin', reason='FIXME: macos can\'t find the python library')
 def test_import_after_reload():
     _run_test("import_after_reload")