From 08ea6f3c56086f661ba3b0d24daa481509c0be1f Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Thu, 17 Dec 2020 15:55:36 -0800 Subject: [PATCH 1/4] refactoring in CreateSubType --- src/runtime/converter.cs | 4 ++- src/runtime/runtime.cs | 2 ++ src/runtime/typemanager.cs | 51 ++++++++++++++++---------------------- 3 files changed, 26 insertions(+), 31 deletions(-) diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index 98fe99141..2f3810c58 100644 --- a/src/runtime/converter.cs +++ b/src/runtime/converter.cs @@ -313,7 +313,9 @@ internal static bool ToManaged(IntPtr value, Type type, return Converter.ToManagedValue(value, type, out result, setError); } - + internal static bool ToManagedValue(BorrowedReference value, Type obType, + out object result, bool setError) + => ToManagedValue(value.DangerousGetAddress(), obType, out result, setError); internal static bool ToManagedValue(IntPtr value, Type obType, out object result, bool setError) { diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 4cb8db71f..374e88cf6 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -1641,6 +1641,8 @@ internal static bool PyDict_Check(IntPtr ob) /// [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern IntPtr PyDict_GetItem(IntPtr pointer, IntPtr key); + [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] + internal static extern BorrowedReference PyDict_GetItemWithError(BorrowedReference pointer, BorrowedReference key); /// /// Return value: Borrowed reference. diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index 89ccdea4c..7b4cbc09a 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -6,6 +6,7 @@ using System.Runtime.InteropServices; using System.Diagnostics; using Python.Runtime.Slots; +using static Python.Runtime.PythonException; namespace Python.Runtime { @@ -302,6 +303,7 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType) internal static IntPtr CreateSubType(IntPtr py_name, IntPtr py_base_type, IntPtr py_dict) { + var dictRef = new BorrowedReference(py_dict); // Utility to create a subtype of a managed type with the ability for the // a python subtype able to override the managed implementation string name = Runtime.GetManagedString(py_name); @@ -311,40 +313,29 @@ internal static IntPtr CreateSubType(IntPtr py_name, IntPtr py_base_type, IntPtr object assembly = null; object namespaceStr = null; - var disposeList = new List(); - try + using (var assemblyKey = new PyString("__assembly__")) { - var assemblyKey = new PyObject(Converter.ToPython("__assembly__", typeof(string))); - disposeList.Add(assemblyKey); - if (0 != Runtime.PyMapping_HasKey(py_dict, assemblyKey.Handle)) + var assemblyPtr = Runtime.PyDict_GetItemWithError(dictRef, assemblyKey.Reference); + if (assemblyPtr.IsNull) { - var pyAssembly = new PyObject(Runtime.PyDict_GetItem(py_dict, assemblyKey.Handle)); - Runtime.XIncref(pyAssembly.Handle); - disposeList.Add(pyAssembly); - if (!Converter.ToManagedValue(pyAssembly.Handle, typeof(string), out assembly, false)) - { - throw new InvalidCastException("Couldn't convert __assembly__ value to string"); - } + if (Exceptions.ErrorOccurred()) return IntPtr.Zero; + } + else if (!Converter.ToManagedValue(assemblyPtr, typeof(string), out assembly, false)) + { + return Exceptions.RaiseTypeError("Couldn't convert __assembly__ value to string"); } - var namespaceKey = new PyObject(Converter.ToPythonImplicit("__namespace__")); - disposeList.Add(namespaceKey); - if (0 != Runtime.PyMapping_HasKey(py_dict, namespaceKey.Handle)) + using (var namespaceKey = new PyString("__namespace__")) { - var pyNamespace = new PyObject(Runtime.PyDict_GetItem(py_dict, namespaceKey.Handle)); - Runtime.XIncref(pyNamespace.Handle); - disposeList.Add(pyNamespace); - if (!Converter.ToManagedValue(pyNamespace.Handle, typeof(string), out namespaceStr, false)) + var pyNamespace = Runtime.PyDict_GetItemWithError(dictRef, namespaceKey.Reference); + if (pyNamespace.IsNull) { - throw new InvalidCastException("Couldn't convert __namespace__ value to string"); + if (Exceptions.ErrorOccurred()) return IntPtr.Zero; + } + else if (!Converter.ToManagedValue(pyNamespace, typeof(string), out namespaceStr, false)) + { + return Exceptions.RaiseTypeError("Couldn't convert __namespace__ value to string"); } - } - } - finally - { - foreach (PyObject o in disposeList) - { - o.Dispose(); } } @@ -370,14 +361,14 @@ internal static IntPtr CreateSubType(IntPtr py_name, IntPtr py_base_type, IntPtr // by default the class dict will have all the C# methods in it, but as this is a // derived class we want the python overrides in there instead if they exist. IntPtr cls_dict = Marshal.ReadIntPtr(py_type, TypeOffset.tp_dict); - Runtime.PyDict_Update(cls_dict, py_dict); + ThrowIfIsNotZero(Runtime.PyDict_Update(cls_dict, py_dict)); Runtime.XIncref(py_type); // Update the __classcell__ if it exists var cell = new BorrowedReference(Runtime.PyDict_GetItemString(cls_dict, "__classcell__")); if (!cell.IsNull) { - Runtime.PyCell_Set(cell, py_type); - Runtime.PyDict_DelItemString(cls_dict, "__classcell__"); + ThrowIfIsNotZero(Runtime.PyCell_Set(cell, py_type)); + ThrowIfIsNotZero(Runtime.PyDict_DelItemString(cls_dict, "__classcell__")); } return py_type; From 9682dc6e3b3f9231d7a801ae3202a65ed68fbcca Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Thu, 17 Dec 2020 19:46:00 -0800 Subject: [PATCH 2/4] allocate space for GCHandle in instances of CLR Metatype (which are types themselves) --- src/runtime/typemanager.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index 7b4cbc09a..49a46cb72 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -143,7 +143,7 @@ internal static IntPtr GetTypeHandle(ManagedType obj, Type type) /// internal static IntPtr CreateType(Type impl) { - IntPtr type = AllocateTypeObject(impl.Name); + IntPtr type = AllocateTypeObject(impl.Name, metatype: Runtime.PyTypeType); int ob_size = ObjectOffset.Size(type); // Set tp_basicsize to the size of our managed instance objects. @@ -212,7 +212,7 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType) base_ = bc.pyHandle; } - IntPtr type = AllocateTypeObject(name); + IntPtr type = AllocateTypeObject(name, Runtime.PyCLRMetaType); Marshal.WriteIntPtr(type, TypeOffset.ob_type, Runtime.PyCLRMetaType); Runtime.XIncref(Runtime.PyCLRMetaType); @@ -427,12 +427,15 @@ internal static IntPtr CreateMetaType(Type impl, out SlotsHolder slotsHolder) // the standard type slots, and has to subclass PyType_Type for // certain functions in the C runtime to work correctly with it. - IntPtr type = AllocateTypeObject("CLR Metatype"); - IntPtr py_type = Runtime.PyTypeType; + IntPtr type = AllocateTypeObject("CLR Metatype", metatype: Runtime.PyTypeType); + IntPtr py_type = Runtime.PyTypeType; Marshal.WriteIntPtr(type, TypeOffset.tp_base, py_type); Runtime.XIncref(py_type); + int size = TypeOffset.magic() + IntPtr.Size; + Marshal.WriteIntPtr(type, TypeOffset.tp_basicsize, new IntPtr(size)); + const int flags = TypeFlags.Default | TypeFlags.Managed | TypeFlags.HeapType @@ -522,7 +525,7 @@ internal static IntPtr BasicSubType(string name, IntPtr base_, Type impl) // Utility to create a subtype of a std Python type, but with // a managed type able to override implementation - IntPtr type = AllocateTypeObject(name); + IntPtr type = AllocateTypeObject(name, metatype: Runtime.PyTypeType); //Marshal.WriteIntPtr(type, TypeOffset.tp_basicsize, (IntPtr)obSize); //Marshal.WriteIntPtr(type, TypeOffset.tp_itemsize, IntPtr.Zero); @@ -564,9 +567,9 @@ internal static IntPtr BasicSubType(string name, IntPtr base_, Type impl) /// /// Utility method to allocate a type object & do basic initialization. /// - internal static IntPtr AllocateTypeObject(string name) + internal static IntPtr AllocateTypeObject(string name, IntPtr metatype) { - IntPtr type = Runtime.PyType_GenericAlloc(Runtime.PyTypeType, 0); + IntPtr type = Runtime.PyType_GenericAlloc(metatype, 0); // Clr type would not use __slots__, // and the PyMemberDef after PyHeapTypeObject will have other uses(e.g. type handle), // thus set the ob_size to 0 for avoiding slots iterations. From 38b3f01ad70f5d622788f28dd45af572478f14d4 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Thu, 17 Dec 2020 22:29:47 -0800 Subject: [PATCH 3/4] classderived: handle tp_dealloc called after tp_clear Because tp_clear sets tpHandle to NULL, it can't be used. Fortunately, we can simply read object's type from pyHandle. --- src/runtime/classderived.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs index e55e89240..dad9b039d 100644 --- a/src/runtime/classderived.cs +++ b/src/runtime/classderived.cs @@ -75,7 +75,8 @@ internal ClassDerivedObject(Type tp) : base(tp) // So we don't call PyObject_GC_Del here and instead we set the python // reference to a weak reference so that the C# object can be collected. GCHandle gc = GCHandle.Alloc(self, GCHandleType.Weak); - Marshal.WriteIntPtr(self.pyHandle, ObjectOffset.magic(self.tpHandle), (IntPtr)gc); + int gcOffset = ObjectOffset.magic(Runtime.PyObject_TYPE(self.pyHandle)); + Marshal.WriteIntPtr(self.pyHandle, gcOffset, (IntPtr)gc); self.gcHandle.Free(); self.gcHandle = gc; } From 039b3ca24ad01931088dfddbf7dbf5840192b77e Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Thu, 17 Dec 2020 22:44:26 -0800 Subject: [PATCH 4/4] a few extra assertions --- src/runtime/clrobject.cs | 2 ++ src/runtime/native/TypeOffset.cs | 8 +++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/runtime/clrobject.cs b/src/runtime/clrobject.cs index a79662ccc..0a352b381 100644 --- a/src/runtime/clrobject.cs +++ b/src/runtime/clrobject.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Runtime.InteropServices; namespace Python.Runtime @@ -85,6 +86,7 @@ internal static CLRObject Restore(object ob, IntPtr pyHandle, InterDomainContext pyHandle = pyHandle, tpHandle = Runtime.PyObject_TYPE(pyHandle) }; + Debug.Assert(co.tpHandle != IntPtr.Zero); co.Load(context); return co; } diff --git a/src/runtime/native/TypeOffset.cs b/src/runtime/native/TypeOffset.cs index 0c51110da..bca191565 100644 --- a/src/runtime/native/TypeOffset.cs +++ b/src/runtime/native/TypeOffset.cs @@ -85,9 +85,11 @@ internal static void Use(ITypeOffsets offsets) internal static Dictionary GetOffsets() { var properties = typeof(TypeOffset).GetProperties(FieldFlags); - return properties.ToDictionary( - keySelector: p => p.Name, - elementSelector: p => (int)p.GetValue(obj: null, index: null)); + var result = properties.ToDictionary( + keySelector: p => p.Name, + elementSelector: p => (int)p.GetValue(obj: null, index: null)); + Debug.Assert(result.Values.Any(v => v != 0)); + return result; } internal static int GetOffsetUncached(string name)