Merge pull request #1400 from losttech/cleanup/21-02-25

lostmsu · web-flow · commit f2dc8f19c1dc · 2021-02-27T12:52:32.000-08:00
ImportHook cleanup + PyObject.Length exception
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -36,6 +36,7 @@ when .NET expects an integer [#1342][i1342]
 -   BREAKING: Methods with `ref` or `out` parameters and void return type return a tuple of only the `ref` and `out` parameters.
 -   BREAKING: to call Python from .NET `Runtime.PythonDLL` property must be set to Python DLL name
 or the DLL must be loaded in advance. This must be done before calling any other Python.NET functions.
+-   BREAKING: `PyObject.Length()` now raises a `PythonException` when object does not support a concept of length.
 -   Sign Runtime DLL with a strong name
 -   Implement loading through `clr_loader` instead of the included `ClrModule`, enables
     support for .NET Core
diff --git a/src/embed_tests/pyimport.cs b/src/embed_tests/pyimport.cs
@@ -34,7 +34,9 @@ public void SetUp()
             TestContext.Out.WriteLine(testPath);
 
             IntPtr str = Runtime.Runtime.PyString_FromString(testPath);
+            Assert.IsFalse(str == IntPtr.Zero);
             BorrowedReference path = Runtime.Runtime.PySys_GetObject("path");
+            Assert.IsFalse(path.IsNull);
             Runtime.Runtime.PyList_Append(path, str);
             Runtime.Runtime.XDecref(str);
         }
diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs
@@ -61,6 +61,13 @@ 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);
@@ -88,7 +95,7 @@ internal static unsafe void Initialize()
 
             // both dicts are borrowed references
             BorrowedReference mod_dict = Runtime.PyModule_GetDict(ClrModuleReference);
-            BorrowedReference clr_dict = *Runtime._PyObject_GetDictPtr(root.ObjectReference);
+            using var clr_dict = Runtime.PyObject_GenericGetDict(root.ObjectReference);
 
             Runtime.PyDict_Update(mod_dict, clr_dict);
             BorrowedReference dict = Runtime.PyImport_GetModuleDict();
@@ -150,8 +157,10 @@ public static unsafe NewReference GetCLRModule(BorrowedReference fromList = defa
             // update the module dictionary with the contents of the root dictionary
             root.LoadNames();
             BorrowedReference py_mod_dict = Runtime.PyModule_GetDict(ClrModuleReference);
-            BorrowedReference clr_dict = *Runtime._PyObject_GetDictPtr(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);
+            }
 
             // find any items from the from list and get them from the root if they're not
             // already in the module dictionary
@@ -250,7 +259,6 @@ public static IntPtr __import__(IntPtr self, IntPtr argsRaw, IntPtr kw)
             }
 
             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
@@ -308,10 +316,7 @@ public static IntPtr __import__(IntPtr self, IntPtr argsRaw, IntPtr kw)
                     }
                     return new NewReference(module).DangerousMoveToPointer();
                 }
-                if (clr_prefix != null)
-                {
-                    return GetCLRModule(fromList).DangerousMoveToPointerOrNull();
-                }
+
                 module = Runtime.PyDict_GetItemString(modules, names[0]);
                 return new NewReference(module, canBeNull: true).DangerousMoveToPointer();
             }
@@ -351,12 +356,6 @@ public static IntPtr __import__(IntPtr self, IntPtr argsRaw, IntPtr kw)
 
                 // Add the module to sys.modules
                 Runtime.PyDict_SetItemString(modules, tail.moduleName, tail.ObjectReference);
