From 0d7498b7574137f8c82242b6f8aa94210329e8f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Wed, 9 Dec 2020 15:43:30 -0500 Subject: [PATCH 01/13] Simplify the Unity custom patch Lay the burden to provide the library name and directory on the user. A.K.A.: Improve portability. --- src/runtime/platform/LibraryLoader.cs | 14 ++++++++------ src/runtime/pythonengine.cs | 11 +++++++++++ src/runtime/runtime.cs | 4 ---- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/runtime/platform/LibraryLoader.cs b/src/runtime/platform/LibraryLoader.cs index 193dff274..11aafc9e8 100644 --- a/src/runtime/platform/LibraryLoader.cs +++ b/src/runtime/platform/LibraryLoader.cs @@ -1,12 +1,13 @@ using System; using System.ComponentModel; +using System.IO; using System.Runtime.InteropServices; namespace Python.Runtime.Platform { interface ILibraryLoader { - IntPtr Load(string dllToLoad); + IntPtr Load(string dllToLoad, string directory = ""); IntPtr GetFunction(IntPtr hModule, string procedureName); @@ -47,9 +48,9 @@ class LinuxLoader : ILibraryLoader private static IntPtr RTLD_DEFAULT = IntPtr.Zero; private const string NativeDll = "libdl.so"; - public IntPtr Load(string dllToLoad) + public IntPtr Load(string dllToLoad, string directory = "") { - var filename = $"lib{dllToLoad}.so"; + var filename = $"{directory}lib{dllToLoad}.so"; ClearError(); var res = dlopen(filename, RTLD_NOW | RTLD_GLOBAL); if (res == IntPtr.Zero) @@ -118,9 +119,9 @@ class DarwinLoader : ILibraryLoader private const string NativeDll = "/usr/lib/libSystem.dylib"; private static IntPtr RTLD_DEFAULT = new IntPtr(-2); - public IntPtr Load(string dllToLoad) + public IntPtr Load(string dllToLoad, string directory = "") { - var filename = $"lib{dllToLoad}.dylib"; + var filename = $"{directory}lib{dllToLoad}.dylib"; ClearError(); var res = dlopen(filename, RTLD_NOW | RTLD_GLOBAL); if (res == IntPtr.Zero) @@ -187,8 +188,9 @@ class WindowsLoader : ILibraryLoader private const string NativeDll = "kernel32.dll"; - public IntPtr Load(string dllToLoad) + public IntPtr Load(string dllToLoad, string directory = "") { + dllToLoad = Path.GetFullPath($"{directory}{dllToLoad}.dll"); var res = WindowsLoader.LoadLibrary(dllToLoad); if (res == IntPtr.Zero) throw new DllNotFoundException($"Could not load {dllToLoad}", new Win32Exception()); diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs index 781d345e7..fdc428570 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -150,6 +150,17 @@ public static int RunSimpleString(string code) return Runtime.PyRun_SimpleString(code); } + static bool libraryLoaded = false; + public static void InitializeLibrary(string library, string directory) + { + if (!libraryLoaded) + { + var _loader = Python.Runtime.Platform.LibraryLoader.Get(Python.Runtime.Platform.NativeCodePageHelper.OperatingSystem); + _loader.Load(library, directory); + libraryLoaded = true; + } + } + public static void Initialize() { Initialize(setSysArgv: true); diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 8197f027b..27b054cd6 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -57,11 +57,7 @@ public class Runtime public static readonly string PythonDLL = _PythonDll; -#if PYTHON_WITHOUT_ENABLE_SHARED && !NETSTANDARD internal const string _PythonDll = "__Internal"; -#else - internal const string _PythonDll = dllBase + dllWithPyDebug + dllWithPyMalloc; -#endif // set to true when python is finalizing internal static object IsFinalizingLock = new object(); From 744f99120ff17ad88403a5cac869f670ba60f375 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 02/13] Modernize the import hook Implement a meta path loader instead Add the loaded namespaces tracking Fix a bug where clr wasn't in sys.modules after reload Further refinements to setattr logic on ModuleObjects --- src/domain_tests/TestRunner.cs | 23 ++ src/domain_tests/test_domain_reload.py | 4 + src/runtime/assemblymanager.cs | 23 ++ src/runtime/extensiontype.cs | 2 +- src/runtime/importhook.cs | 356 ++++++++++--------------- src/runtime/moduleobject.cs | 77 +++++- src/runtime/native/TypeOffset.cs | 1 + 7 files changed, 262 insertions(+), 224 deletions(-) diff --git a/src/domain_tests/TestRunner.cs b/src/domain_tests/TestRunner.cs index a21297829..716fe079b 100644 --- a/src/domain_tests/TestRunner.cs +++ b/src/domain_tests/TestRunner.cs @@ -1092,6 +1092,29 @@ 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; we're testing + // that the import hook behaves properly after a domain reload + Name = "import_after_reload", + DotNetBefore = "", + DotNetAfter = "", + 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/src/domain_tests/test_domain_reload.py b/src/domain_tests/test_domain_reload.py index e24eb6976..9b2ba877e 100644 --- a/src/domain_tests/test_domain_reload.py +++ b/src/domain_tests/test_domain_reload.py @@ -98,3 +98,7 @@ def test_in_to_ref_param(): @pytest.mark.skipif(platform.system() == 'Darwin', reason='FIXME: macos can\'t find the python library') 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 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 a5f0f1219..623dd927a 100644 --- a/src/runtime/extensiontype.cs +++ b/src/runtime/extensiontype.cs @@ -76,7 +76,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 af6174188..91c69e77a 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 @@ -9,13 +8,46 @@ 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; private static IntPtr module_def = IntPtr.Zero; + 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 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 + +sys.meta_path.append(DotNetFinder()) + "; + const string availableNsKey = "_availableNamespaces"; + internal static void InitializeModuleDef() { if (module_def == IntPtr.Zero) @@ -34,50 +66,11 @@ internal static void ReleaseModuleDef() module_def = IntPtr.Zero; } - /// - /// 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(); - - 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); - } - /// /// Initialization performed on startup of the Python runtime. /// internal static void Initialize() { - InitImport(); - // Initialize the clr module and tell Python about it. root = new CLRModule(); @@ -94,6 +87,10 @@ internal static void Initialize() IntPtr dict = Runtime.PyImport_GetModuleDict(); Runtime.PyDict_SetItemString(dict, "CLR", py_clr_module); Runtime.PyDict_SetItemString(dict, "clr", py_clr_module); + + // Add/create the MetaPathLoader + SetupNamespaceTracking(); + PythonEngine.Exec(LoaderCode); } @@ -107,8 +104,6 @@ internal static void Shutdown() return; } - RestoreImport(); - bool shouldFreeDef = Runtime.Refcount(py_clr_module) == 1; Runtime.XDecref(py_clr_module); py_clr_module = IntPtr.Zero; @@ -117,6 +112,7 @@ internal static void Shutdown() ReleaseModuleDef(); } + TeardownNameSpaceTracking(); Runtime.XDecref(root.pyHandle); root = null; CLRModule.Reset(); @@ -134,199 +130,135 @@ 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); + IntPtr dict = Runtime.PyImport_GetModuleDict(); + Runtime.PyDict_SetItemString(dict, "CLR", py_clr_module); + Runtime.PyDict_SetItemString(dict, "clr", py_clr_module); + SetupNamespaceTracking(); } /// - /// Return the clr python module (new reference) + /// 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. /// - public static IntPtr GetCLRModule(IntPtr? fromList = null) + static void SetupNamespaceTracking () { - root.InitializePreload(); - - // update the module dictionary with the contents of the root dictionary - root.LoadNames(); - IntPtr py_mod_dict = Runtime.PyModule_GetDict(py_clr_module); - IntPtr clr_dict = Runtime._PyObject_GetDictPtr(root.pyHandle); // PyObject** - clr_dict = (IntPtr)Marshal.PtrToStructure(clr_dict, typeof(IntPtr)); - 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 && fromList != IntPtr.Zero) + var newset = Runtime.PySet_New(IntPtr.Zero); + try { - if (Runtime.PyTuple_Check(fromList.GetValueOrDefault())) + foreach (var ns in AssemblyManager.GetNamespaces()) { - Runtime.XIncref(py_mod_dict); - using (var mod_dict = new PyDict(py_mod_dict)) + var pyNs = Runtime.PyString_FromString(ns); + try { - Runtime.XIncref(fromList.GetValueOrDefault()); - using (var from = new PyTuple(fromList.GetValueOrDefault())) + if(Runtime.PySet_Add(newset, pyNs) != 0) { - 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); - } - } + throw new PythonException(); } } + finally + { + Runtime.XDecref(pyNs); + } + } + + if(Runtime.PyDict_SetItemString(root.dict, availableNsKey, newset) != 0) + { + throw new PythonException(); } } - Runtime.XIncref(py_clr_module); - return py_clr_module; + finally + { + Runtime.XDecref(newset); + } + + AssemblyManager.namespaceAdded += OnNamespaceAdded; + PythonEngine.AddShutdownHandler(()=>AssemblyManager.namespaceAdded -= OnNamespaceAdded); } /// - /// The actual import hook that ties Python to the managed world. + /// Removes the set of available namespaces from the clr module and + /// removes the callback on the OnNamespaceAdded event. /// - public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw) + static void TeardownNameSpaceTracking() { - // 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)"); - } - - // borrowed reference - IntPtr py_mod_name = Runtime.PyTuple_GetItem(args, 0); - if (py_mod_name == IntPtr.Zero || - !Runtime.IsStringType(py_mod_name)) + 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))) { - return Exceptions.RaiseTypeError("string expected"); + // Trying to remove a key that's not in the dictionary + // raises an error. We don't care about it. + Runtime.PyErr_Clear(); } - - // Check whether the import is of the form 'from x import y'. - // This determines whether we return the head or tail module. - - IntPtr fromList = IntPtr.Zero; - var fromlist = false; - if (num_args >= 4) + else if (Exceptions.ErrorOccurred()) { - fromList = Runtime.PyTuple_GetItem(args, 3); - if (fromList != IntPtr.Zero && - Runtime.PyObject_IsTrue(fromList) == 1) - { - fromlist = true; - } + throw new PythonException(); } + } - 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") + static void OnNamespaceAdded (string name) + { + using(Py.GIL()) { - IntPtr clr_module = GetCLRModule(fromList); - if (clr_module != IntPtr.Zero) + var pyNs = Runtime.PyString_FromString(name); + try { - IntPtr sys_modules = Runtime.PyImport_GetModuleDict(); - if (sys_modules != IntPtr.Zero) + var nsSet = Runtime.PyDict_GetItemString(root.dict, availableNsKey); + if (nsSet != IntPtr.Zero) { - Runtime.PyDict_SetItemString(sys_modules, "clr", clr_module); + if(Runtime.PySet_Add(nsSet, pyNs) != 0) + { + throw new PythonException(); + } } } - return clr_module; - } - - string realname = mod_name; - string clr_prefix = null; - - // 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, kw); - if (res != IntPtr.Zero) - { - // There was no error. - if (fromlist && IsLoadAll(fromList)) + finally { - var mod = ManagedType.GetManagedObject(res) as ModuleObject; - mod?.LoadNames(); + Runtime.XDecref(pyNs); } - 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 = new PythonException(); - // Otherwise, just clear the it. - Exceptions.Clear(); - string[] names = realname.Split('.'); + /// + /// 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. + /// + internal static void UpdateCLRModuleDict() + { + root.InitializePreload(); - // See if sys.modules for this interpreter already has the - // requested module. If so, just return the existing module. - IntPtr modules = Runtime.PyImport_GetModuleDict(); - IntPtr module = Runtime.PyDict_GetItem(modules, py_mod_name); + // update the module dictionary with the contents of the root dictionary + root.LoadNames(); + IntPtr py_mod_dict = Runtime.PyModule_GetDict(py_clr_module); + IntPtr clr_dict = Runtime._PyObject_GetDictPtr(root.pyHandle); // PyObject** + clr_dict = (IntPtr)Marshal.PtrToStructure(clr_dict, typeof(IntPtr)); + Runtime.PyDict_Update(py_mod_dict, clr_dict); + } - if (module != IntPtr.Zero) - { - if (fromlist) - { - if (IsLoadAll(fromList)) - { - var mod = ManagedType.GetManagedObject(module) as ModuleObject; - mod?.LoadNames(); - } - Runtime.XIncref(module); - return module; - } - if (clr_prefix != null) - { - return GetCLRModule(fromList); - } - module = Runtime.PyDict_GetItemString(modules, names[0]); - Runtime.XIncref(module); - return module; - } - Exceptions.Clear(); + /// + /// Return the clr python module (new reference) + /// + public static IntPtr GetCLRModule() + { + + UpdateCLRModuleDict(); + Runtime.XIncref(py_clr_module); + return py_clr_module; + } - // Traverse the qualified module name to get the named module - // and place references in sys.modules as we go. Note that if + /// + /// The hook to import a CLR module into Python + /// + public static ModuleObject __import__(string modname) + { + // 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 @@ -335,17 +267,18 @@ public static IntPtr __import__(IntPtr self, IntPtr args, 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(); + string[] names = modname.Split('.'); foreach (string name in names) { ManagedType mt = tail.GetAttribute(name, true); if (!(mt is ModuleObject)) { - originalException.Restore(); - return IntPtr.Zero; + Exceptions.SetError(Exceptions.ImportError, $"'{name}' Is not a ModuleObject."); + throw new PythonException(); } if (head == null) { @@ -356,28 +289,9 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw) { tail.LoadNames(); } - - // Add the module to sys.modules - Runtime.PyDict_SetItemString(modules, tail.moduleName, tail.pyHandle); - - // If imported from CLR add clr. to sys.modules as well - if (clr_prefix != null) - { - Runtime.PyDict_SetItemString(modules, clr_prefix + tail.moduleName, tail.pyHandle); - } } - { - var mod = fromlist ? tail : head; - - if (fromlist && IsLoadAll(fromList)) - { - mod.LoadNames(); - } - - Runtime.XIncref(mod.pyHandle); - return mod.pyHandle; - } + return tail; } private static bool IsLoadAll(IntPtr fromList) diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 07dd20e55..7067bfca2 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -19,6 +19,12 @@ internal class ModuleObject : ExtensionType internal string moduleName; internal IntPtr dict; 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) { @@ -44,6 +50,7 @@ public ModuleObject(string name) } dict = Runtime.PyDict_New(); + __all__ = Runtime.PyList_New(0); IntPtr pyname = Runtime.PyString_FromString(moduleName); IntPtr pyfilename = Runtime.PyString_FromString(filename); IntPtr pydocstring = Runtime.PyString_FromString(docstring); @@ -183,7 +190,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); + } + } } } @@ -265,6 +288,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 @@ -330,6 +360,25 @@ public static int tp_clear(IntPtr ob) return 0; } + /// + /// 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 managedKey = Runtime.GetManagedString(key); + if ((settableAttributes.Contains(managedKey)) || + (ManagedType.GetManagedObject(val)?.GetType() == typeof(ModuleObject)) ) + { + var self = (ModuleObject)ManagedType.GetManagedObject(ob); + return Runtime.PyDict_SetItem(self.dict, key, val); + } + + return ExtensionType.tp_setattro(ob, key, val); + } + protected override void OnSave(InterDomainContext context) { base.OnSave(context); @@ -489,7 +538,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; } @@ -542,5 +592,28 @@ 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) + { + 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); + } } } diff --git a/src/runtime/native/TypeOffset.cs b/src/runtime/native/TypeOffset.cs index 4c1bcefa0..c1e02757c 100644 --- a/src/runtime/native/TypeOffset.cs +++ b/src/runtime/native/TypeOffset.cs @@ -157,6 +157,7 @@ static void ValidateRequiredOffsetsPresent(PropertyInfo[] offsetProperties) "getPreload", "Initialize", "ListAssemblies", + "_LoadClrModule", "Release", "Reset", "set_SuppressDocs", From d7c20ed25a0704fb4fa730e6970b54ae3d85f93c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Tue, 26 Jan 2021 13:25:55 -0500 Subject: [PATCH 03/13] Fix library loader usage It was modernized in the upstream --- src/runtime/pythonengine.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs index fdc428570..2b66cb889 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -155,7 +155,7 @@ public static void InitializeLibrary(string library, string directory) { if (!libraryLoaded) { - var _loader = Python.Runtime.Platform.LibraryLoader.Get(Python.Runtime.Platform.NativeCodePageHelper.OperatingSystem); + var _loader = Python.Runtime.Platform.LibraryLoader.Instance; _loader.Load(library, directory); libraryLoaded = true; } From 9d0bd9c2bbee00f09f9948437879b557b6202533 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Tue, 26 Jan 2021 13:58:11 -0500 Subject: [PATCH 04/13] Remove StrongNameIdentity attribute from PropertyObject --- src/runtime/propertyobject.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/runtime/propertyobject.cs b/src/runtime/propertyobject.cs index 20061b358..7fd0f4016 100644 --- a/src/runtime/propertyobject.cs +++ b/src/runtime/propertyobject.cs @@ -1,6 +1,5 @@ using System; using System.Reflection; -using System.Security.Permissions; namespace Python.Runtime { @@ -15,7 +14,6 @@ internal class PropertyObject : ExtensionType private MaybeMethodInfo getter; private MaybeMethodInfo setter; - [StrongNameIdentityPermission(SecurityAction.Assert)] public PropertyObject(PropertyInfo md) { getter = md.GetGetMethod(true); From c75ee464010266d1794f8a65dbe6d29c63f66dd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Thu, 23 Jun 2022 11:24:47 -0400 Subject: [PATCH 05/13] Last line of defense, best effort serialization This commit adds a "last line of defense, best effort serialization" to serialize types not marked as Serializable. Such objects are deserialized as derived classes with all methods and properties overriden to throw a "Not Serialized" Exception. Fields are not initialized and may be null. Sealed classes and implemented interface methods are still a problem to be solved. --- src/runtime/StateSerialization/RuntimeData.cs | 384 +++++++++++++++++- tests/domain_tests/TestRunner.cs | 162 ++++++++ tests/domain_tests/test_domain_reload.py | 8 + 3 files changed, 534 insertions(+), 20 deletions(-) diff --git a/src/runtime/StateSerialization/RuntimeData.cs b/src/runtime/StateSerialization/RuntimeData.cs index 204e15b5b..cbcf160af 100644 --- a/src/runtime/StateSerialization/RuntimeData.cs +++ b/src/runtime/StateSerialization/RuntimeData.cs @@ -3,18 +3,303 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; +using System.Dynamic; using System.IO; using System.Linq; +using System.Reflection; +using System.Reflection.Emit; using System.Runtime.InteropServices; using System.Runtime.Serialization; using System.Runtime.Serialization.Formatters.Binary; - using Python.Runtime.StateSerialization; using static Python.Runtime.Runtime; namespace Python.Runtime { + [System.Serializable] + public sealed class NotSerializedException: SerializationException + { + static string _message = "The underlying C# object has been deleted."; + public NotSerializedException() : base(_message){} + private NotSerializedException(SerializationInfo info, StreamingContext context) : base(info, context){} + override public void GetObjectData(SerializationInfo info, StreamingContext context) => base.GetObjectData(info, context); + } + + + // empty attribute to mark classes created by the "non-serializer" so we don't loop-inherit + // on multiple cycles of de/serialization + [System.AttributeUsage(System.AttributeTargets.All, Inherited = false, AllowMultiple = true)] + [System.Serializable] + sealed class PyNet_NotSerializedAttribute : System.Attribute {} + + [Serializable] + internal static class NonSerializedTypeBuilder + { + + internal static AssemblyName nonSerializedAssemblyName = + new AssemblyName("Python.Runtime.NonSerialized.dll, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null"); + internal static AssemblyBuilder assemblyForNonSerializedClasses = + AppDomain.CurrentDomain.DefineDynamicAssembly(nonSerializedAssemblyName, AssemblyBuilderAccess.Run); + internal static ModuleBuilder moduleBuilder = assemblyForNonSerializedClasses.DefineDynamicModule("NotSerializedModule"); + internal static HashSet dontReimplementMethods = new(){"Finalize", "Dispose", "GetType", "ReferenceEquals", "GetHashCode", "Equals"}; + const string notSerializedSuffix = "_NotSerialized"; + + public static object CreateNewObject(Type baseType) + { + var myType = CreateType(baseType); + var myObject = Activator.CreateInstance(myType); + return myObject; + } + + public static Type CreateType(Type tp) + { + Type existingType = assemblyForNonSerializedClasses.GetType(tp.Name + "_NotSerialized", throwOnError:false); + if (existingType is not null) + { + return existingType; + } + + TypeBuilder tb = GetTypeBuilder(tp); + ConstructorBuilder constructor = tb.DefineDefaultConstructor(MethodAttributes.Public | MethodAttributes.SpecialName | MethodAttributes.RTSpecialName); + var properties = tp.GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static); + foreach (var prop in properties) + { + CreateProperty(tb, prop); + } + + var methods = tp.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static); + foreach (var meth in methods) + { + CreateMethod(tb, meth); + } + + ImplementEqualityAndHash(tb); + + return tb.CreateType(); + } + + private static void ImplementEqualityAndHash(TypeBuilder tb) + { + var hashCodeMb = tb.DefineMethod("GetHashCode", + MethodAttributes.Public | MethodAttributes.Final | MethodAttributes.ReuseSlot, + CallingConventions.Standard, + typeof(int), + Type.EmptyTypes + ); + var getHashIlGen = hashCodeMb.GetILGenerator(); + getHashIlGen.Emit(OpCodes.Ldarg_0); + getHashIlGen.EmitCall(OpCodes.Call, typeof(object).GetMethod("GetType"), Type.EmptyTypes); + getHashIlGen.EmitCall(OpCodes.Call, typeof(Type).GetProperty("Name").GetMethod, Type.EmptyTypes); + getHashIlGen.EmitCall(OpCodes.Call, typeof(string).GetMethod("GetHashCode"), Type.EmptyTypes); + getHashIlGen.Emit(OpCodes.Ret); + + Type[] equalsArgs = new Type[] {typeof(object), typeof(object)}; + var equalsMb = tb.DefineMethod("Equals", + MethodAttributes.Public | MethodAttributes.Final | MethodAttributes.ReuseSlot, + CallingConventions.Standard, + typeof(bool), + equalsArgs + ); + var equalsIlGen = equalsMb.GetILGenerator(); + equalsIlGen.Emit(OpCodes.Ldarg_0); // this + equalsIlGen.Emit(OpCodes.Ldarg_1); // the other object + equalsIlGen.EmitCall(OpCodes.Call, typeof(object).GetMethod("ReferenceEquals"), equalsArgs); + equalsIlGen.Emit(OpCodes.Ret); + } + + private static TypeBuilder GetTypeBuilder(Type baseType) + { + string typeSignature = baseType.Name + notSerializedSuffix; + + TypeBuilder tb = moduleBuilder.DefineType(typeSignature, + baseType.Attributes, + baseType, + baseType.GetInterfaces()); + + ConstructorInfo attrCtorInfo = typeof(PyNet_NotSerializedAttribute).GetConstructor(new Type[]{}); + CustomAttributeBuilder attrBuilder = new CustomAttributeBuilder(attrCtorInfo,new object[]{}); + tb.SetCustomAttribute(attrBuilder); + + return tb; + } + + static ILGenerator GenerateExceptionILCode(dynamic builder) + { + ILGenerator ilgen = builder.GetILGenerator(); + var seriExc = typeof(NotSerializedException); + var exCtorInfo = seriExc.GetConstructor(new Type[]{}); + ilgen.Emit(OpCodes.Newobj, exCtorInfo); + ilgen.ThrowException(seriExc); + return ilgen; + } + + private static MethodAttributes GetMethodAttrs (MethodInfo minfo) + { + var methAttributes = minfo.Attributes; + // Always implement/shadow the method + methAttributes &=(~MethodAttributes.Abstract); + methAttributes &=(~MethodAttributes.NewSlot); + methAttributes |= MethodAttributes.ReuseSlot; + methAttributes |= MethodAttributes.HideBySig; + methAttributes |= MethodAttributes.Final; + + if (minfo.IsFinal) + { + // can't override a final method, new it instead. + methAttributes &= (~MethodAttributes.Virtual); + methAttributes |= MethodAttributes.NewSlot; + } + + return methAttributes; + } + + private static void CreateProperty(TypeBuilder tb, PropertyInfo pinfo) + { + string propertyName = pinfo.Name; + Type propertyType = pinfo.PropertyType; + FieldBuilder fieldBuilder = tb.DefineField("_" + propertyName, propertyType, FieldAttributes.Private); + PropertyBuilder propertyBuilder = tb.DefineProperty(propertyName, pinfo.Attributes, propertyType, null); + if (pinfo.GetMethod is not null) + { + var methAttributes = GetMethodAttrs(pinfo.GetMethod); + + MethodBuilder getPropMthdBldr = + tb.DefineMethod("get_" + propertyName, + methAttributes, + propertyType, + Type.EmptyTypes); + GenerateExceptionILCode(getPropMthdBldr); + propertyBuilder.SetGetMethod(getPropMthdBldr); + } + if (pinfo.SetMethod is not null) + { + var methAttributes = GetMethodAttrs(pinfo.SetMethod); + MethodBuilder setPropMthdBldr = + tb.DefineMethod("set_" + propertyName, + methAttributes, + null, + new[] { propertyType }); + + GenerateExceptionILCode(setPropMthdBldr); + propertyBuilder.SetSetMethod(setPropMthdBldr); + } + } + + private static void CreateMethod(TypeBuilder tb, MethodInfo minfo) + { + Console.WriteLine($"overimplementing method for: {minfo} {minfo.IsVirtual} {minfo.IsFinal} "); + string methodName = minfo.Name; + + if (dontReimplementMethods.Contains(methodName)) + { + // Some methods must *not* be reimplemented (who wants to throw from Dispose?) + // and some methods we need to implement in a more specific way (Equals, GetHashCode) + return; + } + var methAttributes = GetMethodAttrs(minfo); + var @params = (from paraminfo in minfo.GetParameters() select paraminfo.ParameterType).ToArray(); + MethodBuilder mbuilder = tb.DefineMethod(methodName, methAttributes, minfo.CallingConvention, minfo.ReturnType, @params); + GenerateExceptionILCode(mbuilder); + } + } + + class NotSerializableSerializer : ISerializationSurrogate + { + + public NotSerializableSerializer() + { + } + + public void GetObjectData(object obj, SerializationInfo info, StreamingContext context) + { + // This type is private to System.Runtime.Serialization. We get an + // object of this type when, amongst others, the type didn't exist (yet?) + // (dll not loaded, type was removed/renamed) when we previously + // deserialized the previous domain objects. Don't serialize this + // object. + if (obj.GetType().Name == "TypeLoadExceptionHolder") + { + obj = null!; + return; + } + + MaybeType type = obj.GetType(); + + var hasAttr = (from attr in obj.GetType().CustomAttributes select attr.AttributeType == typeof(PyNet_NotSerializedAttribute)).Count() != 0; + if (hasAttr) + { + // Don't serialize a _NotSerialized. Serialize the base type, and deserialize as a _NotSerialized + type = type.Value.BaseType; + obj = null!; + } + + info.AddValue("notSerialized_tp", type); + + } + + public object SetObjectData(object obj, SerializationInfo info, StreamingContext context, ISurrogateSelector selector) + { + if (info is null) + { + // `obj` is of type TypeLoadExceptionHolder. This means the type + // we're trying to load doesn't exist anymore or we haven't created + // it yet, and the runtime doesn't even gives us the chance to + // recover from this as info is null. We may even get objects + // this serializer did not serialize in a previous domain, + // like in the case of the "namespace_rename" domain reload + // test: the object successfully serialized, but it cannot be + // deserialized. + // just return null. + return null!; + } + + object nameObj = null!; + try + { + nameObj = info.GetValue($"notSerialized_tp", typeof(object)); + } + catch + { + // we didn't find the expected information. We don't know + // what to do with this; return null. + return null!; + } + Debug.Assert(nameObj.GetType() == typeof(MaybeType)); + MaybeType name = (MaybeType)nameObj; + Debug.Assert(name.Valid); + if (!name.Valid) + { + // The type couldn't be loaded + return null!; + } + + obj = NonSerializedTypeBuilder.CreateNewObject(name.Value); + return obj; + } + } + + class NonSerializableSelector : SurrogateSelector + { + public override ISerializationSurrogate? GetSurrogate (Type type, StreamingContext context, out ISurrogateSelector selector) + { + if (type is null) + { + throw new ArgumentNullException(); + } + if (type.IsSerializable) + { + selector = this; + return null; // use whichever default + } + else + { + selector = this; + return new NotSerializableSerializer(); + } + } + } + public static class RuntimeData { private static Type? _formatterType; @@ -47,6 +332,73 @@ static void ClearCLRData () } } + internal static void SerializeNonSerializableTypes () + { + // Serialize the Types (Type objects) that couldn't be (de)serialized. + // This needs to be done otherwise at deserialization time we get + // TypeLoadExceptionHolder objects and we can't even recover. + + // We don't serialize the "_NotSerialized" Types, we serialize their base + // to recreate the "_NotSerialized" versions on the next domain load. + + Dictionary invalidTypes = new(); + foreach(var tp in NonSerializedTypeBuilder.assemblyForNonSerializedClasses.GetTypes()) + { + invalidTypes[tp.FullName] = new MaybeType(tp.BaseType); + } + + // delete previous data if any + BorrowedReference oldCapsule = PySys_GetObject("clr_nonSerializedTypes"); + if (!oldCapsule.IsNull) + { + IntPtr oldData = PyCapsule_GetPointer(oldCapsule, IntPtr.Zero); + PyMem_Free(oldData); + PyCapsule_SetPointer(oldCapsule, IntPtr.Zero); + } + IFormatter formatter = CreateFormatter(); + var ms = new MemoryStream(); + formatter.Serialize(ms, invalidTypes); + + Debug.Assert(ms.Length <= int.MaxValue); + byte[] data = ms.GetBuffer(); + + IntPtr mem = PyMem_Malloc(ms.Length + IntPtr.Size); + Marshal.WriteIntPtr(mem, (IntPtr)ms.Length); + Marshal.Copy(data, 0, mem + IntPtr.Size, (int)ms.Length); + + using NewReference capsule = PyCapsule_New(mem, IntPtr.Zero, IntPtr.Zero); + int res = PySys_SetObject("clr_nonSerializedTypes", capsule.BorrowOrThrow()); + PythonException.ThrowIfIsNotZero(res); + + } + + internal static void DeserializeNonSerializableTypes () + { + BorrowedReference capsule = PySys_GetObject("clr_nonSerializedTypes"); + if (capsule.IsNull) + { + // nothing to do. + return; + } + // get the memory stream from the capsule. + IntPtr mem = PyCapsule_GetPointer(capsule, IntPtr.Zero); + int length = (int)Marshal.ReadIntPtr(mem); + byte[] data = new byte[length]; + Marshal.Copy(mem + IntPtr.Size, data, 0, length); + var ms = new MemoryStream(data); + var formatter = CreateFormatter(); + var storage = (Dictionary)formatter.Deserialize(ms); + foreach(var item in storage) + { + if(item.Value.Valid) + { + // recreate the "_NotSerialized" Types + NonSerializedTypeBuilder.CreateType(item.Value.Value); + } + } + + } + internal static void Stash() { var runtimeStorage = new PythonNetState @@ -74,6 +426,8 @@ internal static void Stash() using NewReference capsule = PyCapsule_New(mem, IntPtr.Zero, IntPtr.Zero); int res = PySys_SetObject("clr_data", capsule.BorrowOrThrow()); PythonException.ThrowIfIsNotZero(res); + SerializeNonSerializableTypes(); + } internal static void RestoreRuntimeData() @@ -90,6 +444,9 @@ internal static void RestoreRuntimeData() private static void RestoreRuntimeDataImpl() { + // The "_NotSerialized" Types must exist before the rest of the data + // is deserialized. + DeserializeNonSerializableTypes(); BorrowedReference capsule = PySys_GetObject("clr_data"); if (capsule.IsNull) { @@ -123,19 +480,6 @@ public static void ClearStash() PySys_SetObject("clr_data", default); } - static bool CheckSerializable (object o) - { - Type type = o.GetType(); - do - { - if (!type.IsSerializable) - { - return false; - } - } while ((type = type.BaseType) != null); - return true; - } - private static SharedObjectsState SaveRuntimeDataObjects() { var contexts = new Dictionary>(PythonReferenceComparer.Instance); @@ -150,7 +494,6 @@ private static SharedObjectsState SaveRuntimeDataObjects() foreach (var pyObj in extensions) { var extension = (ExtensionType)ManagedType.GetManagedObject(pyObj)!; - Debug.Assert(CheckSerializable(extension)); var context = extension.Save(pyObj); if (context is not null) { @@ -170,6 +513,7 @@ private static SharedObjectsState SaveRuntimeDataObjects() .ToList(); foreach (var pyObj in reflectedObjects) { + // Console.WriteLine($"saving object: {pyObj} {pyObj.rawPtr} "); // Wrapper must be the CLRObject var clrObj = (CLRObject)ManagedType.GetManagedObject(pyObj)!; object inst = clrObj.inst; @@ -199,10 +543,6 @@ private static SharedObjectsState SaveRuntimeDataObjects() { if (!item.Stored) { - if (!CheckSerializable(item.Instance)) - { - continue; - } var clrO = wrappers[item.Instance].First(); foreach (var @ref in item.PyRefs) { @@ -254,7 +594,11 @@ internal static IFormatter CreateFormatter() { return FormatterType != null ? (IFormatter)Activator.CreateInstance(FormatterType) - : new BinaryFormatter(); + : new BinaryFormatter() + { + SurrogateSelector = new NonSerializableSelector(), + // Binder = new CustomizedBinder() + }; } } } diff --git a/tests/domain_tests/TestRunner.cs b/tests/domain_tests/TestRunner.cs index 4f6a3ea28..0097ea202 100644 --- a/tests/domain_tests/TestRunner.cs +++ b/tests/domain_tests/TestRunner.cs @@ -1132,6 +1132,168 @@ import System ", }, + new TestCase + { + Name = "serialize_not_serializable", + DotNetBefore = @" + namespace TestNamespace + { + + public class NotSerializableTextWriter : System.IO.TextWriter + { + override public System.Text.Encoding Encoding { get { return System.Text.Encoding.ASCII;} } + } + + [System.Serializable] + public static class SerializableWriter + { + private static System.IO.TextWriter _writer = null; + + public static System.IO.TextWriter Writer {get { return _writer; }} + + public static void SetWriter() + { + _writer = System.IO.TextWriter.Synchronized(new NotSerializableTextWriter()); + } + } + }", + DotNetAfter = @" + namespace TestNamespace + { + + public class NotSerializableTextWriter : System.IO.TextWriter + { + override public System.Text.Encoding Encoding { get { return System.Text.Encoding.ASCII;} } + } + + [System.Serializable] + public static class SerializableWriter + { + private static System.IO.TextWriter _writer = null; + + public static System.IO.TextWriter Writer {get { return _writer; }} + + public static void SetWriter(System.IO.TextWriter w) + { + _writer = System.IO.TextWriter.Synchronized(w); + } + } + }", + PythonCode = @" +import clr +import sys +clr.AddReference('DomainTests') +import TestNamespace +import System + +def before_reload(): + + TestNamespace.SerializableWriter.SetWriter(); + sys.log_writer = TestNamespace.SerializableWriter.Writer + +def after_reload(): + + assert sys.log_writer is not None + try: + encoding = sys.log_writer.Write('baba') + except System.Runtime.Serialization.SerializationException: + pass + else: + raise AssertionError('Serialized non-serializable objects should be deserialized to throwing objects') +", + }, + new TestCase + { + Name = "serialize_not_serializable_interface", + DotNetBefore = @" + namespace TestNamespace + { + public interface MyInterface + { + int InterfaceMethod(); + } + + public class NotSerializableInterfaceImplement : MyInterface + { + public int value = -1; + int MyInterface.InterfaceMethod() + { + return value; + } + } + + [System.Serializable] + public static class SerializableWriter + { + private static MyInterface _iface = null; + + public static MyInterface Writer {get { return _iface; }} + + public static void SetInterface() + { + var temp = new NotSerializableInterfaceImplement(); + temp.value = 12315; + _iface = temp; + } + } + }", + DotNetAfter = @" + namespace TestNamespace + { + public interface MyInterface + { + int InterfaceMethod(); + } + + public class NotSerializableInterfaceImplement : MyInterface + { + public int value = -1; + int MyInterface.InterfaceMethod() + { + return value; + } + } + + [System.Serializable] + public static class SerializableWriter + { + private static MyInterface _iface = null; + + public static MyInterface Writer {get { return _iface; }} + + public static void SetInterface() + { + var temp = new NotSerializableInterfaceImplement(); + temp.value = 123124; + _iface = temp; + } + } + }", + PythonCode = @" +import clr +import sys +clr.AddReference('DomainTests') +import TestNamespace +import System + +def before_reload(): + + TestNamespace.SerializableWriter.SetInterface(); + sys.log_writer = TestNamespace.SerializableWriter.Writer + assert(sys.log_writer.InterfaceMethod() == 12315) + +def after_reload(): + + assert sys.log_writer is not None + try: + retcode = sys.log_writer.InterfaceMethod() + print(f'retcode of InterfaceMethod is {retcode}') + except System.Runtime.Serialization.SerializationException: + pass + else: + raise AssertionError('Serialized non-serializable objects should be deserialized to throwing objects') +", + }, }; /// diff --git a/tests/domain_tests/test_domain_reload.py b/tests/domain_tests/test_domain_reload.py index 8999e481b..bbff73493 100644 --- a/tests/domain_tests/test_domain_reload.py +++ b/tests/domain_tests/test_domain_reload.py @@ -1,6 +1,7 @@ import subprocess import os import platform +from unittest import skip import pytest @@ -88,3 +89,10 @@ def test_nested_type(): def test_import_after_reload(): _run_test("import_after_reload") + +def test_serialize_not_serializable(): + _run_test("serialize_not_serializable") + +@skip("interface methods cannot be overriden") +def test_serialize_not_serializable_interface(): + _run_test("serialize_not_serializable_interface") \ No newline at end of file From b59e2bb23bdc80d1ee6e25627f666f21854a3ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Thu, 30 Jun 2022 13:38:47 -0400 Subject: [PATCH 06/13] Add nested class support plus guard rails for private classes and review fixes --- src/runtime/StateSerialization/RuntimeData.cs | 152 ++++++++++++++---- 1 file changed, 124 insertions(+), 28 deletions(-) diff --git a/src/runtime/StateSerialization/RuntimeData.cs b/src/runtime/StateSerialization/RuntimeData.cs index cbcf160af..c66e27995 100644 --- a/src/runtime/StateSerialization/RuntimeData.cs +++ b/src/runtime/StateSerialization/RuntimeData.cs @@ -1,9 +1,6 @@ using System; -using System.Collections; using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Diagnostics; -using System.Dynamic; using System.IO; using System.Linq; using System.Reflection; @@ -45,38 +42,140 @@ internal static class NonSerializedTypeBuilder internal static HashSet dontReimplementMethods = new(){"Finalize", "Dispose", "GetType", "ReferenceEquals", "GetHashCode", "Equals"}; const string notSerializedSuffix = "_NotSerialized"; - public static object CreateNewObject(Type baseType) + private static Func hasVisibility = (tp, attr) => (tp.Attributes & TypeAttributes.VisibilityMask) == attr; + private static Func isNestedType = (tp) => hasVisibility(tp, TypeAttributes.NestedPrivate) || hasVisibility(tp, TypeAttributes.NestedPublic) || hasVisibility(tp, TypeAttributes.NestedFamily) || hasVisibility(tp, TypeAttributes.NestedAssembly); + private static Func isPrivateType = (tp) => hasVisibility(tp, TypeAttributes.NotPublic) || hasVisibility(tp, TypeAttributes.NestedPrivate) || hasVisibility(tp, TypeAttributes.NestedFamily) || hasVisibility( tp, TypeAttributes.NestedAssembly); + private static Func isPublicType = (tp) => hasVisibility(tp, TypeAttributes.Public) || hasVisibility(tp,TypeAttributes.NestedPublic); + + public static object? CreateNewObject(Type baseType) { var myType = CreateType(baseType); + if (myType is null) + { + return null; + } var myObject = Activator.CreateInstance(myType); return myObject; } - public static Type CreateType(Type tp) + static void FillTypeMethods(TypeBuilder tb) { - Type existingType = assemblyForNonSerializedClasses.GetType(tp.Name + "_NotSerialized", throwOnError:false); - if (existingType is not null) + var constructors = tb.BaseType.GetConstructors(); + if (constructors.Count() == 0) { - return existingType; + // no constructors defined, at least declare a default + ConstructorBuilder constructor = tb.DefineDefaultConstructor(MethodAttributes.Public | MethodAttributes.SpecialName | MethodAttributes.RTSpecialName); } + else + { + foreach (var ctor in constructors) + { + var ctorParams = (from param in ctor.GetParameters() select param.ParameterType).ToArray(); + var ctorbuilder = tb.DefineConstructor(ctor.Attributes, ctor.CallingConvention, ctorParams); + ctorbuilder.GetILGenerator().Emit(OpCodes.Ret); - TypeBuilder tb = GetTypeBuilder(tp); - ConstructorBuilder constructor = tb.DefineDefaultConstructor(MethodAttributes.Public | MethodAttributes.SpecialName | MethodAttributes.RTSpecialName); - var properties = tp.GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static); + } + var parameterless = tb.DefineConstructor(MethodAttributes.Public | MethodAttributes.SpecialName | MethodAttributes.RTSpecialName, CallingConventions.Standard | CallingConventions.HasThis, Type.EmptyTypes); + parameterless.GetILGenerator().Emit(OpCodes.Ret); + } + + var properties = tb.BaseType.GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static); foreach (var prop in properties) { CreateProperty(tb, prop); } - var methods = tp.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static); + var methods = tb.BaseType.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static); foreach (var meth in methods) { CreateMethod(tb, meth); } ImplementEqualityAndHash(tb); + } - return tb.CreateType(); + static string MakeName(Type tp) + { + const string suffix = "_NotSerialized"; + string @out = tp.Name + suffix; + var parentType = tp.DeclaringType; + while (parentType is not null) + { + // If we have a nested class, we need the whole nester/nestee + // chain with the suffix for each. + @out = parentType.Name + suffix + "+" + @out; + parentType = parentType.DeclaringType; + } + return @out; + } + + public static Type? CreateType(Type tp) + { + if (!isPublicType(tp)) + { + return null; + } + + Type existingType = assemblyForNonSerializedClasses.GetType(MakeName(tp), throwOnError:false); + if (existingType is not null) + { + return existingType; + } + var parentType = tp.DeclaringType; + if (parentType is not null) + { + // parent types for nested types must be created first. Climb up the + // declaring type chain until we find a "top-level" class. + while (parentType.DeclaringType is not null) + { + parentType = parentType.DeclaringType; + } + CreateTypeInternal(parentType); + Type nestedType = assemblyForNonSerializedClasses.GetType(MakeName(tp), throwOnError:true); + return nestedType; + } + return CreateTypeInternal(tp); + } + + private static Type? CreateTypeInternal(Type baseType) + { + if (!isPublicType(baseType)) + { + // we can't derive from non-public types. + return null; + } + Type existingType = assemblyForNonSerializedClasses.GetType(MakeName(baseType), throwOnError:false); + if (existingType is not null) + { + return existingType; + } + + TypeBuilder tb = GetTypeBuilder(baseType); + SetNonSerialiedAttr(tb); + FillTypeMethods(tb); + + var nestedtypes = baseType.GetNestedTypes(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static); + List nestedBuilders = new(); + foreach (var nested in nestedtypes) + { + if (isPrivateType(nested)) + { + continue; + } + var nestedBuilder = tb.DefineNestedType(nested.Name + notSerializedSuffix, + TypeAttributes.NestedPublic, + nested + ); + nestedBuilders.Add(nestedBuilder); + } + var outTp = tb.CreateType(); + foreach(var builder in nestedBuilders) + { + FillTypeMethods(builder); + SetNonSerialiedAttr(builder); + builder.CreateType(); + } + return outTp; } private static void ImplementEqualityAndHash(TypeBuilder tb) @@ -91,7 +190,7 @@ private static void ImplementEqualityAndHash(TypeBuilder tb) getHashIlGen.Emit(OpCodes.Ldarg_0); getHashIlGen.EmitCall(OpCodes.Call, typeof(object).GetMethod("GetType"), Type.EmptyTypes); getHashIlGen.EmitCall(OpCodes.Call, typeof(Type).GetProperty("Name").GetMethod, Type.EmptyTypes); - getHashIlGen.EmitCall(OpCodes.Call, typeof(string).GetMethod("GetHashCode"), Type.EmptyTypes); + getHashIlGen.EmitCall(OpCodes.Call, typeof(string).GetMethod("GetHashCode", Type.EmptyTypes), Type.EmptyTypes); getHashIlGen.Emit(OpCodes.Ret); Type[] equalsArgs = new Type[] {typeof(object), typeof(object)}; @@ -108,19 +207,20 @@ private static void ImplementEqualityAndHash(TypeBuilder tb) equalsIlGen.Emit(OpCodes.Ret); } + private static void SetNonSerialiedAttr(TypeBuilder tb) + { + ConstructorInfo attrCtorInfo = typeof(PyNet_NotSerializedAttribute).GetConstructor(new Type[]{}); + CustomAttributeBuilder attrBuilder = new CustomAttributeBuilder(attrCtorInfo,new object[]{}); + tb.SetCustomAttribute(attrBuilder); + } + private static TypeBuilder GetTypeBuilder(Type baseType) { string typeSignature = baseType.Name + notSerializedSuffix; - TypeBuilder tb = moduleBuilder.DefineType(typeSignature, baseType.Attributes, baseType, baseType.GetInterfaces()); - - ConstructorInfo attrCtorInfo = typeof(PyNet_NotSerializedAttribute).GetConstructor(new Type[]{}); - CustomAttributeBuilder attrBuilder = new CustomAttributeBuilder(attrCtorInfo,new object[]{}); - tb.SetCustomAttribute(attrBuilder); - return tb; } @@ -188,7 +288,6 @@ private static void CreateProperty(TypeBuilder tb, PropertyInfo pinfo) private static void CreateMethod(TypeBuilder tb, MethodInfo minfo) { - Console.WriteLine($"overimplementing method for: {minfo} {minfo.IsVirtual} {minfo.IsFinal} "); string methodName = minfo.Name; if (dontReimplementMethods.Contains(methodName)) @@ -226,8 +325,7 @@ public void GetObjectData(object obj, SerializationInfo info, StreamingContext c MaybeType type = obj.GetType(); - var hasAttr = (from attr in obj.GetType().CustomAttributes select attr.AttributeType == typeof(PyNet_NotSerializedAttribute)).Count() != 0; - if (hasAttr) + if (type.Value.CustomAttributes.Any((attr) => attr.AttributeType == typeof(NonSerializedAttribute))) { // Don't serialize a _NotSerialized. Serialize the base type, and deserialize as a _NotSerialized type = type.Value.BaseType; @@ -257,7 +355,7 @@ public object SetObjectData(object obj, SerializationInfo info, StreamingContext object nameObj = null!; try { - nameObj = info.GetValue($"notSerialized_tp", typeof(object)); + nameObj = info.GetValue("notSerialized_tp", typeof(object)); } catch { @@ -274,7 +372,7 @@ public object SetObjectData(object obj, SerializationInfo info, StreamingContext return null!; } - obj = NonSerializedTypeBuilder.CreateNewObject(name.Value); + obj = NonSerializedTypeBuilder.CreateNewObject(name.Value)!; return obj; } } @@ -287,14 +385,13 @@ class NonSerializableSelector : SurrogateSelector { throw new ArgumentNullException(); } + selector = this; if (type.IsSerializable) { - selector = this; return null; // use whichever default } else { - selector = this; return new NotSerializableSerializer(); } } @@ -597,7 +694,6 @@ internal static IFormatter CreateFormatter() : new BinaryFormatter() { SurrogateSelector = new NonSerializableSelector(), - // Binder = new CustomizedBinder() }; } } From 100210551cb733d9121abee0b6771efd8e87cacc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Thu, 30 Jun 2022 14:04:24 -0400 Subject: [PATCH 07/13] fixup! Add nested class support --- src/runtime/StateSerialization/RuntimeData.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/StateSerialization/RuntimeData.cs b/src/runtime/StateSerialization/RuntimeData.cs index c66e27995..57f895d4b 100644 --- a/src/runtime/StateSerialization/RuntimeData.cs +++ b/src/runtime/StateSerialization/RuntimeData.cs @@ -325,7 +325,7 @@ public void GetObjectData(object obj, SerializationInfo info, StreamingContext c MaybeType type = obj.GetType(); - if (type.Value.CustomAttributes.Any((attr) => attr.AttributeType == typeof(NonSerializedAttribute))) + if (type.Value.CustomAttributes.Any((attr) => attr.AttributeType == typeof(PyNet_NotSerializedAttribute))) { // Don't serialize a _NotSerialized. Serialize the base type, and deserialize as a _NotSerialized type = type.Value.BaseType; From d2f1ab61772f5026f9322fd65e6e6fc56a8f26b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Wed, 6 Jul 2022 14:57:21 -0400 Subject: [PATCH 08/13] Workaround for mono attribute bug --- src/runtime/StateSerialization/RuntimeData.cs | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/runtime/StateSerialization/RuntimeData.cs b/src/runtime/StateSerialization/RuntimeData.cs index 57f895d4b..ec7cfce58 100644 --- a/src/runtime/StateSerialization/RuntimeData.cs +++ b/src/runtime/StateSerialization/RuntimeData.cs @@ -23,13 +23,6 @@ private NotSerializedException(SerializationInfo info, StreamingContext context) override public void GetObjectData(SerializationInfo info, StreamingContext context) => base.GetObjectData(info, context); } - - // empty attribute to mark classes created by the "non-serializer" so we don't loop-inherit - // on multiple cycles of de/serialization - [System.AttributeUsage(System.AttributeTargets.All, Inherited = false, AllowMultiple = true)] - [System.Serializable] - sealed class PyNet_NotSerializedAttribute : System.Attribute {} - [Serializable] internal static class NonSerializedTypeBuilder { @@ -40,7 +33,12 @@ internal static class NonSerializedTypeBuilder AppDomain.CurrentDomain.DefineDynamicAssembly(nonSerializedAssemblyName, AssemblyBuilderAccess.Run); internal static ModuleBuilder moduleBuilder = assemblyForNonSerializedClasses.DefineDynamicModule("NotSerializedModule"); internal static HashSet dontReimplementMethods = new(){"Finalize", "Dispose", "GetType", "ReferenceEquals", "GetHashCode", "Equals"}; - const string notSerializedSuffix = "_NotSerialized"; + internal const string notSerializedSuffix = "_NotSerialized"; + // dummy field name to mark classes created by the "non-serializer" so we don't loop-inherit + // on multiple cycles of de/serialization. We use a static field instead of an attribute + // becaues of a bug in mono. Put a space in the name so users will be extremely unlikely + // to create a field with the same name. + internal const string notSerializedFieldName = "__PyNet NonSerialized"; private static Func hasVisibility = (tp, attr) => (tp.Attributes & TypeAttributes.VisibilityMask) == attr; private static Func isNestedType = (tp) => hasVisibility(tp, TypeAttributes.NestedPrivate) || hasVisibility(tp, TypeAttributes.NestedPublic) || hasVisibility(tp, TypeAttributes.NestedFamily) || hasVisibility(tp, TypeAttributes.NestedAssembly); @@ -96,14 +94,13 @@ static void FillTypeMethods(TypeBuilder tb) static string MakeName(Type tp) { - const string suffix = "_NotSerialized"; - string @out = tp.Name + suffix; + string @out = tp.Name + notSerializedSuffix; var parentType = tp.DeclaringType; while (parentType is not null) { // If we have a nested class, we need the whole nester/nestee // chain with the suffix for each. - @out = parentType.Name + suffix + "+" + @out; + @out = parentType.Name + notSerializedSuffix + "+" + @out; parentType = parentType.DeclaringType; } return @out; @@ -151,7 +148,7 @@ static string MakeName(Type tp) } TypeBuilder tb = GetTypeBuilder(baseType); - SetNonSerialiedAttr(tb); + SetNonSerializedAttr(tb); FillTypeMethods(tb); var nestedtypes = baseType.GetNestedTypes(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static); @@ -172,7 +169,7 @@ static string MakeName(Type tp) foreach(var builder in nestedBuilders) { FillTypeMethods(builder); - SetNonSerialiedAttr(builder); + SetNonSerializedAttr(builder); builder.CreateType(); } return outTp; @@ -207,11 +204,18 @@ private static void ImplementEqualityAndHash(TypeBuilder tb) equalsIlGen.Emit(OpCodes.Ret); } - private static void SetNonSerialiedAttr(TypeBuilder tb) + private static void SetNonSerializedAttr(TypeBuilder tb) { - ConstructorInfo attrCtorInfo = typeof(PyNet_NotSerializedAttribute).GetConstructor(new Type[]{}); - CustomAttributeBuilder attrBuilder = new CustomAttributeBuilder(attrCtorInfo,new object[]{}); - tb.SetCustomAttribute(attrBuilder); + // Name of the function says we're adding an attribute, but for some + // reason on Mono the attribute is not added, and no exceptions are + // thrown. + tb.DefineField(notSerializedFieldName, typeof(int), FieldAttributes.Public | FieldAttributes.Static); + } + + public static bool IsNonSerializedType(Type tp) + { + return tp.GetField(NonSerializedTypeBuilder.notSerializedFieldName, BindingFlags.Public | BindingFlags.Static) is not null; + } private static TypeBuilder GetTypeBuilder(Type baseType) @@ -325,7 +329,7 @@ public void GetObjectData(object obj, SerializationInfo info, StreamingContext c MaybeType type = obj.GetType(); - if (type.Value.CustomAttributes.Any((attr) => attr.AttributeType == typeof(PyNet_NotSerializedAttribute))) + if (NonSerializedTypeBuilder.IsNonSerializedType(type.Value)) { // Don't serialize a _NotSerialized. Serialize the base type, and deserialize as a _NotSerialized type = type.Value.BaseType; From 1f9d221bb50d18a71a1919b2e78f5c89e4fa280d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Thu, 7 Jul 2022 15:10:24 -0400 Subject: [PATCH 09/13] fixup! Workaround for mono attribute bug don't try to derive from sealed types --- src/runtime/StateSerialization/RuntimeData.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/runtime/StateSerialization/RuntimeData.cs b/src/runtime/StateSerialization/RuntimeData.cs index ec7cfce58..0fa484ada 100644 --- a/src/runtime/StateSerialization/RuntimeData.cs +++ b/src/runtime/StateSerialization/RuntimeData.cs @@ -44,6 +44,7 @@ internal static class NonSerializedTypeBuilder private static Func isNestedType = (tp) => hasVisibility(tp, TypeAttributes.NestedPrivate) || hasVisibility(tp, TypeAttributes.NestedPublic) || hasVisibility(tp, TypeAttributes.NestedFamily) || hasVisibility(tp, TypeAttributes.NestedAssembly); private static Func isPrivateType = (tp) => hasVisibility(tp, TypeAttributes.NotPublic) || hasVisibility(tp, TypeAttributes.NestedPrivate) || hasVisibility(tp, TypeAttributes.NestedFamily) || hasVisibility( tp, TypeAttributes.NestedAssembly); private static Func isPublicType = (tp) => hasVisibility(tp, TypeAttributes.Public) || hasVisibility(tp,TypeAttributes.NestedPublic); + private static Func CanCreateType = (tp) => isPublicType(tp) && ((tp.Attributes & TypeAttributes.Sealed) == 0); public static object? CreateNewObject(Type baseType) { @@ -108,7 +109,7 @@ static string MakeName(Type tp) public static Type? CreateType(Type tp) { - if (!isPublicType(tp)) + if (!CanCreateType(tp)) { return null; } From 34b7445c2195b98bb46166596f4dccf19eb012f4 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 30 Jun 2022 16:25:15 -0700 Subject: [PATCH 10/13] got rid of a few deprecation warnings that pollute GitHub code review --- src/embed_tests/TestConverter.cs | 9 ++++++--- src/embed_tests/TestDomainReload.cs | 3 +-- src/embed_tests/TestFinalizer.cs | 7 +++++-- src/embed_tests/TestNativeTypeOffset.cs | 3 ++- src/embed_tests/TestPythonException.cs | 2 +- src/runtime/InternString.cs | 4 ++-- src/runtime/PythonTypes/PyObject.cs | 4 +++- src/runtime/Types/ReflectedClrType.cs | 2 +- src/runtime/Util/PythonReferenceComparer.cs | 4 ++-- 9 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/embed_tests/TestConverter.cs b/src/embed_tests/TestConverter.cs index e586eda1b..0686d528b 100644 --- a/src/embed_tests/TestConverter.cs +++ b/src/embed_tests/TestConverter.cs @@ -148,7 +148,7 @@ public void PyIntImplicit() { var i = new PyInt(1); var ni = (PyObject)i.As(); - Assert.AreEqual(i.rawPtr, ni.rawPtr); + Assert.IsTrue(PythonReferenceComparer.Instance.Equals(i, ni)); } [Test] @@ -178,8 +178,11 @@ public void RawPyObjectProxy() var clrObject = (CLRObject)ManagedType.GetManagedObject(pyObjectProxy); Assert.AreSame(pyObject, clrObject.inst); - var proxiedHandle = pyObjectProxy.GetAttr("Handle").As(); - Assert.AreEqual(pyObject.Handle, proxiedHandle); +#pragma warning disable CS0612 // Type or member is obsolete + const string handlePropertyName = nameof(PyObject.Handle); +#pragma warning restore CS0612 // Type or member is obsolete + var proxiedHandle = pyObjectProxy.GetAttr(handlePropertyName).As(); + Assert.AreEqual(pyObject.DangerousGetAddressOrNull(), proxiedHandle); } // regression for https://github.com/pythonnet/pythonnet/issues/451 diff --git a/src/embed_tests/TestDomainReload.cs b/src/embed_tests/TestDomainReload.cs index 498119d1e..a0f9b63eb 100644 --- a/src/embed_tests/TestDomainReload.cs +++ b/src/embed_tests/TestDomainReload.cs @@ -99,8 +99,7 @@ from Python.EmbeddingTest.Domain import MyClass { Debug.Assert(obj.AsManagedObject(type).GetType() == type); // We only needs its Python handle - PyRuntime.XIncref(obj); - return obj.Handle; + return new NewReference(obj).DangerousMoveToPointer(); } } } diff --git a/src/embed_tests/TestFinalizer.cs b/src/embed_tests/TestFinalizer.cs index 40ab03395..b748a2244 100644 --- a/src/embed_tests/TestFinalizer.cs +++ b/src/embed_tests/TestFinalizer.cs @@ -212,7 +212,9 @@ public void ValidateRefCount() Assert.AreEqual(ptr, e.Handle); Assert.AreEqual(2, e.ImpactedObjects.Count); // Fix for this test, don't do this on general environment +#pragma warning disable CS0618 // Type or member is obsolete Runtime.Runtime.XIncref(e.Reference); +#pragma warning restore CS0618 // Type or member is obsolete return false; }; Finalizer.Instance.IncorrectRefCntResolver += handler; @@ -234,8 +236,9 @@ private static IntPtr CreateStringGarbage() { PyString s1 = new PyString("test_string"); // s2 steal a reference from s1 - PyString s2 = new PyString(StolenReference.DangerousFromPointer(s1.Handle)); - return s1.Handle; + IntPtr address = s1.Reference.DangerousGetAddress(); + PyString s2 = new (StolenReference.DangerousFromPointer(address)); + return address; } } } diff --git a/src/embed_tests/TestNativeTypeOffset.cs b/src/embed_tests/TestNativeTypeOffset.cs index 2d31fe506..d692c24e6 100644 --- a/src/embed_tests/TestNativeTypeOffset.cs +++ b/src/embed_tests/TestNativeTypeOffset.cs @@ -33,7 +33,8 @@ public void LoadNativeTypeOffsetClass() { PyObject sys = Py.Import("sys"); // We can safely ignore the "m" abi flag - var abiflags = sys.GetAttr("abiflags", "".ToPython()).ToString().Replace("m", ""); + var abiflags = sys.HasAttr("abiflags") ? sys.GetAttr("abiflags").ToString() : ""; + abiflags = abiflags.Replace("m", ""); if (!string.IsNullOrEmpty(abiflags)) { string typeName = "Python.Runtime.NativeTypeOffset, Python.Runtime"; diff --git a/src/embed_tests/TestPythonException.cs b/src/embed_tests/TestPythonException.cs index a7cf05c83..a248b6a1f 100644 --- a/src/embed_tests/TestPythonException.cs +++ b/src/embed_tests/TestPythonException.cs @@ -161,7 +161,7 @@ def __init__(self, val): using var tbObj = tbPtr.MoveToPyObject(); // the type returned from PyErr_NormalizeException should not be the same type since a new // exception was raised by initializing the exception - Assert.AreNotEqual(type.Handle, typeObj.Handle); + Assert.IsFalse(PythonReferenceComparer.Instance.Equals(type, typeObj)); // the message should now be the string from the throw exception during normalization Assert.AreEqual("invalid literal for int() with base 10: 'dummy string'", strObj.ToString()); } diff --git a/src/runtime/InternString.cs b/src/runtime/InternString.cs index b6d9a0e4a..decb3981d 100644 --- a/src/runtime/InternString.cs +++ b/src/runtime/InternString.cs @@ -42,7 +42,7 @@ public static void Initialize() Debug.Assert(name == op.As()); SetIntern(name, op); var field = type.GetField("f" + name, PyIdentifierFieldFlags)!; - field.SetValue(null, op.rawPtr); + field.SetValue(null, op.DangerousGetAddressOrNull()); } } @@ -76,7 +76,7 @@ public static bool TryGetInterned(BorrowedReference op, out string s) private static void SetIntern(string s, PyString op) { _string2interns.Add(s, op); - _intern2strings.Add(op.rawPtr, s); + _intern2strings.Add(op.Reference.DangerousGetAddress(), s); } } } diff --git a/src/runtime/PythonTypes/PyObject.cs b/src/runtime/PythonTypes/PyObject.cs index 3d48e22ed..ce86753eb 100644 --- a/src/runtime/PythonTypes/PyObject.cs +++ b/src/runtime/PythonTypes/PyObject.cs @@ -27,7 +27,7 @@ public partial class PyObject : DynamicObject, IDisposable, ISerializable public StackTrace Traceback { get; } = new StackTrace(1); #endif - protected internal IntPtr rawPtr = IntPtr.Zero; + protected IntPtr rawPtr = IntPtr.Zero; internal readonly int run = Runtime.GetRun(); internal BorrowedReference obj => new (rawPtr); @@ -252,6 +252,8 @@ internal void Leak() rawPtr = IntPtr.Zero; } + internal IntPtr DangerousGetAddressOrNull() => rawPtr; + internal void CheckRun() { if (run != Runtime.GetRun()) diff --git a/src/runtime/Types/ReflectedClrType.cs b/src/runtime/Types/ReflectedClrType.cs index b787939be..d3d89bdb8 100644 --- a/src/runtime/Types/ReflectedClrType.cs +++ b/src/runtime/Types/ReflectedClrType.cs @@ -116,6 +116,6 @@ static ReflectedClrType AllocateClass(Type clrType) return new ReflectedClrType(type.Steal()); } - public override bool Equals(PyObject? other) => other != null && rawPtr == other.rawPtr; + public override bool Equals(PyObject? other) => rawPtr == other?.DangerousGetAddressOrNull(); public override int GetHashCode() => rawPtr.GetHashCode(); } diff --git a/src/runtime/Util/PythonReferenceComparer.cs b/src/runtime/Util/PythonReferenceComparer.cs index dd78f912d..63c35df57 100644 --- a/src/runtime/Util/PythonReferenceComparer.cs +++ b/src/runtime/Util/PythonReferenceComparer.cs @@ -13,10 +13,10 @@ public sealed class PythonReferenceComparer : IEqualityComparer public static PythonReferenceComparer Instance { get; } = new PythonReferenceComparer(); public bool Equals(PyObject? x, PyObject? y) { - return x?.rawPtr == y?.rawPtr; + return x?.DangerousGetAddressOrNull() == y?.DangerousGetAddressOrNull(); } - public int GetHashCode(PyObject obj) => obj.rawPtr.GetHashCode(); + public int GetHashCode(PyObject obj) => obj.DangerousGetAddressOrNull().GetHashCode(); private PythonReferenceComparer() { } } From e5546cb5ce2ee16d67cddc9d10c58194255dab0b Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Wed, 6 Jul 2022 08:32:51 +0200 Subject: [PATCH 11/13] Ensure that version.txt is always read from repo root Allows the project to be referenced in other .NET projects without adjusting its project file (#1853). --- Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Build.props b/Directory.Build.props index 8c5b53685..965610f91 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -6,7 +6,7 @@ Python.NET 10.0 false - $([System.IO.File]::ReadAllText("version.txt")) + $([System.IO.File]::ReadAllText($(MSBuildThisFileDirectory)version.txt)) $(FullVersion.Split('-', 2)[0]) $(FullVersion.Split('-', 2)[1]) From 7ffad42406e36cda9e4be7eb4ad533e45502a60d Mon Sep 17 00:00:00 2001 From: Tim Gates Date: Sun, 17 Jul 2022 09:09:05 +1000 Subject: [PATCH 12/13] docs: Fix a few typos There are small typos in: - pythonnet/__init__.py - tests/test_import.py Fixes: - Should read `splitted` rather than `splited`. - Should read `loaded` rather than `laoded`. Signed-off-by: Tim Gates --- pythonnet/__init__.py | 2 +- tests/test_import.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pythonnet/__init__.py b/pythonnet/__init__.py index 9876a0bec..8f3478713 100644 --- a/pythonnet/__init__.py +++ b/pythonnet/__init__.py @@ -121,7 +121,7 @@ def load( def unload() -> None: - """Explicitly unload a laoded runtime and shut down Python.NET""" + """Explicitly unload a loaded runtime and shut down Python.NET""" global _RUNTIME, _LOADER_ASSEMBLY if _LOADER_ASSEMBLY is not None: diff --git a/tests/test_import.py b/tests/test_import.py index 25877be15..877eacd84 100644 --- a/tests/test_import.py +++ b/tests/test_import.py @@ -15,7 +15,7 @@ def test_relative_missing_import(): def test_import_all_on_second_time(): """Test import all attributes after a normal import without '*'. - Due to import * only allowed at module level, the test body splited + Due to import * only allowed at module level, the test body splitted to a module file.""" from . import importtest del sys.modules[importtest.__name__] From 0cc7069e550eeec74b05153b33816a1ec37a3493 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Bourbonnais?= Date: Fri, 26 May 2023 11:56:10 -0400 Subject: [PATCH 13/13] adjustment to make the tests pass latest clr_loader version breaks the python tests otherwise --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 5ee89d3b7..ba84e7ef4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ license = {text = "MIT"} readme = "README.rst" dependencies = [ - "clr_loader>=0.1.7" + "clr_loader==0.1.7" ] classifiers = [