From a321daa0147a49984a1df027df9d3f6104e77950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Fri, 15 Jan 2021 14:03:40 -0500 Subject: [PATCH 01/20] (WIP) modernize the import hook Implement a meta path loader instead --- src/runtime/assemblymanager.cs | 23 +++ src/runtime/extensiontype.cs | 2 +- src/runtime/importhook.cs | 273 +++++++------------------------ src/runtime/moduleobject.cs | 75 ++++++++- src/runtime/native/TypeOffset.cs | 1 + 5 files changed, 160 insertions(+), 214 deletions(-) diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs index 0387d2dfc..fdde2aeb1 100644 --- a/src/runtime/assemblymanager.cs +++ b/src/runtime/assemblymanager.cs @@ -37,6 +37,9 @@ internal class AssemblyManager // modified from event handlers below, potentially triggered from different .NET threads private static ConcurrentQueue assemblies; internal static List pypath; + + // Triggered when a new namespace is added to the namespaces dictionary + public static event Action namespaceAdded; private AssemblyManager() { @@ -284,6 +287,17 @@ internal static void ScanAssembly(Assembly assembly) if (ns != null) { namespaces[ns].TryAdd(assembly, string.Empty); + try + { + namespaceAdded?.Invoke(ns); + } + catch (Exception e) + { + // For some reason, exceptions happening here does... nothing. + // Even System.AccessViolationExceptions gets ignored. + Console.WriteLine($"Namespace added callback failed with: {e}"); + throw; + } } if (ns != null && t.IsGenericTypeDefinition) @@ -312,6 +326,15 @@ public static bool IsValidNamespace(string name) return !string.IsNullOrEmpty(name) && namespaces.ContainsKey(name); } + /// + /// Returns an IEnumerable containing the namepsaces exported + /// by loaded assemblies in the current app domain. + /// + public static IEnumerable GetNamespaces () + { + return namespaces.Keys; + } + /// /// Returns list of assemblies that declare types in a given namespace /// diff --git a/src/runtime/extensiontype.cs b/src/runtime/extensiontype.cs index 78df805ee..0ebd7ec4c 100644 --- a/src/runtime/extensiontype.cs +++ b/src/runtime/extensiontype.cs @@ -83,7 +83,7 @@ public static int tp_setattro(IntPtr ob, IntPtr key, IntPtr val) { message = "readonly attribute"; } - Exceptions.SetError(Exceptions.TypeError, message); + Exceptions.SetError(Exceptions.AttributeError, message); return -1; } diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 9ac492d21..249fdebbe 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -15,57 +15,55 @@ internal static class ImportHook private static IntPtr py_clr_module; static BorrowedReference ClrModuleReference => new BorrowedReference(py_clr_module); - /// - /// Initialize just the __import__ hook itself. - /// - static void InitImport() - { - // We replace the built-in Python __import__ with our own: first - // look in CLR modules, then if we don't find any call the default - // Python __import__. - IntPtr builtins = Runtime.GetBuiltins(); - py_import = Runtime.PyObject_GetAttr(builtins, PyIdentifier.__import__); - PythonException.ThrowIfIsNull(py_import); - - hook = new MethodWrapper(typeof(ImportHook), "__import__", "TernaryFunc"); - int res = Runtime.PyObject_SetAttr(builtins, PyIdentifier.__import__, hook.ptr); - PythonException.ThrowIfIsNotZero(res); - - Runtime.XDecref(builtins); - } - - /// - /// Restore the __import__ hook. - /// - static void RestoreImport() - { - IntPtr builtins = Runtime.GetBuiltins(); - - IntPtr existing = Runtime.PyObject_GetAttr(builtins, PyIdentifier.__import__); - Runtime.XDecref(existing); - if (existing != hook.ptr) - { - throw new NotSupportedException("Unable to restore original __import__."); - } - - int res = Runtime.PyObject_SetAttr(builtins, PyIdentifier.__import__, py_import); - PythonException.ThrowIfIsNotZero(res); - Runtime.XDecref(py_import); - py_import = IntPtr.Zero; - - hook.Release(); - hook = null; - - Runtime.XDecref(builtins); - } + private const string LoaderCode = @" +import importlib.abc +import sys + +class DotNetLoader(importlib.abc.Loader): + + def __init__(self): + super(DotNetLoader, self).__init__() + + @classmethod + def exec_module(klass, mod): + # this method is needed to mark this + # loader as a non-legacy loader. + pass + + @classmethod + def create_module(klass, spec): + import clr + a = clr._LoadClrModule(spec) + #mod = getattr(clr, '__imported') + print(a) + #print(mod) + #print(mod is a) + #delattr(clr, '__imported') + return a + +class DotNetFinder(importlib.abc.MetaPathFinder): + + def __init__(self): + print('DotNetFinder init') + super(DotNetFinder, self).__init__() + + @classmethod + def find_spec(klass, fullname, paths=None, target=None): + import clr + # print(clr._availableNamespaces) + if (hasattr(clr, '_availableNamespaces') and fullname in clr._availableNamespaces): + #if (clr._NamespaceLoaded(fullname)): + return importlib.machinery.ModuleSpec(fullname, DotNetLoader(), is_package=True) + return None + +sys.meta_path.append(DotNetFinder()) + "; /// /// Initialization performed on startup of the Python runtime. /// internal static unsafe void Initialize() { - InitImport(); - // Initialize the clr module and tell Python about it. root = new CLRModule(); @@ -80,6 +78,9 @@ 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 + PythonEngine.Exec(LoaderCode); } @@ -93,8 +94,7 @@ internal static void Shutdown() return; } - RestoreImport(); - + bool shouldFreeDef = Runtime.Refcount(py_clr_module) == 1; Runtime.XDecref(py_clr_module); py_clr_module = IntPtr.Zero; @@ -115,7 +115,6 @@ internal static void SaveRuntimeData(RuntimeDataStorage storage) internal static void RestoreRuntimeData(RuntimeDataStorage storage) { - InitImport(); storage.GetValue("py_clr_module", out py_clr_module); var rootHandle = storage.GetValue("root"); root = (CLRModule)ManagedType.GetManagedObject(rootHandle); @@ -136,41 +135,6 @@ public static unsafe NewReference GetCLRModule(BorrowedReference fromList = defa Runtime.PyDict_Update(py_mod_dict, clr_dict); } - // find any items from the from list and get them from the root if they're not - // already in the module dictionary - if (fromList != null) - { - if (Runtime.PyTuple_Check(fromList)) - { - using var mod_dict = new PyDict(py_mod_dict); - using var from = new PyTuple(fromList); - foreach (PyObject item in from) - { - if (mod_dict.HasKey(item)) - { - continue; - } - - var s = item.AsManagedObject(typeof(string)) as string; - if (s == null) - { - continue; - } - - ManagedType attr = root.GetAttribute(s, true); - if (attr == null) - { - continue; - } - - Runtime.XIncref(attr.pyHandle); - using (var obj = new PyObject(attr.pyHandle)) - { - mod_dict.SetItem(s, obj); - } - } - } - } Runtime.XIncref(py_clr_module); return NewReference.DangerousFromPointer(py_clr_module); } @@ -178,124 +142,15 @@ public static unsafe NewReference GetCLRModule(BorrowedReference fromList = defa /// /// The actual import hook that ties Python to the managed world. /// - public static IntPtr __import__(IntPtr self, IntPtr argsRaw, IntPtr kw) + public static ModuleObject __import__(string modname) { - var args = new BorrowedReference(argsRaw); - - // Replacement for the builtin __import__. The original import - // hook is saved as this.py_import. This version handles CLR - // import and defers to the normal builtin for everything else. - - var num_args = Runtime.PyTuple_Size(args); - if (num_args < 1) - { - return Exceptions.RaiseTypeError("__import__() takes at least 1 argument (0 given)"); - } - - BorrowedReference py_mod_name = Runtime.PyTuple_GetItem(args, 0); - if (py_mod_name.IsNull || - !Runtime.IsStringType(py_mod_name)) - { - return Exceptions.RaiseTypeError("string expected"); - } - - // Check whether the import is of the form 'from x import y'. - // This determines whether we return the head or tail module. - - BorrowedReference fromList = default; - var fromlist = false; - if (num_args >= 4) - { - fromList = Runtime.PyTuple_GetItem(args, 3); - if (fromList != null && - Runtime.PyObject_IsTrue(fromList) == 1) - { - fromlist = true; - } - } - - string mod_name = Runtime.GetManagedString(py_mod_name); - // Check these BEFORE the built-in import runs; may as well - // do the Incref()ed return here, since we've already found - // the module. - if (mod_name == "clr") - { - NewReference clr_module = GetCLRModule(fromList); - if (!clr_module.IsNull()) - { - BorrowedReference sys_modules = Runtime.PyImport_GetModuleDict(); - if (!sys_modules.IsNull) - { - Runtime.PyDict_SetItemString(sys_modules, "clr", clr_module); - } - } - return clr_module.DangerousMoveToPointerOrNull(); - } - - string realname = mod_name; - - // 2010-08-15: Always seemed smart to let python try first... - // This shaves off a few tenths of a second on test_module.py - // and works around a quirk where 'sys' is found by the - // LoadImplicit() deprecation logic. - // Turns out that the AssemblyManager.ResolveHandler() checks to see if any - // Assembly's FullName.ToLower().StartsWith(name.ToLower()), which makes very - // little sense to me. - IntPtr res = Runtime.PyObject_Call(py_import, args.DangerousGetAddress(), kw); - if (res != IntPtr.Zero) - { - // There was no error. - if (fromlist && IsLoadAll(fromList)) - { - var mod = ManagedType.GetManagedObject(res) as ModuleObject; - mod?.LoadNames(); - } - return res; - } - // There was an error - if (!Exceptions.ExceptionMatches(Exceptions.ImportError)) - { - // and it was NOT an ImportError; bail out here. - return IntPtr.Zero; - } - - if (mod_name == string.Empty) - { - // Most likely a missing relative import. - // For example site-packages\bs4\builder\__init__.py uses it to check if a package exists: - // from . import _html5lib - // We don't support them anyway - return IntPtr.Zero; - } - // Save the exception - var originalException = PythonException.FetchCurrentRaw(); + // Console.WriteLine("Import hook"); + string realname = modname; string[] names = realname.Split('.'); - // See if sys.modules for this interpreter already has the - // requested module. If so, just return the existing module. - BorrowedReference modules = Runtime.PyImport_GetModuleDict(); - BorrowedReference module = Runtime.PyDict_GetItem(modules, py_mod_name); - - if (module != null) - { - if (fromlist) - { - if (IsLoadAll(fromList)) - { - var mod = ManagedType.GetManagedObject(module) as ModuleObject; - mod?.LoadNames(); - } - return new NewReference(module).DangerousMoveToPointer(); - } - - module = Runtime.PyDict_GetItemString(modules, names[0]); - return new NewReference(module, canBeNull: true).DangerousMoveToPointer(); - } - Exceptions.Clear(); - - // Traverse the qualified module name to get the named module - // and place references in sys.modules as we go. Note that if + // Traverse the qualified module name to get the named module. + // Note that if // we are running in interactive mode we pre-load the names in // each module, which is often useful for introspection. If we // are not interactive, we stick to just-in-time creation of @@ -304,17 +159,21 @@ public static IntPtr __import__(IntPtr self, IntPtr argsRaw, IntPtr kw) // enable preloading in a non-interactive python processing by // setting clr.preload = True - ModuleObject head = mod_name == realname ? null : root; + ModuleObject head = null; ModuleObject tail = root; root.InitializePreload(); + // root.LoadNames(); foreach (string name in names) { ManagedType mt = tail.GetAttribute(name, true); if (!(mt is ModuleObject)) { - originalException.Restore(); - return IntPtr.Zero; + // originalException.Restore(); + // Exceptions.SetError(Exceptions.ImportError, ""); + // throw new PythonException(); + // TODO: set exception + return null; } if (head == null) { @@ -325,21 +184,11 @@ public static IntPtr __import__(IntPtr self, IntPtr argsRaw, IntPtr kw) { tail.LoadNames(); } - - // Add the module to sys.modules - Runtime.PyDict_SetItemString(modules, tail.moduleName, tail.ObjectReference); } { - var mod = fromlist ? tail : head; - - if (fromlist && IsLoadAll(fromList)) - { - mod.LoadNames(); - } - - Runtime.XIncref(mod.pyHandle); - return mod.pyHandle; + Runtime.XIncref(tail.pyHandle); + return tail; } } diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index dfb6fdf55..dd4f7f625 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -20,6 +20,7 @@ internal class ModuleObject : ExtensionType internal IntPtr dict; internal BorrowedReference DictRef => new BorrowedReference(dict); protected string _namespace; + private IntPtr __all__ = IntPtr.Zero; public ModuleObject(string name) { @@ -181,7 +182,23 @@ public void LoadNames() { continue; } - GetAttribute(name, true); + + if(GetAttribute(name, true) != null) + { + // if it's a valid attribute, add it to __all__ + var pyname = Runtime.PyString_FromString(name); + try + { + if (Runtime.PyList_Append(new BorrowedReference(__all__), pyname) != 0) + { + throw new PythonException(); + } + } + finally + { + Runtime.XDecref(pyname); + } + } } } @@ -263,6 +280,13 @@ public static IntPtr tp_getattro(IntPtr ob, IntPtr key) return self.dict; } + if (name == "__all__") + { + self.LoadNames(); + Runtime.XIncref(self.__all__); + return self.__all__; + } + ManagedType attr = null; try @@ -320,6 +344,32 @@ protected override void Clear() base.Clear(); } + /// + /// Override the setattr implementation. + /// This is needed because the import mechanics need + /// to set a few attributes + /// + [ForbidPythonThreads] + public new static int tp_setattro(IntPtr ob, IntPtr key, IntPtr val) + { + var self = (ModuleObject)ManagedType.GetManagedObject(ob); + var dict = self.dict; + + var current = Runtime.PyDict_GetItem(dict, key); + if (current == val) + { + return 0; + } + else if (ManagedType.GetManagedObject(current) != null) + { + var message = "Can't override a .NET object"; + Exceptions.SetError(Exceptions.AttributeError, message); + return -1; + } + + return Runtime.PyDict_SetItem(dict, key, val); + } + protected override void OnSave(InterDomainContext context) { base.OnSave(context); @@ -526,5 +576,28 @@ public static string[] ListAssemblies(bool verbose) } return names; } + + [ModuleFunction] + public static int _AtExit() + { + return Runtime.AtExit(); + } + + + [ModuleFunction] + [ForbidPythonThreads] + public static PyObject _LoadClrModule(PyObject spec) + { + var mod = ImportHook.__import__(spec.GetAttr("name").ToString()); + if (mod == null) + { + // __import__ sets the exception.? + Console.WriteLine("NULL module"); + return Runtime.None; + } + // We can't return directly a ModuleObject, because the tpHandle is + // not set, but we can return a PyObject. + return new PyObject(mod.pyHandle); + } } } diff --git a/src/runtime/native/TypeOffset.cs b/src/runtime/native/TypeOffset.cs index a73a9ae43..d9c3ee52c 100644 --- a/src/runtime/native/TypeOffset.cs +++ b/src/runtime/native/TypeOffset.cs @@ -160,6 +160,7 @@ static void ValidateRequiredOffsetsPresent(PropertyInfo[] offsetProperties) "getPreload", "Initialize", "ListAssemblies", + "_LoadClrModule", "Release", "Reset", "set_SuppressDocs", From 279b53546d358582ac9b22ef12b5ad74ba35b064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Fri, 15 Jan 2021 15:27:48 -0500 Subject: [PATCH 02/20] Add the loaded namespaces tracking --- src/runtime/importhook.cs | 114 ++++++++++++++++++++++++++++++------ src/runtime/moduleobject.cs | 10 +--- 2 files changed, 97 insertions(+), 27 deletions(-) diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 249fdebbe..86c2cbac8 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Runtime.InteropServices; namespace Python.Runtime @@ -58,6 +57,7 @@ import clr sys.meta_path.append(DotNetFinder()) "; + const string availableNsKey = "_availableNamespaces"; /// /// Initialization performed on startup of the Python runtime. @@ -80,6 +80,7 @@ internal static unsafe void Initialize() Runtime.PyDict_SetItemString(dict, "clr", ClrModuleReference); // Add/create the MetaPathLoader + SetupNamespaceTracking(); PythonEngine.Exec(LoaderCode); } @@ -98,6 +99,7 @@ internal static void Shutdown() Runtime.XDecref(py_clr_module); py_clr_module = IntPtr.Zero; + TeardownNameSpaceTracking(); Runtime.XDecref(root.pyHandle); root = null; CLRModule.Reset(); @@ -118,12 +120,90 @@ internal static void RestoreRuntimeData(RuntimeDataStorage storage) storage.GetValue("py_clr_module", out py_clr_module); var rootHandle = storage.GetValue("root"); root = (CLRModule)ManagedType.GetManagedObject(rootHandle); + SetupNamespaceTracking(); } + static void SetupNamespaceTracking () + { + var newset = Runtime.PySet_New(IntPtr.Zero); + try + { + foreach (var ns in AssemblyManager.GetNamespaces()) + { + var pyNs = Runtime.PyString_FromString(ns); + try + { + if(Runtime.PySet_Add(newset, pyNs) != 0) + { + throw new PythonException(); + } + } + finally + { + Runtime.XDecref(pyNs); + } + } + + if(Runtime.PyDict_SetItemString(root.dict, availableNsKey, newset) != 0) + { + throw new PythonException(); + } + } + finally + { + Runtime.XDecref(newset); + } + + AssemblyManager.namespaceAdded += OnNamespaceAdded; + PythonEngine.AddShutdownHandler(()=>AssemblyManager.namespaceAdded -= OnNamespaceAdded); + } + + static void TeardownNameSpaceTracking() + { + AssemblyManager.namespaceAdded -= OnNamespaceAdded; + // If the C# runtime isn't loaded, then there is no namespaces available + if ((Runtime.PyDict_DelItemString(root.dict, availableNsKey) != 0) && + (Exceptions.ExceptionMatches(Exceptions.KeyError))) + { + // Trying to remove a key that's not in the dictionary + // raises an error. We don't care about it. + Runtime.PyErr_Clear(); + } + else if (Exceptions.ErrorOccurred()) + { + throw new PythonException(); + } + } + + static void OnNamespaceAdded (string name) + { + using(Py.GIL()) + { + var pyNs = Runtime.PyString_FromString(name); + try + { + var nsSet = Runtime.PyDict_GetItemString(root.dict, availableNsKey); + if (nsSet != IntPtr.Zero) + { + if(Runtime.PySet_Add(nsSet, pyNs) != 0) + { + throw new PythonException(); + } + } + } + finally + { + Runtime.XDecref(pyNs); + } + } + } + + /// - /// Return the clr python module (new reference) + /// Because we use a proxy module for the clr module, we somtimes need + /// to force the py_clr_module to sync with the actual clr module's dict. /// - public static unsafe NewReference GetCLRModule(BorrowedReference fromList = default) + internal static void UpdateCLRModuleDict() { root.InitializePreload(); @@ -134,21 +214,23 @@ public static unsafe NewReference GetCLRModule(BorrowedReference fromList = defa { Runtime.PyDict_Update(py_mod_dict, clr_dict); } + } + /// + /// Return the clr python module (new reference) + /// + public static unsafe NewReference GetCLRModule(BorrowedReference fromList = default) + { + UpdateCLRModuleDict(); Runtime.XIncref(py_clr_module); return NewReference.DangerousFromPointer(py_clr_module); } /// - /// The actual import hook that ties Python to the managed world. + /// The hook to import a CLR module into Python /// public static ModuleObject __import__(string modname) { - // Console.WriteLine("Import hook"); - - string realname = modname; - string[] names = realname.Split('.'); - // Traverse the qualified module name to get the named module. // Note that if // we are running in interactive mode we pre-load the names in @@ -162,18 +244,15 @@ public static ModuleObject __import__(string modname) ModuleObject head = null; ModuleObject tail = root; root.InitializePreload(); - // root.LoadNames(); + string[] names = modname.Split('.'); foreach (string name in names) { ManagedType mt = tail.GetAttribute(name, true); if (!(mt is ModuleObject)) { - // originalException.Restore(); - // Exceptions.SetError(Exceptions.ImportError, ""); - // throw new PythonException(); - // TODO: set exception - return null; + Exceptions.SetError(Exceptions.ImportError, $"'{name}' Is not a ModuleObject."); + throw new PythonException(); } if (head == null) { @@ -186,10 +265,7 @@ public static ModuleObject __import__(string modname) } } - { - Runtime.XIncref(tail.pyHandle); - return tail; - } + return tail; } private static bool IsLoadAll(BorrowedReference fromList) diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index dd4f7f625..1b80390a5 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -529,7 +529,8 @@ public static Assembly AddReference(string name) { throw new FileNotFoundException($"Unable to find assembly '{name}'."); } - + // Classes that are not in a namespace needs an extra nudge to be found. + ImportHook.UpdateCLRModuleDict(); return assembly; } @@ -583,18 +584,11 @@ public static int _AtExit() return Runtime.AtExit(); } - [ModuleFunction] [ForbidPythonThreads] public static PyObject _LoadClrModule(PyObject spec) { var mod = ImportHook.__import__(spec.GetAttr("name").ToString()); - if (mod == null) - { - // __import__ sets the exception.? - Console.WriteLine("NULL module"); - return Runtime.None; - } // We can't return directly a ModuleObject, because the tpHandle is // not set, but we can return a PyObject. return new PyObject(mod.pyHandle); From f92e95b39dc67979dcc17497edfe99873a17bbce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Fri, 15 Jan 2021 15:45:46 -0500 Subject: [PATCH 03/20] cleanup and changelog entry --- CHANGELOG.md | 2 ++ src/runtime/importhook.cs | 15 ++------------- src/runtime/moduleobject.cs | 6 +++++- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5871e7ffb..326c229ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ One must now either use enum members (e.g. `MyEnum.Option`), or use enum constru - .NET and Python exceptions are preserved when crossing Python/.NET boundary - BREAKING: custom encoders are no longer called for instances of `System.Type` - `PythonException.Restore` no longer clears `PythonException` instance. +- Replaced the old `__import__` hook hack with a PEP302-style Meta Path Loader ### Fixed @@ -72,6 +73,7 @@ One must now either use enum members (e.g. `MyEnum.Option`), or use enum constru - Providing an invalid type parameter to a generic type or method produces a helpful Python error - Empty parameter names (as can be generated from F#) do not cause crashes + ### Removed - implicit assembly loading (you have to explicitly `clr.AddReference` before doing import) diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 86c2cbac8..02c9dc364 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -8,9 +8,7 @@ namespace Python.Runtime /// internal static class ImportHook { - private static IntPtr py_import; private static CLRModule root; - private static MethodWrapper hook; private static IntPtr py_clr_module; static BorrowedReference ClrModuleReference => new BorrowedReference(py_clr_module); @@ -25,20 +23,13 @@ def __init__(self): @classmethod def exec_module(klass, mod): - # this method is needed to mark this - # loader as a non-legacy loader. + # This method needs to exist. pass @classmethod def create_module(klass, spec): import clr - a = clr._LoadClrModule(spec) - #mod = getattr(clr, '__imported') - print(a) - #print(mod) - #print(mod is a) - #delattr(clr, '__imported') - return a + return clr._LoadClrModule(spec) class DotNetFinder(importlib.abc.MetaPathFinder): @@ -49,9 +40,7 @@ def __init__(self): @classmethod def find_spec(klass, fullname, paths=None, target=None): import clr - # print(clr._availableNamespaces) if (hasattr(clr, '_availableNamespaces') and fullname in clr._availableNamespaces): - #if (clr._NamespaceLoaded(fullname)): return importlib.machinery.ModuleSpec(fullname, DotNetLoader(), is_package=True) return None diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 1b80390a5..a319867d6 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -588,7 +588,11 @@ public static int _AtExit() [ForbidPythonThreads] public static PyObject _LoadClrModule(PyObject spec) { - var mod = ImportHook.__import__(spec.GetAttr("name").ToString()); + 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); From afffc187515fa7c8e725f2e0cc592cc04966ef7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Mon, 18 Jan 2021 16:17:25 -0500 Subject: [PATCH 04/20] Fix a bug where clr wasn't in sys.modules after reload --- src/runtime/importhook.cs | 3 ++ tests/domain_tests/TestRunner.cs | 40 ++++++++++++++++++++++++ tests/domain_tests/test_domain_reload.py | 4 +++ 3 files changed, 47 insertions(+) diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 02c9dc364..5af702217 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -109,6 +109,9 @@ internal static void RestoreRuntimeData(RuntimeDataStorage storage) storage.GetValue("py_clr_module", out py_clr_module); var rootHandle = storage.GetValue("root"); root = (CLRModule)ManagedType.GetManagedObject(rootHandle); + IntPtr dict = Runtime.PyImport_GetModuleDict(); + Runtime.PyDict_SetItemString(dict, "CLR", py_clr_module); + Runtime.PyDict_SetItemString(dict, "clr", py_clr_module); SetupNamespaceTracking(); } diff --git a/tests/domain_tests/TestRunner.cs b/tests/domain_tests/TestRunner.cs index a21297829..83ae1654d 100644 --- a/tests/domain_tests/TestRunner.cs +++ b/tests/domain_tests/TestRunner.cs @@ -1092,6 +1092,46 @@ assert sys.my_obj is not None foo = sys.my_obj.Inner() print(foo) + ", + }, + new TestCase + { + // The C# code for this test doesn't matter. + Name = "import_after_reload", + DotNetBefore = @" + namespace TestNamespace + { + [System.Serializable] + public class Cls + { + } + }", + DotNetAfter = @" + namespace TestNamespace + { + [System.Serializable] + public class WithNestedType + { + [System.Serializable] + public class Cls + { + } + } + }", + PythonCode = @" +import sys + +def before_reload(): + import clr + import System + + +def after_reload(): + assert 'System' in sys.modules + assert 'clr' in sys.modules + import clr + import System + ", }, }; diff --git a/tests/domain_tests/test_domain_reload.py b/tests/domain_tests/test_domain_reload.py index a7cd2fa4d..71b1e348e 100644 --- a/tests/domain_tests/test_domain_reload.py +++ b/tests/domain_tests/test_domain_reload.py @@ -88,3 +88,7 @@ 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") \ No newline at end of file From d821c0f7094f92508babc4dc7168f133f12894cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Tue, 19 Jan 2021 11:38:29 -0500 Subject: [PATCH 05/20] Further refinements to setattr logic on ModuleObjects --- src/runtime/moduleobject.cs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index a319867d6..eadfeadfb 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -22,6 +22,11 @@ internal class ModuleObject : ExtensionType protected string _namespace; private IntPtr __all__ = IntPtr.Zero; + // Attributes to be set on the module according to PEP302 and 451 + // by the import machinery. + static readonly HashSet settableAttributes = + new HashSet {"__spec__", "__file__", "__name__", "__path__", "__loader__", "__package__"}; + public ModuleObject(string name) { if (name == string.Empty) @@ -352,22 +357,15 @@ protected override void Clear() [ForbidPythonThreads] public new static int tp_setattro(IntPtr ob, IntPtr key, IntPtr val) { - var self = (ModuleObject)ManagedType.GetManagedObject(ob); - var dict = self.dict; - - var current = Runtime.PyDict_GetItem(dict, key); - if (current == val) - { - return 0; - } - else if (ManagedType.GetManagedObject(current) != null) + var managedKey = Runtime.GetManagedString(key); + if ((settableAttributes.Contains(managedKey)) || + (ManagedType.GetManagedObject(val)?.GetType() == typeof(ModuleObject)) ) { - var message = "Can't override a .NET object"; - Exceptions.SetError(Exceptions.AttributeError, message); - return -1; + var self = (ModuleObject)ManagedType.GetManagedObject(ob); + return Runtime.PyDict_SetItem(self.dict, key, val); } - return Runtime.PyDict_SetItem(dict, key, val); + return ExtensionType.tp_setattro(ob, key, val); } protected override void OnSave(InterDomainContext context) From e469a8a73cfc0870cb56ba0368cff44c3938dd24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Wed, 20 Jan 2021 13:26:23 -0500 Subject: [PATCH 06/20] fixups, add docs --- CHANGELOG.md | 1 - src/runtime/importhook.cs | 11 ++++++++++- src/runtime/moduleobject.cs | 9 +++++++++ tests/domain_tests/TestRunner.cs | 25 ++++--------------------- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 326c229ea..f72016742 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,7 +73,6 @@ One must now either use enum members (e.g. `MyEnum.Option`), or use enum constru - Providing an invalid type parameter to a generic type or method produces a helpful Python error - Empty parameter names (as can be generated from F#) do not cause crashes - ### Removed - implicit assembly loading (you have to explicitly `clr.AddReference` before doing import) diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 5af702217..335ff1af8 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -34,7 +34,6 @@ import clr class DotNetFinder(importlib.abc.MetaPathFinder): def __init__(self): - print('DotNetFinder init') super(DotNetFinder, self).__init__() @classmethod @@ -115,6 +114,12 @@ internal static void RestoreRuntimeData(RuntimeDataStorage storage) SetupNamespaceTracking(); } + /// + /// Sets up the tracking of loaded namespaces. This makes available to + /// Python, as a Python object, the loaded namespaces. The set of loaded + /// namespaces is used during the import to verify if we can import a + /// CLR assembly as a module or not. The set is stored on the clr module. + /// static void SetupNamespaceTracking () { var newset = Runtime.PySet_New(IntPtr.Zero); @@ -150,6 +155,10 @@ static void SetupNamespaceTracking () PythonEngine.AddShutdownHandler(()=>AssemblyManager.namespaceAdded -= OnNamespaceAdded); } + /// + /// Removes the set of available namespaces from the clr module and + /// removes the callback on the OnNamespaceAdded event. + /// static void TeardownNameSpaceTracking() { AssemblyManager.namespaceAdded -= OnNamespaceAdded; diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index eadfeadfb..9eb1d997d 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -582,6 +582,15 @@ public static int _AtExit() return Runtime.AtExit(); } + + /// + /// Note: This should *not* be called directly. + /// The function that get/import a CLR assembly as a python module. + /// This function should only be called by the import machinery as seen + /// in importhook.cs + /// + /// A ModuleSpec Python object + /// A new reference to the imported module, as a PyObject. [ModuleFunction] [ForbidPythonThreads] public static PyObject _LoadClrModule(PyObject spec) diff --git a/tests/domain_tests/TestRunner.cs b/tests/domain_tests/TestRunner.cs index 83ae1654d..716fe079b 100644 --- a/tests/domain_tests/TestRunner.cs +++ b/tests/domain_tests/TestRunner.cs @@ -1096,28 +1096,11 @@ assert sys.my_obj is not None }, new TestCase { - // The C# code for this test doesn't matter. + // The C# code for this test doesn't matter; we're testing + // that the import hook behaves properly after a domain reload Name = "import_after_reload", - DotNetBefore = @" - namespace TestNamespace - { - [System.Serializable] - public class Cls - { - } - }", - DotNetAfter = @" - namespace TestNamespace - { - [System.Serializable] - public class WithNestedType - { - [System.Serializable] - public class Cls - { - } - } - }", + DotNetBefore = "", + DotNetAfter = "", PythonCode = @" import sys From 685b972001d2186e60d41f7a42959f74e6e8a5e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Thu, 4 Mar 2021 13:17:41 -0500 Subject: [PATCH 07/20] merge fixup --- src/embed_tests/TestPythonException.cs | 1 + src/runtime/importhook.cs | 35 +++++++++++--------------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/embed_tests/TestPythonException.cs b/src/embed_tests/TestPythonException.cs index 0763bfb34..a4b28906c 100644 --- a/src/embed_tests/TestPythonException.cs +++ b/src/embed_tests/TestPythonException.cs @@ -178,6 +178,7 @@ public void TestPythonException_Normalize_ThrowsWhenErrorSet() var pythonException = PythonException.FetchCurrentRaw(); Exceptions.SetError(Exceptions.TypeError, "Another error"); Assert.Throws(() => pythonException.Normalize()); + Exceptions.Clear(); } } } diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 335ff1af8..948af56e0 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -108,9 +108,9 @@ internal static void RestoreRuntimeData(RuntimeDataStorage storage) storage.GetValue("py_clr_module", out py_clr_module); var rootHandle = storage.GetValue("root"); root = (CLRModule)ManagedType.GetManagedObject(rootHandle); - IntPtr dict = Runtime.PyImport_GetModuleDict(); - Runtime.PyDict_SetItemString(dict, "CLR", py_clr_module); - Runtime.PyDict_SetItemString(dict, "clr", py_clr_module); + BorrowedReference dict = Runtime.PyImport_GetModuleDict(); + Runtime.PyDict_SetItemString(dict.DangerousGetAddress(), "CLR", py_clr_module); + Runtime.PyDict_SetItemString(dict.DangerousGetAddress(), "clr", py_clr_module); SetupNamespaceTracking(); } @@ -122,7 +122,7 @@ internal static void RestoreRuntimeData(RuntimeDataStorage storage) /// static void SetupNamespaceTracking () { - var newset = Runtime.PySet_New(IntPtr.Zero); + var newset = Runtime.PySet_New(new BorrowedReference(IntPtr.Zero)); try { foreach (var ns in AssemblyManager.GetNamespaces()) @@ -130,7 +130,7 @@ static void SetupNamespaceTracking () var pyNs = Runtime.PyString_FromString(ns); try { - if(Runtime.PySet_Add(newset, pyNs) != 0) + if(Runtime.PySet_Add(newset, new BorrowedReference(pyNs)) != 0) { throw new PythonException(); } @@ -141,14 +141,14 @@ static void SetupNamespaceTracking () } } - if(Runtime.PyDict_SetItemString(root.dict, availableNsKey, newset) != 0) + if(Runtime.PyDict_SetItemString(root.dict, availableNsKey, newset.DangerousGetAddress()) != 0) { throw new PythonException(); } } finally { - Runtime.XDecref(newset); + newset.Dispose(); } AssemblyManager.namespaceAdded += OnNamespaceAdded; @@ -163,17 +163,13 @@ static void TeardownNameSpaceTracking() { AssemblyManager.namespaceAdded -= OnNamespaceAdded; // If the C# runtime isn't loaded, then there is no namespaces available - if ((Runtime.PyDict_DelItemString(root.dict, availableNsKey) != 0) && + if ((Runtime.PyDict_DelItemString(new BorrowedReference(root.dict), availableNsKey) != 0) && (Exceptions.ExceptionMatches(Exceptions.KeyError))) { // Trying to remove a key that's not in the dictionary // raises an error. We don't care about it. Runtime.PyErr_Clear(); } - else if (Exceptions.ErrorOccurred()) - { - throw new PythonException(); - } } static void OnNamespaceAdded (string name) @@ -183,10 +179,10 @@ static void OnNamespaceAdded (string name) var pyNs = Runtime.PyString_FromString(name); try { - var nsSet = Runtime.PyDict_GetItemString(root.dict, availableNsKey); - if (nsSet != IntPtr.Zero) + var nsSet = Runtime.PyDict_GetItemString(new BorrowedReference(root.dict), availableNsKey); + if (!nsSet.IsNull) { - if(Runtime.PySet_Add(nsSet, pyNs) != 0) + if(Runtime.PySet_Add(nsSet, new BorrowedReference(pyNs)) != 0) { throw new PythonException(); } @@ -211,16 +207,15 @@ internal static void UpdateCLRModuleDict() // update the module dictionary with the contents of the root dictionary root.LoadNames(); BorrowedReference py_mod_dict = Runtime.PyModule_GetDict(ClrModuleReference); - using (var clr_dict = Runtime.PyObject_GenericGetDict(root.ObjectReference)) - { - Runtime.PyDict_Update(py_mod_dict, clr_dict); - } + using var clr_dict = Runtime.PyObject_GenericGetDict(root.ObjectReference); + + Runtime.PyDict_Update(py_mod_dict, clr_dict); } /// /// Return the clr python module (new reference) /// - public static unsafe NewReference GetCLRModule(BorrowedReference fromList = default) + public static unsafe NewReference GetCLRModule() { UpdateCLRModuleDict(); Runtime.XIncref(py_clr_module); From be8136448e1220f2a3ea93f7243c2f4a8eaa6237 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Wed, 10 Mar 2021 12:57:39 -0500 Subject: [PATCH 08/20] (WIP) import hook in the pytohn module --- pythonnet/util/__init__.py | 2 +- pythonnet/util/import_hook.py | 33 +++++++++++++++++++++++++++++++++ src/runtime/importhook.cs | 4 ++-- src/runtime/moduleobject.cs | 2 +- src/runtime/runtime.cs | 20 ++++++++++---------- 5 files changed, 47 insertions(+), 14 deletions(-) create mode 100644 pythonnet/util/import_hook.py diff --git a/pythonnet/util/__init__.py b/pythonnet/util/__init__.py index 75d4bad8c..e78fbafe6 100644 --- a/pythonnet/util/__init__.py +++ b/pythonnet/util/__init__.py @@ -1 +1 @@ -from .find_libpython import find_libpython +from ..find_libpython import find_libpython diff --git a/pythonnet/util/import_hook.py b/pythonnet/util/import_hook.py new file mode 100644 index 000000000..fc8692cad --- /dev/null +++ b/pythonnet/util/import_hook.py @@ -0,0 +1,33 @@ +import importlib.abc +import sys + +class DotNetLoader(importlib.abc.Loader): + + def __init__(self): + super(DotNetLoader, self).__init__() + + @classmethod + def exec_module(klass, mod): + # This method needs to exist. + pass + + @classmethod + def create_module(klass, spec): + import clr + return clr._LoadClrModule(spec) + +class DotNetFinder(importlib.abc.MetaPathFinder): + + def __init__(self): + super(DotNetFinder, self).__init__() + + @classmethod + def find_spec(klass, fullname, paths=None, target=None): + import clr + if (hasattr(clr, '_availableNamespaces') and fullname in clr._availableNamespaces): + return importlib.machinery.ModuleSpec(fullname, DotNetLoader(), is_package=True) + return None + + +def init_import_hook(): + sys.meta_path.append(DotNetFinder()) \ No newline at end of file diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 948af56e0..4d776123c 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -69,7 +69,7 @@ internal static unsafe void Initialize() // Add/create the MetaPathLoader SetupNamespaceTracking(); - PythonEngine.Exec(LoaderCode); + PythonEngine.Exec("import pythonnet.util.import_hook;pythonnet.util.import_hook.init_import_hook()"); } @@ -225,7 +225,7 @@ public static unsafe NewReference GetCLRModule() /// /// The hook to import a CLR module into Python /// - public static ModuleObject __import__(string modname) + public static ModuleObject Import(string modname) { // Traverse the qualified module name to get the named module. // Note that if diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 9eb1d997d..e710b8ba9 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -598,7 +598,7 @@ public static PyObject _LoadClrModule(PyObject spec) ModuleObject mod = null; using (var modname = spec.GetAttr("name")) { - mod = ImportHook.__import__(modname.ToString()); + mod = ImportHook.Import(modname.ToString()); } // We can't return directly a ModuleObject, because the tpHandle is // not set, but we can return a PyObject. diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 789b71f3e..9f8c87030 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -153,6 +153,16 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd ClassDerivedObject.Reset(); TypeManager.Initialize(); + // Need to add the runtime directory to sys.path so that we + // can find built-in assemblies like System.Data, et. al. + string rtdir = RuntimeEnvironment.GetRuntimeDirectory(); + IntPtr path = PySys_GetObject("path").DangerousGetAddress(); + IntPtr item = PyString_FromString(rtdir); + if (PySequence_Contains(path, item) == 0) + { + PyList_Append(new BorrowedReference(path), item); + } + XDecref(item); // Initialize modules that depend on the runtime class. AssemblyManager.Initialize(); OperatorMethod.Initialize(); @@ -167,16 +177,6 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd } Exceptions.Initialize(); - // Need to add the runtime directory to sys.path so that we - // can find built-in assemblies like System.Data, et. al. - string rtdir = RuntimeEnvironment.GetRuntimeDirectory(); - IntPtr path = PySys_GetObject("path").DangerousGetAddress(); - IntPtr item = PyString_FromString(rtdir); - if (PySequence_Contains(path, item) == 0) - { - PyList_Append(new BorrowedReference(path), item); - } - XDecref(item); AssemblyManager.UpdatePath(); } From 73958ed239b21cc71c0206c3e5ffcc45ed7aa182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Mon, 12 Apr 2021 11:14:04 -0400 Subject: [PATCH 09/20] Revert "(WIP) import hook in the pytohn module" This reverts commit 4684523c076f9815203ae26ff8a6418158315fc3. --- pythonnet/util/__init__.py | 2 +- pythonnet/util/import_hook.py | 33 --------------------------------- src/runtime/importhook.cs | 4 ++-- src/runtime/moduleobject.cs | 2 +- src/runtime/runtime.cs | 20 ++++++++++---------- 5 files changed, 14 insertions(+), 47 deletions(-) delete mode 100644 pythonnet/util/import_hook.py diff --git a/pythonnet/util/__init__.py b/pythonnet/util/__init__.py index e78fbafe6..75d4bad8c 100644 --- a/pythonnet/util/__init__.py +++ b/pythonnet/util/__init__.py @@ -1 +1 @@ -from ..find_libpython import find_libpython +from .find_libpython import find_libpython diff --git a/pythonnet/util/import_hook.py b/pythonnet/util/import_hook.py deleted file mode 100644 index fc8692cad..000000000 --- a/pythonnet/util/import_hook.py +++ /dev/null @@ -1,33 +0,0 @@ -import importlib.abc -import sys - -class DotNetLoader(importlib.abc.Loader): - - def __init__(self): - super(DotNetLoader, self).__init__() - - @classmethod - def exec_module(klass, mod): - # This method needs to exist. - pass - - @classmethod - def create_module(klass, spec): - import clr - return clr._LoadClrModule(spec) - -class DotNetFinder(importlib.abc.MetaPathFinder): - - def __init__(self): - super(DotNetFinder, self).__init__() - - @classmethod - def find_spec(klass, fullname, paths=None, target=None): - import clr - if (hasattr(clr, '_availableNamespaces') and fullname in clr._availableNamespaces): - return importlib.machinery.ModuleSpec(fullname, DotNetLoader(), is_package=True) - return None - - -def init_import_hook(): - sys.meta_path.append(DotNetFinder()) \ No newline at end of file diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 4d776123c..948af56e0 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -69,7 +69,7 @@ internal static unsafe void Initialize() // Add/create the MetaPathLoader SetupNamespaceTracking(); - PythonEngine.Exec("import pythonnet.util.import_hook;pythonnet.util.import_hook.init_import_hook()"); + PythonEngine.Exec(LoaderCode); } @@ -225,7 +225,7 @@ public static unsafe NewReference GetCLRModule() /// /// The hook to import a CLR module into Python /// - public static ModuleObject Import(string modname) + public static ModuleObject __import__(string modname) { // Traverse the qualified module name to get the named module. // Note that if diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index e710b8ba9..9eb1d997d 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -598,7 +598,7 @@ public static PyObject _LoadClrModule(PyObject spec) ModuleObject mod = null; using (var modname = spec.GetAttr("name")) { - mod = ImportHook.Import(modname.ToString()); + mod = ImportHook.__import__(modname.ToString()); } // We can't return directly a ModuleObject, because the tpHandle is // not set, but we can return a PyObject. diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 9f8c87030..789b71f3e 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -153,16 +153,6 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd ClassDerivedObject.Reset(); TypeManager.Initialize(); - // Need to add the runtime directory to sys.path so that we - // can find built-in assemblies like System.Data, et. al. - string rtdir = RuntimeEnvironment.GetRuntimeDirectory(); - IntPtr path = PySys_GetObject("path").DangerousGetAddress(); - IntPtr item = PyString_FromString(rtdir); - if (PySequence_Contains(path, item) == 0) - { - PyList_Append(new BorrowedReference(path), item); - } - XDecref(item); // Initialize modules that depend on the runtime class. AssemblyManager.Initialize(); OperatorMethod.Initialize(); @@ -177,6 +167,16 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd } Exceptions.Initialize(); + // Need to add the runtime directory to sys.path so that we + // can find built-in assemblies like System.Data, et. al. + string rtdir = RuntimeEnvironment.GetRuntimeDirectory(); + IntPtr path = PySys_GetObject("path").DangerousGetAddress(); + IntPtr item = PyString_FromString(rtdir); + if (PySequence_Contains(path, item) == 0) + { + PyList_Append(new BorrowedReference(path), item); + } + XDecref(item); AssemblyManager.UpdatePath(); } From e71a0ef90414bd2cde21901ed3956251fccc4753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Mon, 12 Apr 2021 15:39:12 -0400 Subject: [PATCH 10/20] Import hook as a module, take 2 --- src/runtime/importhook.cs | 93 +++++++++++++++++++++++--------- src/runtime/moduleobject.cs | 4 +- src/runtime/native/TypeOffset.cs | 2 +- 3 files changed, 72 insertions(+), 27 deletions(-) diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 948af56e0..df1ae253e 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -19,7 +19,7 @@ import sys class DotNetLoader(importlib.abc.Loader): def __init__(self): - super(DotNetLoader, self).__init__() + super().__init__() @classmethod def exec_module(klass, mod): @@ -29,21 +29,19 @@ def exec_module(klass, mod): @classmethod def create_module(klass, spec): import clr - return clr._LoadClrModule(spec) + return clr._load_clr_module(spec) class DotNetFinder(importlib.abc.MetaPathFinder): def __init__(self): - super(DotNetFinder, self).__init__() + super().__init__() @classmethod def find_spec(klass, fullname, paths=None, target=None): import clr - if (hasattr(clr, '_availableNamespaces') and fullname in clr._availableNamespaces): + if clr._availableNamespaces and fullname in clr._availableNamespaces: return importlib.machinery.ModuleSpec(fullname, DotNetLoader(), is_package=True) return None - -sys.meta_path.append(DotNetFinder()) "; const string availableNsKey = "_availableNamespaces"; @@ -69,7 +67,7 @@ internal static unsafe void Initialize() // Add/create the MetaPathLoader SetupNamespaceTracking(); - PythonEngine.Exec(LoaderCode); + SetupImportHook(); } @@ -114,13 +112,66 @@ internal static void RestoreRuntimeData(RuntimeDataStorage storage) 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); + + // 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); + Runtime.PyTuple_SetItem(args.DangerousGetAddress(), 0, codeStr); + // PyTuple_SetItem steals a reference, mod_dict is borrowed. + Runtime.XIncref(mod_dict.DangerousGetAddress()); + Runtime.PyTuple_SetItem(args.DangerousGetAddress(), 1, mod_dict.DangerousGetAddress()); + Runtime.PyObject_Call(exec.DangerousGetAddress(), args.DangerousGetAddress(), IntPtr.Zero); + + 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) + { + Runtime.XDecref(import_hook_module); + 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 metapath = Runtime.PySys_GetObject("meta_path"); + Runtime.PyList_Append(metapath, finder); + } + /// /// Sets up the tracking of loaded namespaces. This makes available to /// Python, as a Python object, the loaded namespaces. The set of loaded /// namespaces is used during the import to verify if we can import a /// CLR assembly as a module or not. The set is stored on the clr module. /// - static void SetupNamespaceTracking () + static void SetupNamespaceTracking() { var newset = Runtime.PySet_New(new BorrowedReference(IntPtr.Zero)); try @@ -130,7 +181,7 @@ static void SetupNamespaceTracking () var pyNs = Runtime.PyString_FromString(ns); try { - if(Runtime.PySet_Add(newset, new BorrowedReference(pyNs)) != 0) + if (Runtime.PySet_Add(newset, new BorrowedReference(pyNs)) != 0) { throw new PythonException(); } @@ -141,7 +192,7 @@ static void SetupNamespaceTracking () } } - if(Runtime.PyDict_SetItemString(root.dict, availableNsKey, newset.DangerousGetAddress()) != 0) + if (Runtime.PyDict_SetItemString(root.dict, availableNsKey, newset.DangerousGetAddress()) != 0) { throw new PythonException(); } @@ -152,7 +203,7 @@ static void SetupNamespaceTracking () } AssemblyManager.namespaceAdded += OnNamespaceAdded; - PythonEngine.AddShutdownHandler(()=>AssemblyManager.namespaceAdded -= OnNamespaceAdded); + PythonEngine.AddShutdownHandler(() => AssemblyManager.namespaceAdded -= OnNamespaceAdded); } /// @@ -162,27 +213,21 @@ static void SetupNamespaceTracking () static void TeardownNameSpaceTracking() { AssemblyManager.namespaceAdded -= OnNamespaceAdded; - // If the C# runtime isn't loaded, then there is no namespaces available - if ((Runtime.PyDict_DelItemString(new BorrowedReference(root.dict), availableNsKey) != 0) && - (Exceptions.ExceptionMatches(Exceptions.KeyError))) - { - // Trying to remove a key that's not in the dictionary - // raises an error. We don't care about it. - Runtime.PyErr_Clear(); - } + // If the C# runtime isn't loaded, then there are no namespaces available + Runtime.PyDict_SetItemString(root.dict, availableNsKey, Runtime.PyNone); } - static void OnNamespaceAdded (string name) + static void OnNamespaceAdded(string name) { - using(Py.GIL()) + using (Py.GIL()) { var pyNs = Runtime.PyString_FromString(name); try { var nsSet = Runtime.PyDict_GetItemString(new BorrowedReference(root.dict), availableNsKey); - if (!nsSet.IsNull) + if (!nsSet.IsNull || nsSet.DangerousGetAddress() != Runtime.PyNone) { - if(Runtime.PySet_Add(nsSet, new BorrowedReference(pyNs)) != 0) + if (Runtime.PySet_Add(nsSet, new BorrowedReference(pyNs)) != 0) { throw new PythonException(); } @@ -225,7 +270,7 @@ public static unsafe NewReference GetCLRModule() /// /// The hook to import a CLR module into Python /// - public static ModuleObject __import__(string modname) + public static ModuleObject Import(string modname) { // Traverse the qualified module name to get the named module. // Note that if diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 9eb1d997d..19178fe41 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -593,12 +593,12 @@ public static int _AtExit() /// A new reference to the imported module, as a PyObject. [ModuleFunction] [ForbidPythonThreads] - public static PyObject _LoadClrModule(PyObject spec) + public static PyObject _load_clr_module(PyObject spec) { ModuleObject mod = null; using (var modname = spec.GetAttr("name")) { - mod = ImportHook.__import__(modname.ToString()); + mod = ImportHook.Import(modname.ToString()); } // We can't return directly a ModuleObject, because the tpHandle is // not set, but we can return a PyObject. diff --git a/src/runtime/native/TypeOffset.cs b/src/runtime/native/TypeOffset.cs index d9c3ee52c..6e6da2d93 100644 --- a/src/runtime/native/TypeOffset.cs +++ b/src/runtime/native/TypeOffset.cs @@ -160,7 +160,7 @@ static void ValidateRequiredOffsetsPresent(PropertyInfo[] offsetProperties) "getPreload", "Initialize", "ListAssemblies", - "_LoadClrModule", + "_load_clr_module", "Release", "Reset", "set_SuppressDocs", From 2af066db6da89e3cad1119b9d4408c24f533e6ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Mon, 12 Apr 2021 15:46:59 -0400 Subject: [PATCH 11/20] fixup! Merge remote-tracking branch 'origin/master' into modernize-import-hook --- src/runtime/importhook.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index df1ae253e..4eaf3b97e 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -39,11 +39,11 @@ def __init__(self): @classmethod def find_spec(klass, fullname, paths=None, target=None): import clr - if clr._availableNamespaces and fullname in clr._availableNamespaces: + if clr._available_namespaces and fullname in clr._available_namespaces: return importlib.machinery.ModuleSpec(fullname, DotNetLoader(), is_package=True) return None "; - const string availableNsKey = "_availableNamespaces"; + const string availableNsKey = "_available_namespaces"; /// /// Initialization performed on startup of the Python runtime. From bb490bf5f324f6f63f68bfb5cdb231f1d9934e63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Mon, 12 Apr 2021 15:53:01 -0400 Subject: [PATCH 12/20] fixup! fixup! Merge remote-tracking branch 'origin/master' into modernize-import-hook --- src/runtime/runtime.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 789b71f3e..c94531c60 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -1944,6 +1944,11 @@ 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) + { + using var namePtr = new StrPtr(name, Encoding.UTF8); + return Delegates.PyModule_AddObject(module, namePtr, stolenObject); + } /// /// Return value: New reference. @@ -2502,6 +2507,7 @@ static Delegates() { PyModule_Create2 = (delegate* unmanaged[Cdecl])GetFunctionByName("PyModule_Create2TraceRefs", GetUnmanagedDll(_PythonDll)); } + PyModule_AddObject = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyModule_AddObject), GetUnmanagedDll(_PythonDll)); PyImport_Import = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyImport_Import), GetUnmanagedDll(_PythonDll)); PyImport_ImportModule = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyImport_ImportModule), GetUnmanagedDll(_PythonDll)); PyImport_ReloadModule = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyImport_ReloadModule), GetUnmanagedDll(_PythonDll)); @@ -2791,6 +2797,7 @@ static Delegates() internal static delegate* unmanaged[Cdecl] PyModule_GetDict { get; } internal static delegate* unmanaged[Cdecl] PyModule_GetFilename { get; } internal static delegate* unmanaged[Cdecl] PyModule_Create2 { get; } + internal static delegate* unmanaged[Cdecl] PyModule_AddObject { get; } internal static delegate* unmanaged[Cdecl] PyImport_Import { get; } internal static delegate* unmanaged[Cdecl] PyImport_ImportModule { get; } internal static delegate* unmanaged[Cdecl] PyImport_ReloadModule { get; } From 31ea87698811ac382243f7328bc0885bfb4e5c75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Tue, 1 Jun 2021 11:21:49 -0400 Subject: [PATCH 13/20] Create a clr.loader module * Return ModuleObject.pyHandle, do not convert. * Write domain tests generated code to file. --- src/runtime/converter.cs | 8 +++ src/runtime/importhook.cs | 65 ++++++++---------------- src/runtime/moduleobject.cs | 15 ++---- src/runtime/runtime.cs | 6 +-- tests/domain_tests/TestRunner.cs | 13 ++++- tests/domain_tests/test_domain_reload.py | 1 - 6 files changed, 46 insertions(+), 62 deletions(-) diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index 47263e8c4..109c590c7 100644 --- 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 index 4eaf3b97e..0e1076ce4 100644 --- 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(); } - /// /// Cleanup resources upon shutdown of the Python runtime. /// @@ -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,7 +101,6 @@ internal static void RestoreRuntimeData(RuntimeDataStorage storage) var rootHandle = storage.GetValue("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(); } @@ -115,54 +108,35 @@ internal static void RestoreRuntimeData(RuntimeDataStorage storage) 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); - 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); } /// @@ -268,7 +242,8 @@ public static unsafe NewReference GetCLRModule() } /// - /// 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. /// 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 index 19178fe41..28c8c2237 100644 --- 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(); - } - - /// /// 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() /// A new reference to the imported module, as a PyObject. [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 index c94531c60..1838d00e6 100644 --- 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])GetFunctionByName("PyModule_Create2TraceRefs", GetUnmanagedDll(_PythonDll)); } - PyModule_AddObject = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyModule_AddObject), GetUnmanagedDll(_PythonDll)); + PyModule_AddObject = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyModule_AddObject), GetUnmanagedDll(_PythonDll)); PyImport_Import = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyImport_Import), GetUnmanagedDll(_PythonDll)); PyImport_ImportModule = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyImport_ImportModule), GetUnmanagedDll(_PythonDll)); PyImport_ReloadModule = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyImport_ReloadModule), GetUnmanagedDll(_PythonDll)); @@ -2797,7 +2797,7 @@ static Delegates() internal static delegate* unmanaged[Cdecl] PyModule_GetDict { get; } internal static delegate* unmanaged[Cdecl] PyModule_GetFilename { get; } internal static delegate* unmanaged[Cdecl] PyModule_Create2 { get; } - internal static delegate* unmanaged[Cdecl] PyModule_AddObject { get; } + internal static delegate* unmanaged[Cdecl] PyModule_AddObject { get; } internal static delegate* unmanaged[Cdecl] PyImport_Import { get; } internal static delegate* unmanaged[Cdecl] PyImport_ImportModule { get; } internal static delegate* unmanaged[Cdecl] PyImport_ReloadModule { get; } diff --git a/tests/domain_tests/TestRunner.cs b/tests/domain_tests/TestRunner.cs index 716fe079b..66fb4f894 100644 --- 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. /// /// 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 index 71b1e348e..e7a82ded2 100644 --- 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") \ No newline at end of file From c02d5c626a400d4d0f2847d24ba958b0fc7af97a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Wed, 2 Jun 2021 10:50:56 -0400 Subject: [PATCH 14/20] Test to test if the test passes --- src/runtime/BorrowedReference.cs | 1 + src/runtime/assemblymanager.cs | 24 ++++++++++++------------ src/runtime/importhook.cs | 19 +++++++++++++------ src/runtime/moduleobject.cs | 5 +++++ 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/runtime/BorrowedReference.cs b/src/runtime/BorrowedReference.cs index bf8a91d3e..75070aac2 100644 --- a/src/runtime/BorrowedReference.cs +++ b/src/runtime/BorrowedReference.cs @@ -9,6 +9,7 @@ readonly ref struct BorrowedReference { readonly IntPtr pointer; public bool IsNull => this.pointer == IntPtr.Zero; + public bool IsNone => this.pointer == Runtime.PyNone; /// Gets a raw pointer to the Python object public IntPtr DangerousGetAddress() diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs index fdde2aeb1..5b4f465b1 100644 --- a/src/runtime/assemblymanager.cs +++ b/src/runtime/assemblymanager.cs @@ -39,7 +39,7 @@ internal class AssemblyManager internal static List pypath; // Triggered when a new namespace is added to the namespaces dictionary - public static event Action namespaceAdded; + // public static event Action namespaceAdded; private AssemblyManager() { @@ -287,17 +287,17 @@ internal static void ScanAssembly(Assembly assembly) if (ns != null) { namespaces[ns].TryAdd(assembly, string.Empty); - try - { - namespaceAdded?.Invoke(ns); - } - catch (Exception e) - { - // For some reason, exceptions happening here does... nothing. - // Even System.AccessViolationExceptions gets ignored. - Console.WriteLine($"Namespace added callback failed with: {e}"); - throw; - } + // try + // { + // namespaceAdded?.Invoke(ns); + // } + // catch (Exception e) + // { + // // For some reason, exceptions happening here does... nothing. + // // Even System.AccessViolationExceptions gets ignored. + // Console.WriteLine($"Namespace added callback failed with: {e}"); + // throw; + // } } if (ns != null && t.IsGenericTypeDefinition) diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 0e1076ce4..8d99980cb 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -147,7 +147,7 @@ static void SetupImportHook() /// static void SetupNamespaceTracking() { - var newset = Runtime.PySet_New(new BorrowedReference(IntPtr.Zero)); + var newset = Runtime.PySet_New(default); try { foreach (var ns in AssemblyManager.GetNamespaces()) @@ -176,8 +176,8 @@ static void SetupNamespaceTracking() newset.Dispose(); } - AssemblyManager.namespaceAdded += OnNamespaceAdded; - PythonEngine.AddShutdownHandler(() => AssemblyManager.namespaceAdded -= OnNamespaceAdded); + // AssemblyManager.namespaceAdded += OnNamespaceAdded; + // PythonEngine.AddShutdownHandler(() => AssemblyManager.namespaceAdded -= OnNamespaceAdded); } /// @@ -186,20 +186,25 @@ static void SetupNamespaceTracking() /// static void TeardownNameSpaceTracking() { - AssemblyManager.namespaceAdded -= OnNamespaceAdded; + // AssemblyManager.namespaceAdded -= OnNamespaceAdded; // If the C# runtime isn't loaded, then there are no namespaces available Runtime.PyDict_SetItemString(root.dict, availableNsKey, Runtime.PyNone); } - static void OnNamespaceAdded(string name) + public static void OnNamespaceAdded(string name) { + Console.WriteLine(System.Environment.StackTrace); + Console.WriteLine("OnNamespaceAdded: acquiring"); + Console.Out.Flush(); using (Py.GIL()) { + Console.WriteLine("OnNamespaceAdded: acquired"); + Console.Out.Flush(); var pyNs = Runtime.PyString_FromString(name); try { var nsSet = Runtime.PyDict_GetItemString(new BorrowedReference(root.dict), availableNsKey); - if (!nsSet.IsNull || nsSet.DangerousGetAddress() != Runtime.PyNone) + if (!(nsSet.IsNull && nsSet.IsNone)) { if (Runtime.PySet_Add(nsSet, new BorrowedReference(pyNs)) != 0) { @@ -212,6 +217,8 @@ static void OnNamespaceAdded(string name) Runtime.XDecref(pyNs); } } + Console.WriteLine("OnNamespaceAdded: released"); + Console.Out.Flush(); } diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 28c8c2237..f0b6840b0 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -529,6 +529,11 @@ public static Assembly AddReference(string name) } // Classes that are not in a namespace needs an extra nudge to be found. ImportHook.UpdateCLRModuleDict(); + + // Heavyhanded but otherwise we'd need a "addedSinceLastCall". + foreach(var ns in AssemblyManager.GetNamespaces()){ + ImportHook.OnNamespaceAdded(ns); + } return assembly; } From 970a18904675dbc6677b0d4262c72290c9ea9896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Wed, 2 Jun 2021 11:08:14 -0400 Subject: [PATCH 15/20] fix new exception usage --- src/runtime/importhook.cs | 10 +++++----- src/runtime/moduleobject.cs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 8d99980cb..69042185f 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -127,7 +127,7 @@ static void SetupImportHook() if(Runtime.PyModule_AddObject(ClrModuleReference, "loader", import_hook_module) != 0) { Runtime.XDecref(import_hook_module.DangerousGetAddress()); - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } // Finally, add the hook to the meta path @@ -157,7 +157,7 @@ static void SetupNamespaceTracking() { if (Runtime.PySet_Add(newset, new BorrowedReference(pyNs)) != 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } finally @@ -168,7 +168,7 @@ static void SetupNamespaceTracking() if (Runtime.PyDict_SetItemString(root.dict, availableNsKey, newset.DangerousGetAddress()) != 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } finally @@ -208,7 +208,7 @@ public static void OnNamespaceAdded(string name) { if (Runtime.PySet_Add(nsSet, new BorrowedReference(pyNs)) != 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } } @@ -275,7 +275,7 @@ public static ModuleObject Import(string modname) if (!(mt is ModuleObject)) { Exceptions.SetError(Exceptions.ImportError, $"'{name}' Is not a ModuleObject."); - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } if (head == null) { diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index f0b6840b0..039562ef8 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -196,7 +196,7 @@ public void LoadNames() { if (Runtime.PyList_Append(new BorrowedReference(__all__), pyname) != 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } finally From 059605bc249cbd7ca2e92e0aa642f8da6b3c3eaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Wed, 2 Jun 2021 13:59:33 -0400 Subject: [PATCH 16/20] kick the build because I can't repro From ff170e99b78ef411e54efd4d2d429799670d043f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Wed, 2 Jun 2021 16:07:27 -0400 Subject: [PATCH 17/20] Add Namespaces to the import hook only through AddReference Don't piggyback on AssemblyManager's AssemblyLoadHandler method because it may be called from other threads, leading to deadlocks if it is called while Python code is executing --- src/runtime/assemblymanager.cs | 15 --------------- src/runtime/importhook.cs | 15 ++------------- src/runtime/moduleobject.cs | 10 +++++++--- 3 files changed, 9 insertions(+), 31 deletions(-) diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs index 5b4f465b1..d44f5f666 100644 --- a/src/runtime/assemblymanager.cs +++ b/src/runtime/assemblymanager.cs @@ -37,10 +37,6 @@ internal class AssemblyManager // modified from event handlers below, potentially triggered from different .NET threads private static ConcurrentQueue assemblies; internal static List pypath; - - // Triggered when a new namespace is added to the namespaces dictionary - // public static event Action namespaceAdded; - private AssemblyManager() { } @@ -287,17 +283,6 @@ internal static void ScanAssembly(Assembly assembly) if (ns != null) { namespaces[ns].TryAdd(assembly, string.Empty); - // try - // { - // namespaceAdded?.Invoke(ns); - // } - // catch (Exception e) - // { - // // For some reason, exceptions happening here does... nothing. - // // Even System.AccessViolationExceptions gets ignored. - // Console.WriteLine($"Namespace added callback failed with: {e}"); - // throw; - // } } if (ns != null && t.IsGenericTypeDefinition) diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 69042185f..acab01792 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -176,30 +176,21 @@ static void SetupNamespaceTracking() newset.Dispose(); } - // AssemblyManager.namespaceAdded += OnNamespaceAdded; - // PythonEngine.AddShutdownHandler(() => AssemblyManager.namespaceAdded -= OnNamespaceAdded); } /// - /// Removes the set of available namespaces from the clr module and - /// removes the callback on the OnNamespaceAdded event. + /// Removes the set of available namespaces from the clr module. /// static void TeardownNameSpaceTracking() { - // AssemblyManager.namespaceAdded -= OnNamespaceAdded; // If the C# runtime isn't loaded, then there are no namespaces available Runtime.PyDict_SetItemString(root.dict, availableNsKey, Runtime.PyNone); } - public static void OnNamespaceAdded(string name) + public static void AddNamespace(string name) { - Console.WriteLine(System.Environment.StackTrace); - Console.WriteLine("OnNamespaceAdded: acquiring"); - Console.Out.Flush(); using (Py.GIL()) { - Console.WriteLine("OnNamespaceAdded: acquired"); - Console.Out.Flush(); var pyNs = Runtime.PyString_FromString(name); try { @@ -217,8 +208,6 @@ public static void OnNamespaceAdded(string name) Runtime.XDecref(pyNs); } } - Console.WriteLine("OnNamespaceAdded: released"); - Console.Out.Flush(); } diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 039562ef8..9dc09ba58 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -509,6 +509,7 @@ public static bool SuppressOverloads public static Assembly AddReference(string name) { AssemblyManager.UpdatePath(); + var origNs = AssemblyManager.GetNamespaces(); Assembly assembly = null; assembly = AssemblyManager.FindLoadedAssembly(name); if (assembly == null) @@ -530,9 +531,12 @@ public static Assembly AddReference(string name) // Classes that are not in a namespace needs an extra nudge to be found. ImportHook.UpdateCLRModuleDict(); - // Heavyhanded but otherwise we'd need a "addedSinceLastCall". - foreach(var ns in AssemblyManager.GetNamespaces()){ - ImportHook.OnNamespaceAdded(ns); + // A bit heavyhanded, but we can't use the AssemblyManager's AssemblyLoadHandler + // method because it may be called from other threads, leading to deadlocks + // if it is called while Python code is executing. + var currNs = AssemblyManager.GetNamespaces().Except(origNs); + foreach(var ns in currNs){ + ImportHook.AddNamespace(ns); } return assembly; } From 63de923a44259732525810bcbcca93bb963d0b9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Tue, 8 Jun 2021 15:16:28 -0400 Subject: [PATCH 18/20] Review changes, update API usage --- src/embed_tests/pyimport.cs | 2 +- src/runtime/BorrowedReference.cs | 1 - src/runtime/importhook.cs | 69 +++++++++++--------------------- src/runtime/moduleobject.cs | 8 ++-- src/runtime/pylist.cs | 2 +- src/runtime/runtime.cs | 26 ++++++++++-- 6 files changed, 51 insertions(+), 57 deletions(-) diff --git a/src/embed_tests/pyimport.cs b/src/embed_tests/pyimport.cs index e98461cbb..de8a06bf8 100644 --- a/src/embed_tests/pyimport.cs +++ b/src/embed_tests/pyimport.cs @@ -37,7 +37,7 @@ public void SetUp() Assert.IsFalse(str == IntPtr.Zero); BorrowedReference path = Runtime.Runtime.PySys_GetObject("path"); Assert.IsFalse(path.IsNull); - Runtime.Runtime.PyList_Append(path, str); + Runtime.Runtime.PyList_Append(path, new BorrowedReference(str)); Runtime.Runtime.XDecref(str); } diff --git a/src/runtime/BorrowedReference.cs b/src/runtime/BorrowedReference.cs index 75070aac2..bf8a91d3e 100644 --- a/src/runtime/BorrowedReference.cs +++ b/src/runtime/BorrowedReference.cs @@ -9,7 +9,6 @@ readonly ref struct BorrowedReference { readonly IntPtr pointer; public bool IsNull => this.pointer == IntPtr.Zero; - public bool IsNone => this.pointer == Runtime.PyNone; /// Gets a raw pointer to the Python object public IntPtr DangerousGetAddress() diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index acab01792..6ad9155a1 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -101,7 +101,7 @@ internal static void RestoreRuntimeData(RuntimeDataStorage storage) var rootHandle = storage.GetValue("root"); root = (CLRModule)ManagedType.GetManagedObject(rootHandle); BorrowedReference dict = Runtime.PyImport_GetModuleDict(); - Runtime.PyDict_SetItemString(dict.DangerousGetAddress(), "clr", py_clr_module); + Runtime.PyDict_SetItemString(dict, "clr", ClrModuleReference); SetupNamespaceTracking(); } @@ -115,14 +115,12 @@ static void SetupImportHook() var exec = Runtime.PyDict_GetItemString(builtins, "exec"); using var args = NewReference.DangerousFromPointer(Runtime.PyTuple_New(2)); - IntPtr codeStr = Runtime.PyString_FromString(LoaderCode); - Runtime.PyTuple_SetItem(args.DangerousGetAddress(), 0, codeStr); - // PyTuple_SetItem steals a reference, mod_dict is borrowed. + var codeStr = NewReference.DangerousFromPointer(Runtime.PyString_FromString(LoaderCode)); + Runtime.PyTuple_SetItem(args, 0, codeStr); 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); - + // reference not stolen due to overload incref'ing for us. + Runtime.PyTuple_SetItem(args, 1, mod_dict); + Runtime.PyObject_Call(exec, args, default); // Set as a sub-module of clr. if(Runtime.PyModule_AddObject(ClrModuleReference, "loader", import_hook_module) != 0) { @@ -132,9 +130,8 @@ static void SetupImportHook() // Finally, add the hook to the meta path 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 finderCtorArgs = NewReference.DangerousFromPointer(Runtime.PyTuple_New(0)); + var finder_inst = Runtime.PyObject_CallObject(findercls, finderCtorArgs); var metapath = Runtime.PySys_GetObject("meta_path"); Runtime.PyList_Append(metapath, finder_inst); } @@ -147,34 +144,19 @@ static void SetupImportHook() /// static void SetupNamespaceTracking() { - var newset = Runtime.PySet_New(default); - try + using var newset = Runtime.PySet_New(default); + foreach (var ns in AssemblyManager.GetNamespaces()) { - foreach (var ns in AssemblyManager.GetNamespaces()) + using var pyNs = NewReference.DangerousFromPointer(Runtime.PyString_FromString(ns)); + if (Runtime.PySet_Add(newset, pyNs) != 0) { - var pyNs = Runtime.PyString_FromString(ns); - try - { - if (Runtime.PySet_Add(newset, new BorrowedReference(pyNs)) != 0) - { - throw PythonException.ThrowLastAsClrException(); - } - } - finally - { - Runtime.XDecref(pyNs); - } + throw PythonException.ThrowLastAsClrException(); } - - if (Runtime.PyDict_SetItemString(root.dict, availableNsKey, newset.DangerousGetAddress()) != 0) + if (Runtime.PyDict_SetItemString(root.DictRef, availableNsKey, newset) != 0) { throw PythonException.ThrowLastAsClrException(); } } - finally - { - newset.Dispose(); - } } @@ -189,24 +171,21 @@ static void TeardownNameSpaceTracking() public static void AddNamespace(string name) { - using (Py.GIL()) + var pyNs = Runtime.PyString_FromString(name); + try { - var pyNs = Runtime.PyString_FromString(name); - try + var nsSet = Runtime.PyDict_GetItemString(new BorrowedReference(root.dict), availableNsKey); + if (!(nsSet.IsNull || nsSet.DangerousGetAddress() == Runtime.PyNone)) { - var nsSet = Runtime.PyDict_GetItemString(new BorrowedReference(root.dict), availableNsKey); - if (!(nsSet.IsNull && nsSet.IsNone)) + if (Runtime.PySet_Add(nsSet, new BorrowedReference(pyNs)) != 0) { - if (Runtime.PySet_Add(nsSet, new BorrowedReference(pyNs)) != 0) - { - throw PythonException.ThrowLastAsClrException(); - } + throw PythonException.ThrowLastAsClrException(); } } - finally - { - Runtime.XDecref(pyNs); - } + } + finally + { + Runtime.XDecref(pyNs); } } diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 9dc09ba58..c2614b1d8 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -194,7 +194,7 @@ public void LoadNames() var pyname = Runtime.PyString_FromString(name); try { - if (Runtime.PyList_Append(new BorrowedReference(__all__), pyname) != 0) + if (Runtime.PyList_Append(new BorrowedReference(__all__), new BorrowedReference(pyname)) != 0) { throw PythonException.ThrowLastAsClrException(); } @@ -598,10 +598,8 @@ public static string[] ListAssemblies(bool verbose) public static ModuleObject _load_clr_module(PyObject spec) { ModuleObject mod = null; - using (var modname = spec.GetAttr("name")) - { - mod = ImportHook.Import(modname.ToString()); - } + using var modname = spec.GetAttr("name"); + mod = ImportHook.Import(modname.ToString()); return mod; } } diff --git a/src/runtime/pylist.cs b/src/runtime/pylist.cs index 039f5e313..8f346524f 100644 --- a/src/runtime/pylist.cs +++ b/src/runtime/pylist.cs @@ -132,7 +132,7 @@ public static PyList AsList(PyObject value) /// public void Append(PyObject item) { - int r = Runtime.PyList_Append(this.Reference, item.obj); + int r = Runtime.PyList_Append(this.Reference, new BorrowedReference(item.obj)); if (r < 0) { throw PythonException.ThrowLastAsClrException(); diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 1838d00e6..b3979c5c9 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -174,7 +174,7 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd IntPtr item = PyString_FromString(rtdir); if (PySequence_Contains(path, item) == 0) { - PyList_Append(new BorrowedReference(path), item); + PyList_Append(new BorrowedReference(path), new BorrowedReference(item)); } XDecref(item); AssemblyManager.UpdatePath(); @@ -1087,6 +1087,8 @@ internal static IntPtr PyObject_GetAttr(IntPtr pointer, IntPtr name) internal static IntPtr PyObject_Call(IntPtr pointer, IntPtr args, IntPtr kw) => Delegates.PyObject_Call(pointer, args, kw); + internal static IntPtr PyObject_Call(BorrowedReference pointer, BorrowedReference args, BorrowedReference kw) + => Delegates.PyObject_Call(pointer.DangerousGetAddress(), args.DangerousGetAddress(), kw.DangerousGetAddressOrNull()); internal static NewReference PyObject_CallObject(BorrowedReference callable, BorrowedReference args) => Delegates.PyObject_CallObject(callable, args); @@ -1822,7 +1824,7 @@ internal static int PyList_Insert(BorrowedReference pointer, long index, IntPtr private static int PyList_Insert(BorrowedReference pointer, IntPtr index, IntPtr value) => Delegates.PyList_Insert(pointer, index, value); - internal static int PyList_Append(BorrowedReference pointer, IntPtr value) => Delegates.PyList_Append(pointer, value); + internal static int PyList_Append(BorrowedReference pointer, BorrowedReference value) => Delegates.PyList_Append(pointer, value); internal static int PyList_Reverse(BorrowedReference pointer) => Delegates.PyList_Reverse(pointer); @@ -1885,7 +1887,15 @@ internal static int PyTuple_SetItem(IntPtr pointer, long index, IntPtr value) { return PyTuple_SetItem(pointer, new IntPtr(index), value); } + internal static int PyTuple_SetItem(BorrowedReference pointer, long index, StolenReference value) + => PyTuple_SetItem(pointer.DangerousGetAddress(), new IntPtr(index), value.DangerousGetAddressOrNull()); + internal static int PyTuple_SetItem(BorrowedReference pointer, long index, BorrowedReference value) + { + var increfValue = value.DangerousGetAddress(); + Runtime.XIncref(increfValue); + return PyTuple_SetItem(pointer.DangerousGetAddress(), new IntPtr(index), increfValue); + } private static int PyTuple_SetItem(IntPtr pointer, IntPtr index, IntPtr value) => Delegates.PyTuple_SetItem(pointer, index, value); @@ -1944,6 +1954,14 @@ internal static string PyModule_GetFilename(IntPtr module) internal static IntPtr PyImport_Import(IntPtr name) => Delegates.PyImport_Import(name); + + /// + /// We can't use a StolenReference here because the reference is stolen only on success. + /// + /// The module to add the object to. + /// The key that will refer to the object. + /// The object to add to the module. + /// Return -1 on error, 0 on success. internal static int PyModule_AddObject(BorrowedReference module, string name, BorrowedReference stolenObject) { using var namePtr = new StrPtr(name, Encoding.UTF8); @@ -2483,7 +2501,7 @@ static Delegates() PyList_GetItem = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyList_GetItem), GetUnmanagedDll(_PythonDll)); PyList_SetItem = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyList_SetItem), GetUnmanagedDll(_PythonDll)); PyList_Insert = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyList_Insert), GetUnmanagedDll(_PythonDll)); - PyList_Append = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyList_Append), GetUnmanagedDll(_PythonDll)); + PyList_Append = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyList_Append), GetUnmanagedDll(_PythonDll)); PyList_Reverse = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyList_Reverse), GetUnmanagedDll(_PythonDll)); PyList_Sort = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyList_Sort), GetUnmanagedDll(_PythonDll)); PyList_GetSlice = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyList_GetSlice), GetUnmanagedDll(_PythonDll)); @@ -2780,7 +2798,7 @@ static Delegates() internal static delegate* unmanaged[Cdecl] PyList_GetItem { get; } internal static delegate* unmanaged[Cdecl] PyList_SetItem { get; } internal static delegate* unmanaged[Cdecl] PyList_Insert { get; } - internal static delegate* unmanaged[Cdecl] PyList_Append { get; } + internal static delegate* unmanaged[Cdecl] PyList_Append { get; } internal static delegate* unmanaged[Cdecl] PyList_Reverse { get; } internal static delegate* unmanaged[Cdecl] PyList_Sort { get; } internal static delegate* unmanaged[Cdecl] PyList_GetSlice { get; } From bd7e7450847dec33f9906d95270eae61fb8eead6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Mon, 14 Jun 2021 09:57:53 -0400 Subject: [PATCH 19/20] make PyModule_AddObject in line with CPython --- src/runtime/importhook.cs | 2 +- src/runtime/runtime.cs | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 6ad9155a1..1111adc28 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -122,7 +122,7 @@ static void SetupImportHook() Runtime.PyTuple_SetItem(args, 1, mod_dict); Runtime.PyObject_Call(exec, args, default); // Set as a sub-module of clr. - if(Runtime.PyModule_AddObject(ClrModuleReference, "loader", import_hook_module) != 0) + if(Runtime.PyModule_AddObject(ClrModuleReference, "loader", import_hook_module.DangerousGetAddress()) != 0) { Runtime.XDecref(import_hook_module.DangerousGetAddress()); throw PythonException.ThrowLastAsClrException(); diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index ec3d64000..009412ea5 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -1934,9 +1934,12 @@ internal static string PyModule_GetFilename(IntPtr module) /// /// The module to add the object to. /// The key that will refer to the object. - /// The object to add to the module. + /// + /// The object to add to the module. The reference will be stolen only if the + /// method returns 0. + /// /// Return -1 on error, 0 on success. - internal static int PyModule_AddObject(BorrowedReference module, string name, BorrowedReference stolenObject) + internal static int PyModule_AddObject(BorrowedReference module, string name, IntPtr stolenObject) { using var namePtr = new StrPtr(name, Encoding.UTF8); return Delegates.PyModule_AddObject(module, namePtr, stolenObject); @@ -2498,7 +2501,7 @@ static Delegates() { PyModule_Create2 = (delegate* unmanaged[Cdecl])GetFunctionByName("PyModule_Create2TraceRefs", GetUnmanagedDll(_PythonDll)); } - PyModule_AddObject = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyModule_AddObject), GetUnmanagedDll(_PythonDll)); + PyModule_AddObject = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyModule_AddObject), GetUnmanagedDll(_PythonDll)); PyImport_Import = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyImport_Import), GetUnmanagedDll(_PythonDll)); PyImport_ImportModule = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyImport_ImportModule), GetUnmanagedDll(_PythonDll)); PyImport_ReloadModule = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyImport_ReloadModule), GetUnmanagedDll(_PythonDll)); @@ -2787,7 +2790,7 @@ static Delegates() internal static delegate* unmanaged[Cdecl] PyModule_GetDict { get; } internal static delegate* unmanaged[Cdecl] PyModule_GetFilename { get; } internal static delegate* unmanaged[Cdecl] PyModule_Create2 { get; } - internal static delegate* unmanaged[Cdecl] PyModule_AddObject { get; } + internal static delegate* unmanaged[Cdecl] PyModule_AddObject { get; } internal static delegate* unmanaged[Cdecl] PyImport_Import { get; } internal static delegate* unmanaged[Cdecl] PyImport_ImportModule { get; } internal static delegate* unmanaged[Cdecl] PyImport_ReloadModule { get; } From 46a85fedd1197a3ff3d4f2df337e5d900639b696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Tue, 15 Jun 2021 09:33:45 -0400 Subject: [PATCH 20/20] take care of stragglers --- src/runtime/converter.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index e19a62618..7cee0890c 100644 --- a/src/runtime/converter.cs +++ b/src/runtime/converter.cs @@ -207,7 +207,9 @@ internal static IntPtr ToPython(object value, Type type) // pyHandle as is, do not convert. if (value is ModuleObject modobj) { - return modobj.pyHandle; + var handle = modobj.pyHandle; + Runtime.XIncref(handle); + return handle; } // hmm - from Python, we almost never care what the declared