-
-                // If imported from CLR add clr.<modulename> to sys.modules as well
-                if (clr_prefix != null)
-                {
-                    Runtime.PyDict_SetItemString(modules, clr_prefix + tail.moduleName, tail.ObjectReference);
-                }
             }
 
             {
@@ -374,6 +373,8 @@ public static IntPtr __import__(IntPtr self, IntPtr argsRaw, IntPtr kw)
 
         private static bool IsLoadAll(BorrowedReference fromList)
         {
+            if (fromList == null) throw new ArgumentNullException(nameof(fromList));
+
             if (CLRModule.preload)
             {
                 return false;
diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs
@@ -615,19 +615,15 @@ public virtual void DelItem(int index)
 
 
         /// <summary>
-        /// Length Method
-        /// </summary>
-        /// <remarks>
         /// Returns the length for objects that support the Python sequence
-        /// protocol, or 0 if the object does not support the protocol.
-        /// </remarks>
+        /// protocol.
+        /// </summary>
         public virtual long Length()
         {
-            var s = Runtime.PyObject_Size(obj);
+            var s = Runtime.PyObject_Size(Reference);
             if (s < 0)
             {
-                Runtime.PyErr_Clear();
-                return 0;
+                throw new PythonException();
             }
             return s;
         }
diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs
@@ -1121,13 +1121,7 @@ internal static int PyObject_Compare(IntPtr value1, IntPtr value2)
 
         internal static int PyObject_Not(IntPtr pointer) => Delegates.PyObject_Not(pointer);
 
-        internal static long PyObject_Size(IntPtr pointer)
-        {
-            return (long)_PyObject_Size(pointer);
-        }
-
-
-        private static IntPtr _PyObject_Size(IntPtr pointer) => Delegates._PyObject_Size(pointer);
+        internal static nint PyObject_Size(BorrowedReference pointer) => Delegates.PyObject_Size(pointer);
 
 
         internal static nint PyObject_Hash(IntPtr op) => Delegates.PyObject_Hash(op);
@@ -2028,9 +2022,8 @@ internal static IntPtr PyType_GenericAlloc(IntPtr type, long n)
 
         internal static int PyObject_GenericSetAttr(IntPtr obj, IntPtr name, IntPtr value) => Delegates.PyObject_GenericSetAttr(obj, name, value);
 
-
-        internal static BorrowedReference* _PyObject_GetDictPtr(BorrowedReference obj) => Delegates._PyObject_GetDictPtr(obj);
-
+        internal static NewReference PyObject_GenericGetDict(BorrowedReference o) => PyObject_GenericGetDict(o, IntPtr.Zero);
+        internal static NewReference PyObject_GenericGetDict(BorrowedReference o, IntPtr context) => Delegates.PyObject_GenericGetDict(o, context);
 
         internal static void PyObject_GC_Del(IntPtr tp) => Delegates.PyObject_GC_Del(tp);
 
@@ -2323,7 +2316,7 @@ static Delegates()
                 PyCallable_Check = (delegate* unmanaged[Cdecl]<IntPtr, int>)GetFunctionByName(nameof(PyCallable_Check), GetUnmanagedDll(_PythonDll));
                 PyObject_IsTrue = (delegate* unmanaged[Cdecl]<BorrowedReference, int>)GetFunctionByName(nameof(PyObject_IsTrue), GetUnmanagedDll(_PythonDll));
                 PyObject_Not = (delegate* unmanaged[Cdecl]<IntPtr, int>)GetFunctionByName(nameof(PyObject_Not), GetUnmanagedDll(_PythonDll));
-                _PyObject_Size = (delegate* unmanaged[Cdecl]<IntPtr, IntPtr>)GetFunctionByName("PyObject_Size", GetUnmanagedDll(_PythonDll));
+                PyObject_Size = (delegate* unmanaged[Cdecl]<BorrowedReference, nint>)GetFunctionByName("PyObject_Size", GetUnmanagedDll(_PythonDll));
                 PyObject_Hash = (delegate* unmanaged[Cdecl]<IntPtr, IntPtr>)GetFunctionByName(nameof(PyObject_Hash), GetUnmanagedDll(_PythonDll));
                 PyObject_Repr = (delegate* unmanaged[Cdecl]<IntPtr, IntPtr>)GetFunctionByName(nameof(PyObject_Repr), GetUnmanagedDll(_PythonDll));
                 PyObject_Str = (delegate* unmanaged[Cdecl]<IntPtr, IntPtr>)GetFunctionByName(nameof(PyObject_Str), GetUnmanagedDll(_PythonDll));
@@ -2478,8 +2471,8 @@ static Delegates()
                 PyType_Ready = (delegate* unmanaged[Cdecl]<IntPtr, int>)GetFunctionByName(nameof(PyType_Ready), GetUnmanagedDll(_PythonDll));
                 _PyType_Lookup = (delegate* unmanaged[Cdecl]<IntPtr, IntPtr, IntPtr>)GetFunctionByName(nameof(_PyType_Lookup), GetUnmanagedDll(_PythonDll));
                 PyObject_GenericGetAttr = (delegate* unmanaged[Cdecl]<IntPtr, IntPtr, IntPtr>)GetFunctionByName(nameof(PyObject_GenericGetAttr), GetUnmanagedDll(_PythonDll));
+                PyObject_GenericGetDict = (delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, NewReference>)GetFunctionByName(nameof(PyObject_GenericGetDict), GetUnmanagedDll(PythonDLL));
                 PyObject_GenericSetAttr = (delegate* unmanaged[Cdecl]<IntPtr, IntPtr, IntPtr, int>)GetFunctionByName(nameof(PyObject_GenericSetAttr), GetUnmanagedDll(_PythonDll));
-                _PyObject_GetDictPtr = (delegate* unmanaged[Cdecl]<BorrowedReference, BorrowedReference*>)GetFunctionByName(nameof(_PyObject_GetDictPtr), GetUnmanagedDll(_PythonDll));
                 PyObject_GC_Del = (delegate* unmanaged[Cdecl]<IntPtr, void>)GetFunctionByName(nameof(PyObject_GC_Del), GetUnmanagedDll(_PythonDll));
                 PyObject_GC_Track = (delegate* unmanaged[Cdecl]<IntPtr, void>)GetFunctionByName(nameof(PyObject_GC_Track), GetUnmanagedDll(_PythonDll));
                 PyObject_GC_UnTrack = (delegate* unmanaged[Cdecl]<IntPtr, void>)GetFunctionByName(nameof(PyObject_GC_UnTrack), GetUnmanagedDll(_PythonDll));
@@ -2595,7 +2588,7 @@ static Delegates()
             internal static delegate* unmanaged[Cdecl]<IntPtr, int> PyCallable_Check { get; }
             internal static delegate* unmanaged[Cdecl]<BorrowedReference, int> PyObject_IsTrue { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, int> PyObject_Not { get; }
-            internal static delegate* unmanaged[Cdecl]<IntPtr, IntPtr> _PyObject_Size { get; }
+            internal static delegate* unmanaged[Cdecl]<BorrowedReference, nint> PyObject_Size { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, IntPtr> PyObject_Hash { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, IntPtr> PyObject_Repr { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, IntPtr> PyObject_Str { get; }
@@ -2744,7 +2737,6 @@ static Delegates()
             internal static delegate* unmanaged[Cdecl]<IntPtr, IntPtr, IntPtr> _PyType_Lookup { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, IntPtr, IntPtr> PyObject_GenericGetAttr { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, IntPtr, IntPtr, int> PyObject_GenericSetAttr { get; }
-            internal static delegate* unmanaged[Cdecl]<BorrowedReference, BorrowedReference*> _PyObject_GetDictPtr { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, void> PyObject_GC_Del { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, void> PyObject_GC_Track { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, void> PyObject_GC_UnTrack { get; }
@@ -2781,6 +2773,7 @@ static Delegates()
             internal static delegate* unmanaged[Cdecl]<IntPtr, IntPtr, void> PyException_SetCause { get; }
             internal static delegate* unmanaged[Cdecl]<uint, IntPtr, int> PyThreadState_SetAsyncExcLLP64 { get; }
             internal static delegate* unmanaged[Cdecl]<ulong, IntPtr, int> PyThreadState_SetAsyncExcLP64 { get; }
+            internal static delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, NewReference> PyObject_GenericGetDict { get; }
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,9 @@ public void SetUp()`
`34`	`34`	`TestContext.Out.WriteLine(testPath);`
`35`	`35`
`36`	`36`	`IntPtr str = Runtime.Runtime.PyString_FromString(testPath);`
	`37`	`+ Assert.IsFalse(str == IntPtr.Zero);`
`37`	`38`	`BorrowedReference path = Runtime.Runtime.PySys_GetObject("path");`
	`39`	`+ Assert.IsFalse(path.IsNull);`
`38`	`40`	`Runtime.Runtime.PyList_Append(path, str);`
`39`	`41`	`Runtime.Runtime.XDecref(str);`
`40`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -615,19 +615,15 @@ public virtual void DelItem(int index)`
`615`	`615`
`616`	`616`
`617`	`617`	`/// <summary>`
`618`		`- /// Length Method`
`619`		`- /// </summary>`
`620`		`- /// <remarks>`
`621`	`618`	`/// Returns the length for objects that support the Python sequence`
`622`		`- /// protocol, or 0 if the object does not support the protocol.`
`623`		`- /// </remarks>`
	`619`	`+ /// protocol.`
	`620`	`+ /// </summary>`
`624`	`621`	`public virtual long Length()`
`625`	`622`	`{`
`626`		`- var s = Runtime.PyObject_Size(obj);`
	`623`	`+ var s = Runtime.PyObject_Size(Reference);`
`627`	`624`	`if (s < 0)`
`628`	`625`	`{`
`629`		`- Runtime.PyErr_Clear();`
`630`		`- return 0;`
	`626`	`+ throw new PythonException();`
`631`	`627`	`}`
`632`	`628`	`return s;`
`633`	`629`	`}`