From d6c971663be854c7ce56ea23f6bec01aac1f517a Mon Sep 17 00:00:00 2001 From: amos402 Date: Thu, 24 Sep 2020 15:00:55 +0800 Subject: [PATCH 1/6] Support weakref for CLR types --- src/embed_tests/Python.EmbeddingTest.csproj | 3 +- src/embed_tests/TestClass.cs | 82 +++++++ src/runtime/classbase.cs | 36 ++- src/runtime/classmanager.cs | 1 + src/runtime/clrobject.cs | 8 +- src/runtime/exceptions.cs | 25 ++- src/runtime/interop.cs | 85 +++---- src/runtime/managedtype.cs | 52 ++--- src/runtime/metatype.cs | 56 +++-- src/runtime/pyobject.cs | 4 +- src/runtime/runtime.cs | 30 ++- src/runtime/typemanager.cs | 236 ++++++++++++-------- 12 files changed, 384 insertions(+), 234 deletions(-) create mode 100644 src/embed_tests/TestClass.cs diff --git a/src/embed_tests/Python.EmbeddingTest.csproj b/src/embed_tests/Python.EmbeddingTest.csproj index eff226dd5..ce611e0b7 100644 --- a/src/embed_tests/Python.EmbeddingTest.csproj +++ b/src/embed_tests/Python.EmbeddingTest.csproj @@ -1,4 +1,4 @@ - + Debug @@ -90,6 +90,7 @@ + diff --git a/src/embed_tests/TestClass.cs b/src/embed_tests/TestClass.cs new file mode 100644 index 000000000..08a0a497d --- /dev/null +++ b/src/embed_tests/TestClass.cs @@ -0,0 +1,82 @@ +using System; +using System.Runtime.InteropServices; + +using NUnit.Framework; + +using Python.Runtime; + +using PyRuntime = Python.Runtime.Runtime; + +namespace Python.EmbeddingTest +{ + public class TestClass + { + public class MyClass + { + } + + [OneTimeSetUp] + public void SetUp() + { + PythonEngine.Initialize(); + } + + [OneTimeTearDown] + public void Dispose() + { + PythonEngine.Shutdown(); + } + + [Test] + public void WeakRefForClrObject() + { + var obj = new MyClass(); + using (var scope = Py.CreateScope()) + { + scope.Set("clr_obj", obj); + scope.Exec(@" +import weakref +ref = weakref.ref(clr_obj) +"); + using (PyObject pyobj = scope.Get("clr_obj")) + { + ValidateAttachedGCHandle(obj, pyobj.Handle); + } + } + } + + [Test] + public void WeakRefForSubClass() + { + using (var scope = Py.CreateScope()) + { + scope.Exec(@" +from Python.EmbeddingTest import TestClass +import weakref + +class Sub(TestClass.MyClass): + pass + +obj = Sub() +ref = weakref.ref(obj) +"); + using (PyObject pyobj = scope.Get("obj")) + { + IntPtr op = pyobj.Handle; + IntPtr type = PyRuntime.PyObject_TYPE(op); + IntPtr clrHandle = Marshal.ReadIntPtr(op, ObjectOffset.magic(type)); + var clobj = (CLRObject)GCHandle.FromIntPtr(clrHandle).Target; + Assert.IsTrue(clobj.inst is MyClass); + } + } + } + + private static void ValidateAttachedGCHandle(object obj, IntPtr op) + { + IntPtr type = PyRuntime.PyObject_TYPE(op); + IntPtr clrHandle = Marshal.ReadIntPtr(op, ObjectOffset.magic(type)); + var clobj = (CLRObject)GCHandle.FromIntPtr(clrHandle).Target; + Assert.True(ReferenceEquals(clobj.inst, obj)); + } + } +} diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs index f26079de2..9b25be17b 100644 --- a/src/runtime/classbase.cs +++ b/src/runtime/classbase.cs @@ -1,8 +1,6 @@ using System; using System.Collections; -using System.Diagnostics; using System.Runtime.InteropServices; -using System.Runtime.Serialization; namespace Python.Runtime { @@ -288,44 +286,40 @@ public static IntPtr tp_repr(IntPtr ob) public static void tp_dealloc(IntPtr ob) { ManagedType self = GetManagedObject(ob); + if (Runtime.PyType_SUPPORTS_WEAKREFS(Runtime.PyObject_TYPE(ob))) + { + Runtime.PyObject_ClearWeakRefs(ob); + } tp_clear(ob); - Runtime.PyObject_GC_UnTrack(self.pyHandle); - Runtime.PyObject_GC_Del(self.pyHandle); + Runtime.PyObject_GC_UnTrack(ob); + Runtime.PyObject_GC_Del(ob); self.FreeGCHandle(); } public static int tp_clear(IntPtr ob) { ManagedType self = GetManagedObject(ob); - if (!self.IsTypeObject()) - { - ClearObjectDict(ob); - } - self.tpHandle = IntPtr.Zero; + ClearObjectDict(ob); + Runtime.Py_CLEAR(ref self.tpHandle); return 0; } protected override void OnSave(InterDomainContext context) { base.OnSave(context); - if (pyHandle != tpHandle) - { - IntPtr dict = GetObjectDict(pyHandle); - Runtime.XIncref(dict); - context.Storage.AddValue("dict", dict); - } + IntPtr dict = GetObjectDict(pyHandle); + Runtime.XIncref(dict); + Runtime.XIncref(tpHandle); + context.Storage.AddValue("dict", dict); } protected override void OnLoad(InterDomainContext context) { base.OnLoad(context); - if (pyHandle != tpHandle) - { - IntPtr dict = context.Storage.GetValue("dict"); - SetObjectDict(pyHandle, dict); - } + IntPtr dict = context.Storage.GetValue("dict"); + SetObjectDict(pyHandle, dict); gcHandle = AllocGCHandle(); - Marshal.WriteIntPtr(pyHandle, TypeOffset.magic(), (IntPtr)gcHandle); + Marshal.WriteIntPtr(pyHandle, ObjectOffset.magic(tpHandle), (IntPtr)gcHandle); } } } diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index 1d569d2c8..731c4e6cb 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -443,6 +443,7 @@ private static ClassInfo GetClassInfo(Type type) } // Note the given instance might be uninitialized ob = GetClass(tp); + ob.IncrRefCount(); ci.members[mi.Name] = ob; continue; } diff --git a/src/runtime/clrobject.cs b/src/runtime/clrobject.cs index 0b62fecba..bc639f193 100644 --- a/src/runtime/clrobject.cs +++ b/src/runtime/clrobject.cs @@ -29,10 +29,6 @@ internal CLRObject(object ob, IntPtr tp) tpHandle = tp; pyHandle = py; inst = ob; - - // Fix the BaseException args (and __cause__ in case of Python 3) - // slot if wrapping a CLR exception - Exceptions.SetArgsAndCause(py); } protected CLRObject() @@ -48,7 +44,7 @@ static CLRObject GetInstance(object ob, IntPtr pyType) static CLRObject GetInstance(object ob) { ClassBase cc = ClassManager.GetClass(ob.GetType()); - return GetInstance(ob, cc.tpHandle); + return GetInstance(ob, cc.pyHandle); } @@ -62,7 +58,7 @@ internal static IntPtr GetInstHandle(object ob, IntPtr pyType) internal static IntPtr GetInstHandle(object ob, Type type) { ClassBase cc = ClassManager.GetClass(type); - CLRObject co = GetInstance(ob, cc.tpHandle); + CLRObject co = GetInstance(ob, cc.pyHandle); return co.pyHandle; } diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index 58506bfbb..8c2213104 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -82,6 +82,13 @@ internal static Exception ToException(IntPtr ob) } return Runtime.PyUnicode_FromString(message); } + + public static int tp_init(IntPtr ob, IntPtr args, IntPtr kwds) + { + Exceptions.SetArgsAndCause(ob); + return 0; + } + } /// @@ -177,15 +184,23 @@ internal static void SetArgsAndCause(IntPtr ob) args = Runtime.PyTuple_New(0); } - Marshal.WriteIntPtr(ob, ExceptionOffset.args, args); - + int baseOffset = OriginalObjectOffsets.Size; + Runtime.Py_SETREF(ob, baseOffset + ExceptionOffset.args, args); + if (e.InnerException != null) { - IntPtr cause = CLRObject.GetInstHandle(e.InnerException); - Marshal.WriteIntPtr(ob, ExceptionOffset.cause, cause); + IntPtr cause = GetExceptHandle(e.InnerException); + Runtime.Py_SETREF(ob, baseOffset + ExceptionOffset.cause, cause); } } + internal static IntPtr GetExceptHandle(Exception e) + { + IntPtr op = CLRObject.GetInstHandle(e); + SetArgsAndCause(op); + return op; + } + /// /// Shortcut for (pointer == NULL) -> throw PythonException /// @@ -283,7 +298,7 @@ public static void SetError(Exception e) return; } - IntPtr op = CLRObject.GetInstHandle(e); + IntPtr op = GetExceptHandle(e); IntPtr etype = Runtime.PyObject_GetAttrString(op, "__class__"); Runtime.PyErr_SetObject(new BorrowedReference(etype), new BorrowedReference(op)); Runtime.XDecref(etype); diff --git a/src/runtime/interop.cs b/src/runtime/interop.cs index 1caabab17..94145e89f 100644 --- a/src/runtime/interop.cs +++ b/src/runtime/interop.cs @@ -82,26 +82,25 @@ static TypeOffset() fi.SetValue(null, offset); } } - - public static int magic() => ManagedDataOffsets.Magic; } + internal static class ManagedDataOffsets { - public static int Magic { get; private set; } public static readonly Dictionary NameMapping = new Dictionary(); static class DataOffsets { public static readonly int ob_data; public static readonly int ob_dict; + public static readonly int ob_weaklist; static DataOffsets() { FieldInfo[] fields = typeof(DataOffsets).GetFields(BindingFlags.Static | BindingFlags.Public); for (int i = 0; i < fields.Length; i++) { - fields[i].SetValue(null, -(i * IntPtr.Size) - IntPtr.Size); + fields[i].SetValue(null, (i) * IntPtr.Size); } } } @@ -113,11 +112,8 @@ static ManagedDataOffsets() { NameMapping[fi.Name] = (int)fi.GetValue(null); } - // XXX: Use the members after PyHeapTypeObject as magic slot - Magic = TypeOffset.members; - FieldInfo[] fields = typeof(DataOffsets).GetFields(BindingFlags.Static | BindingFlags.Public); - size = fields.Length * IntPtr.Size; + Size = fields.Length * IntPtr.Size; } public static int GetSlotOffset(string name) @@ -125,29 +121,10 @@ public static int GetSlotOffset(string name) return NameMapping[name]; } - private static int BaseOffset(IntPtr type) - { - Debug.Assert(type != IntPtr.Zero); - int typeSize = Marshal.ReadInt32(type, TypeOffset.tp_basicsize); - Debug.Assert(typeSize > 0); - return typeSize; - } - - public static int DataOffset(IntPtr type) - { - return BaseOffset(type) + DataOffsets.ob_data; - } - - public static int DictOffset(IntPtr type) - { - return BaseOffset(type) + DataOffsets.ob_dict; - } - public static int ob_data => DataOffsets.ob_data; public static int ob_dict => DataOffsets.ob_dict; - public static int Size { get { return size; } } - - private static readonly int size; + public static int ob_weaklist => DataOffsets.ob_weaklist; + public static int Size { get; private set; } } internal static class OriginalObjectOffsets @@ -182,6 +159,26 @@ static OriginalObjectOffsets() public static int ob_type; } + + internal static class ManagedExceptionOffset + { + public static int ob_data { get; private set; } + public static int ob_dict { get; private set; } + public static int ob_weaklist { get; private set; } + + public static int Size { get; private set; } + + static ManagedExceptionOffset() + { + int baseOffset = OriginalObjectOffsets.Size + ExceptionOffset.Size(); + ob_data = baseOffset + ManagedDataOffsets.ob_data; + ob_dict = baseOffset + ManagedDataOffsets.ob_dict; + ob_weaklist = baseOffset + ManagedDataOffsets.ob_weaklist; + Size = ob_weaklist + IntPtr.Size; + } + } + + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] internal class ObjectOffset { @@ -194,27 +191,17 @@ static ObjectOffset() ob_refcnt = OriginalObjectOffsets.ob_refcnt; ob_type = OriginalObjectOffsets.ob_type; - size = OriginalObjectOffsets.Size + ManagedDataOffsets.Size; + Size = OriginalObjectOffsets.Size + ManagedDataOffsets.Size; } public static int magic(IntPtr type) { - return ManagedDataOffsets.DataOffset(type); + return (int)Marshal.ReadIntPtr(type, TypeOffset.members); } public static int TypeDictOffset(IntPtr type) { - return ManagedDataOffsets.DictOffset(type); - } - - public static int Size(IntPtr pyType) - { - if (IsException(pyType)) - { - return ExceptionOffset.Size(); - } - - return size; + return (int)Marshal.ReadIntPtr(type, TypeOffset.tp_dictoffset); } #if PYTHON_WITH_PYDEBUG @@ -223,15 +210,8 @@ public static int Size(IntPtr pyType) #endif public static int ob_refcnt; public static int ob_type; - private static readonly int size; - private static bool IsException(IntPtr pyObject) - { - var type = Runtime.PyObject_TYPE(pyObject); - return Runtime.PyType_IsSameAsOrSubtype(type, ofType: Exceptions.BaseException) - || Runtime.PyType_IsSameAsOrSubtype(type, ofType: Runtime.PyTypeType) - && Runtime.PyType_IsSubtype(pyObject, Exceptions.BaseException); - } + public static int Size { get; private set; } } [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] @@ -243,10 +223,9 @@ static ExceptionOffset() FieldInfo[] fi = type.GetFields(BindingFlags.Static | BindingFlags.Public); for (int i = 0; i < fi.Length; i++) { - fi[i].SetValue(null, (i * IntPtr.Size) + OriginalObjectOffsets.Size); + fi[i].SetValue(null, i * IntPtr.Size); } - - size = fi.Length * IntPtr.Size + OriginalObjectOffsets.Size + ManagedDataOffsets.Size; + size = fi.Length * IntPtr.Size; } public static int Size() { return size; } diff --git a/src/runtime/managedtype.cs b/src/runtime/managedtype.cs index 171d2f2cd..9e77b1e1e 100644 --- a/src/runtime/managedtype.cs +++ b/src/runtime/managedtype.cs @@ -26,7 +26,6 @@ internal enum TrackTypes internal IntPtr pyHandle; // PyObject * internal IntPtr tpHandle; // PyType * - private static readonly Dictionary _managedObjs = new Dictionary(); internal void IncrRefCount() @@ -80,29 +79,28 @@ internal void FreeGCHandle() /// internal static ManagedType GetManagedObject(IntPtr ob) { - if (ob != IntPtr.Zero) + if (ob == IntPtr.Zero) { - IntPtr tp = Runtime.PyObject_TYPE(ob); - if (tp == Runtime.PyTypeType || tp == Runtime.PyCLRMetaType) - { - tp = ob; - } - - var flags = Util.ReadCLong(tp, TypeOffset.tp_flags); - if ((flags & TypeFlags.Managed) != 0) - { - IntPtr op = tp == ob - ? Marshal.ReadIntPtr(tp, TypeOffset.magic()) - : Marshal.ReadIntPtr(ob, ObjectOffset.magic(tp)); - if (op == IntPtr.Zero) - { - return null; - } - var gc = (GCHandle)op; - return (ManagedType)gc.Target; - } + return null; } - return null; + IntPtr tp = Runtime.PyObject_TYPE(ob); + var flags = Util.ReadCLong(tp, TypeOffset.tp_flags); + if ((flags & TypeFlags.Managed) == 0) + { + return null; + } + int offset = ObjectOffset.magic(tp); + if (offset == 0) + { + return null; + } + IntPtr op = Marshal.ReadIntPtr(ob, offset); + if (op == IntPtr.Zero) + { + return null; + } + var gc = (GCHandle)op; + return (ManagedType)gc.Target; } @@ -122,11 +120,6 @@ internal static bool IsManagedType(IntPtr ob) if (ob != IntPtr.Zero) { IntPtr tp = Runtime.PyObject_TYPE(ob); - if (tp == Runtime.PyTypeType || tp == Runtime.PyCLRMetaType) - { - tp = ob; - } - var flags = Util.ReadCLong(tp, TypeOffset.tp_flags); if ((flags & TypeFlags.Managed) != 0) { @@ -136,11 +129,6 @@ internal static bool IsManagedType(IntPtr ob) return false; } - public bool IsTypeObject() - { - return pyHandle == tpHandle; - } - internal static IDictionary GetManagedObjects() { return _managedObjs; diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs index f7afd5d6d..dcb77075a 100644 --- a/src/runtime/metatype.cs +++ b/src/runtime/metatype.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; @@ -147,37 +148,26 @@ public static IntPtr tp_new(IntPtr tp, IntPtr args, IntPtr kw) flags |= TypeFlags.HaveGC; Util.WriteCLong(type, TypeOffset.tp_flags, flags); - TypeManager.CopySlot(base_type, type, TypeOffset.tp_dealloc); + //TypeManager.CopySlot(base_type, type, TypeOffset.tp_dealloc); // Hmm - the standard subtype_traverse, clear look at ob_size to // do things, so to allow gc to work correctly we need to move // our hidden handle out of ob_size. Then, in theory we can // comment this out and still not crash. - TypeManager.CopySlot(base_type, type, TypeOffset.tp_traverse); - TypeManager.CopySlot(base_type, type, TypeOffset.tp_clear); - + //TypeManager.CopySlot(base_type, type, TypeOffset.tp_traverse); + //TypeManager.CopySlot(base_type, type, TypeOffset.tp_clear); // for now, move up hidden handle... - IntPtr gc = Marshal.ReadIntPtr(base_type, TypeOffset.magic()); - Marshal.WriteIntPtr(type, TypeOffset.magic(), gc); - - return type; - } - - - public static IntPtr tp_alloc(IntPtr mt, int n) - { - IntPtr type = Runtime.PyType_GenericAlloc(mt, n); + unsafe + { + var baseTypeEx = ClrMetaTypeEx.FromType(base_type); + var typeEx = ClrMetaTypeEx.FromType(type); + typeEx->ClrHandleOffset = (IntPtr)(OriginalObjectOffsets.Size + ManagedDataOffsets.ob_data); + typeEx->ClrHandle = baseTypeEx->ClrHandle; + } return type; } - - public static void tp_free(IntPtr tp) - { - Runtime.PyObject_GC_Del(tp); - } - - /// /// Metatype __call__ implementation. This is needed to ensure correct /// initialization (__init__ support), because the tp_call we inherit @@ -280,7 +270,7 @@ public static void tp_dealloc(IntPtr tp) var flags = Util.ReadCLong(tp, TypeOffset.tp_flags); if ((flags & TypeFlags.Subclass) == 0) { - IntPtr gc = Marshal.ReadIntPtr(tp, TypeOffset.magic()); + IntPtr gc = Marshal.ReadIntPtr(tp, ObjectOffset.magic(Runtime.PyObject_TYPE(tp))); ((GCHandle)gc).Free(); } @@ -296,6 +286,14 @@ public static void tp_dealloc(IntPtr tp) NativeCall.Void_Call_1(op, tp); } + public static int tp_clear(IntPtr ob) + { + IntPtr type = Runtime.PyObject_TYPE(ob); + int dictOffset = ObjectOffset.TypeDictOffset(type); + Runtime.Py_SETREF(ob, dictOffset, IntPtr.Zero); + return 0; + } + private static IntPtr DoInstanceCheck(IntPtr tp, IntPtr args, bool checkType) { var cb = GetManagedObject(tp) as ClassBase; @@ -352,4 +350,18 @@ public static IntPtr __subclasscheck__(IntPtr tp, IntPtr args) return DoInstanceCheck(tp, args, true); } } + + + [StructLayout(LayoutKind.Sequential)] + struct ClrMetaTypeEx + { + public IntPtr ClrHandleOffset; + public IntPtr ClrHandle; + + public static unsafe ClrMetaTypeEx* FromType(IntPtr type) + { + return (ClrMetaTypeEx*)(type + TypeOffset.members); + } + } + } diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index d2fb3f6de..395664f1c 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -122,7 +122,9 @@ public static PyObject FromManagedObject(object ob) Runtime.XIncref(Runtime.PyNone); return new PyObject(Runtime.PyNone); } - IntPtr op = CLRObject.GetInstHandle(ob); + IntPtr op = typeof(Exception).IsAssignableFrom(ob.GetType()) ? + Exceptions.GetExceptHandle((Exception)ob) + : CLRObject.GetInstHandle(ob); return new PyObject(op); } diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 559817776..e6a29b8c5 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -386,8 +386,8 @@ internal static void Shutdown(ShutdownMode mode) ClearClrModules(); RemoveClrRootModule(); - MoveClrInstancesOnwershipToPython(); ClassManager.DisposePythonWrappersForClrTypes(); + MoveClrInstancesOnwershipToPython(); TypeManager.RemoveTypes(); MetaType.Release(); @@ -563,11 +563,8 @@ private static void MoveClrInstancesOnwershipToPython() PyObject_GC_Track(obj.pyHandle); } } - if (obj.gcHandle.IsAllocated) - { - obj.gcHandle.Free(); - } - obj.gcHandle = default; + obj.FreeGCHandle(); + Marshal.WriteIntPtr(obj.pyHandle, ObjectOffset.magic(obj.tpHandle), IntPtr.Zero); } ManagedType.ClearTrackedObjects(); } @@ -1986,6 +1983,11 @@ internal static IntPtr PyType_GenericAlloc(IntPtr type, long n) [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern IntPtr _PyType_Lookup(IntPtr type, IntPtr name); + internal static bool PyType_SUPPORTS_WEAKREFS(IntPtr type) + { + return Marshal.ReadIntPtr(type, TypeOffset.tp_weaklistoffset) != IntPtr.Zero; + } + [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern IntPtr PyObject_GenericGetAttr(IntPtr obj, IntPtr name); @@ -1995,6 +1997,9 @@ internal static IntPtr PyType_GenericAlloc(IntPtr type, long n) [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern IntPtr _PyObject_GetDictPtr(IntPtr obj); + [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] + internal static extern IntPtr _PyObject_GC_Calloc(IntPtr basicsize); + [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern void PyObject_GC_Del(IntPtr tp); @@ -2004,6 +2009,9 @@ internal static IntPtr PyType_GenericAlloc(IntPtr type, long n) [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern void PyObject_GC_UnTrack(IntPtr tp); + [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] + internal static extern void PyObject_ClearWeakRefs(IntPtr obj); + [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern void _PyObject_Dump(IntPtr ob); @@ -2136,6 +2144,16 @@ internal static void Py_CLEAR(ref IntPtr ob) ob = IntPtr.Zero; } + internal static unsafe void Py_SETREF(IntPtr ob, int offset, IntPtr target) + { + var p = (void**)(ob + offset); + if (*p != null) + { + XDecref((IntPtr)(*p)); + } + *p = (void*)target; + } + //==================================================================== // Python Capsules API //==================================================================== diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index 425b91c09..7298e8161 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -43,10 +43,10 @@ internal static void Initialize() internal static void RemoveTypes() { - foreach (var tpHandle in cache.Values) + foreach (var entry in _slotsHolders) { - SlotsHolder holder; - if (_slotsHolders.TryGetValue(tpHandle, out holder)) + IntPtr tpHandle = entry.Key; + SlotsHolder holder = entry.Value; { // If refcount > 1, it needs to reset the managed slot, // otherwise it can dealloc without any trick. @@ -143,13 +143,13 @@ internal static IntPtr GetTypeHandle(ManagedType obj, Type type) internal static IntPtr CreateType(Type impl) { IntPtr type = AllocateTypeObject(impl.Name); - int ob_size = ObjectOffset.Size(type); + int ob_size = ObjectOffset.Size; // Set tp_basicsize to the size of our managed instance objects. Marshal.WriteIntPtr(type, TypeOffset.tp_basicsize, (IntPtr)ob_size); - var offset = (IntPtr)ObjectOffset.TypeDictOffset(type); - Marshal.WriteIntPtr(type, TypeOffset.tp_dictoffset, offset); + var offset = OriginalObjectOffsets.Size + ManagedDataOffsets.ob_dict; + Marshal.WriteIntPtr(type, TypeOffset.tp_dictoffset, (IntPtr)offset); SlotsHolder slotsHolder = CreateSolotsHolder(type); InitializeSlots(type, impl, slotsHolder); @@ -169,6 +169,11 @@ internal static IntPtr CreateType(Type impl) Runtime.XDecref(mod); InitMethods(type, impl); + unsafe + { + var typeEx = ClrMetaTypeEx.FromType(type); + typeEx->ClrHandleOffset = (IntPtr)OriginalObjectOffsets.Size + ManagedDataOffsets.ob_data; + } return type; } @@ -189,17 +194,25 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType) } IntPtr base_ = IntPtr.Zero; - int ob_size = ObjectOffset.Size(Runtime.PyTypeType); - + int baseOffset = OriginalObjectOffsets.Size; + int ob_size, tp_dictoffset, tp_weaklistoffset, magicOffset; // XXX Hack, use a different base class for System.Exception // Python 2.5+ allows new style class exceptions but they *must* // subclass BaseException (or better Exception). if (typeof(Exception).IsAssignableFrom(clrType)) { - ob_size = ObjectOffset.Size(Exceptions.Exception); + tp_dictoffset = ManagedExceptionOffset.ob_dict; + tp_weaklistoffset = 0; + ob_size = ManagedExceptionOffset.Size; + magicOffset = ManagedExceptionOffset.ob_data; + } + else + { + tp_dictoffset = baseOffset + ManagedDataOffsets.ob_dict; + tp_weaklistoffset = baseOffset + ManagedDataOffsets.ob_weaklist; + ob_size = baseOffset + ManagedDataOffsets.Size; + magicOffset = baseOffset + ManagedDataOffsets.ob_data; } - - int tp_dictoffset = ob_size + ManagedDataOffsets.ob_dict; if (clrType == typeof(Exception)) { @@ -219,6 +232,7 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType) Marshal.WriteIntPtr(type, TypeOffset.tp_basicsize, (IntPtr)ob_size); Marshal.WriteIntPtr(type, TypeOffset.tp_itemsize, IntPtr.Zero); Marshal.WriteIntPtr(type, TypeOffset.tp_dictoffset, (IntPtr)tp_dictoffset); + Marshal.WriteIntPtr(type, TypeOffset.tp_weaklistoffset, (IntPtr)tp_weaklistoffset); // we want to do this after the slot stuff above in case the class itself implements a slot method SlotsHolder slotsHolder = CreateSolotsHolder(type); @@ -260,10 +274,16 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType) // Hide the gchandle of the implementation in a magic type slot. GCHandle gc = impl.AllocGCHandle(); - Marshal.WriteIntPtr(type, TypeOffset.magic(), (IntPtr)gc); + unsafe + { + var typePtr = ClrMetaTypeEx.FromType(type); + typePtr->ClrHandle = (IntPtr)gc; + typePtr->ClrHandleOffset = (IntPtr)magicOffset; + } // Set the handle attributes on the implementing instance. - impl.tpHandle = type; + impl.tpHandle = Runtime.PyCLRMetaType; + Runtime.XIncref(type); impl.pyHandle = type; //DebugUtil.DumpType(type); @@ -407,11 +427,19 @@ 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; + var heapTypeSize = (int)Marshal.ReadIntPtr(py_type, TypeOffset.tp_basicsize); + Debug.Assert(heapTypeSize == TypeOffset.members); + int metaSize = heapTypeSize + Marshal.SizeOf(typeof(ClrMetaTypeEx)); - Marshal.WriteIntPtr(type, TypeOffset.tp_base, py_type); + IntPtr type = Runtime._PyObject_GC_Calloc(new IntPtr(metaSize)); Runtime.XIncref(py_type); + Marshal.WriteIntPtr(type, TypeOffset.tp_base, py_type); + Marshal.WriteIntPtr(type, TypeOffset.ob_refcnt, (IntPtr)1); + Marshal.WriteIntPtr(type, TypeOffset.tp_basicsize, (IntPtr)metaSize); + Marshal.WriteIntPtr(type, heapTypeSize, (IntPtr)(heapTypeSize + IntPtr.Size)); + + SetupHeapType(type, "CLR Metatype"); const int flags = TypeFlags.Default | TypeFlags.Managed @@ -544,39 +572,13 @@ internal static IntPtr BasicSubType(string name, IntPtr base_, Type impl) return type; } - /// /// Utility method to allocate a type object & do basic initialization. /// internal static IntPtr AllocateTypeObject(string name) { - IntPtr type = Runtime.PyType_GenericAlloc(Runtime.PyTypeType, 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. - Marshal.WriteIntPtr(type, TypeOffset.ob_size, IntPtr.Zero); - - // Cheat a little: we'll set tp_name to the internal char * of - // the Python version of the type name - otherwise we'd have to - // allocate the tp_name and would have no way to free it. - IntPtr temp = Runtime.PyUnicode_FromString(name); - IntPtr raw = Runtime.PyUnicode_AsUTF8(temp); - Marshal.WriteIntPtr(type, TypeOffset.tp_name, raw); - Marshal.WriteIntPtr(type, TypeOffset.name, temp); - - Runtime.XIncref(temp); - Marshal.WriteIntPtr(type, TypeOffset.qualname, temp); - temp = type + TypeOffset.nb_add; - Marshal.WriteIntPtr(type, TypeOffset.tp_as_number, temp); - - temp = type + TypeOffset.sq_length; - Marshal.WriteIntPtr(type, TypeOffset.tp_as_sequence, temp); - - temp = type + TypeOffset.mp_length; - Marshal.WriteIntPtr(type, TypeOffset.tp_as_mapping, temp); - - temp = type + TypeOffset.bf_getbuffer; - Marshal.WriteIntPtr(type, TypeOffset.tp_as_buffer, temp); + IntPtr type = Runtime.PyType_GenericAlloc(Runtime.PyCLRMetaType, 0); + SetupHeapType(type, name); return type; } @@ -734,6 +736,36 @@ private static SlotsHolder CreateSolotsHolder(IntPtr type) _slotsHolders.Add(type, holder); return holder; } + + private static void SetupHeapType(IntPtr type, string name) + { + // 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. + Marshal.WriteIntPtr(type, TypeOffset.ob_size, IntPtr.Zero); + + // Cheat a little: we'll set tp_name to the internal char * of + // the Python version of the type name - otherwise we'd have to + // allocate the tp_name and would have no way to free it. + IntPtr temp = Runtime.PyUnicode_FromString(name); + IntPtr raw = Runtime.PyUnicode_AsUTF8(temp); + Marshal.WriteIntPtr(type, TypeOffset.tp_name, raw); + Marshal.WriteIntPtr(type, TypeOffset.name, temp); + + Runtime.XIncref(temp); + Marshal.WriteIntPtr(type, TypeOffset.qualname, temp); + temp = type + TypeOffset.nb_add; + Marshal.WriteIntPtr(type, TypeOffset.tp_as_number, temp); + + temp = type + TypeOffset.sq_length; + Marshal.WriteIntPtr(type, TypeOffset.tp_as_sequence, temp); + + temp = type + TypeOffset.mp_length; + Marshal.WriteIntPtr(type, TypeOffset.tp_as_mapping, temp); + + temp = type + TypeOffset.bf_getbuffer; + Marshal.WriteIntPtr(type, TypeOffset.tp_as_buffer, temp); + } } @@ -742,10 +774,10 @@ class SlotsHolder public delegate void Resetor(IntPtr type, int offset); private readonly IntPtr _type; - private Dictionary _slots = new Dictionary(); - private List _keepalive = new List(); - private Dictionary _customResetors = new Dictionary(); - private List _deallocators = new List(); + private Dictionary _slots; + private List _keepalive; + private Dictionary _customResetors; + private List _deallocators; private bool _alreadyReset = false; /// @@ -759,21 +791,25 @@ public SlotsHolder(IntPtr type) public void Set(int offset, ThunkInfo thunk) { + if (_slots == null) _slots = new Dictionary(); _slots[offset] = thunk; } public void Set(int offset, Resetor resetor) { + if (_customResetors == null) _customResetors = new Dictionary(); _customResetors[offset] = resetor; } public void AddDealloctor(Action deallocate) { + if (_deallocators == null) _deallocators = new List(); _deallocators.Add(deallocate); } public void KeeapAlive(ThunkInfo thunk) { + if (_keepalive == null) _keepalive = new List(); _keepalive.Add(thunk); } @@ -784,47 +820,15 @@ public void ResetSlots() return; } _alreadyReset = true; -#if DEBUG - IntPtr tp_name = Marshal.ReadIntPtr(_type, TypeOffset.tp_name); - string typeName = Marshal.PtrToStringAnsi(tp_name); -#endif - foreach (var offset in _slots.Keys) - { - IntPtr ptr = GetDefaultSlot(offset); -#if DEBUG - //DebugUtil.Print($"Set slot<{TypeOffsetHelper.GetSlotNameByOffset(offset)}> to 0x{ptr.ToString("X")} at {typeName}<0x{_type}>"); -#endif - Marshal.WriteIntPtr(_type, offset, ptr); - } - - foreach (var action in _deallocators) - { - action(); - } - - foreach (var pair in _customResetors) - { - int offset = pair.Key; - var resetor = pair.Value; - resetor?.Invoke(_type, offset); - } - - _customResetors.Clear(); - _slots.Clear(); - _keepalive.Clear(); - _deallocators.Clear(); + ResetDefaultSlots(); + InvokeDeallocActions(); + InvokeCustomResetors(); - // Custom reset - IntPtr handlePtr = Marshal.ReadIntPtr(_type, TypeOffset.magic()); - if (handlePtr != IntPtr.Zero) + if (_keepalive != null) { - GCHandle handle = GCHandle.FromIntPtr(handlePtr); - if (handle.IsAllocated) - { - handle.Free(); - } - Marshal.WriteIntPtr(_type, TypeOffset.magic(), IntPtr.Zero); + _keepalive.Clear(); } + ReleaseGCHandle(); } public static IntPtr GetDefaultSlot(int offset) @@ -869,6 +873,65 @@ public static IntPtr GetDefaultSlot(int offset) return Marshal.ReadIntPtr(Runtime.PyTypeType, offset); } + + private void InvokeCustomResetors() + { + if (_customResetors == null) return; + foreach (var pair in _customResetors) + { + int offset = pair.Key; + var resetor = pair.Value; + resetor?.Invoke(_type, offset); + } + _customResetors.Clear(); + } + + private void InvokeDeallocActions() + { + if (_deallocators == null) return; + foreach (var action in _deallocators) + { + action(); + } + _deallocators.Clear(); + } + + private void ResetDefaultSlots() + { + if (_slots == null) return; +#if DEBUG + IntPtr tp_name = Marshal.ReadIntPtr(_type, TypeOffset.tp_name); + string typeName = Marshal.PtrToStringAnsi(tp_name); +#endif + foreach (var offset in _slots.Keys) + { + IntPtr ptr = GetDefaultSlot(offset); +#if DEBUG + //DebugUtil.Print($"Set slot<{TypeOffsetHelper.GetSlotNameByOffset(offset)}> to 0x{ptr.ToString("X")} at {typeName}<0x{_type}>"); +#endif + Marshal.WriteIntPtr(_type, offset, ptr); + } + _slots.Clear(); + } + + private void ReleaseGCHandle() + { + if (!ManagedType.IsManagedType(_type)) + { + return; + } + int offset = ObjectOffset.magic(Runtime.PyObject_TYPE(_type)); + IntPtr handlePtr = Marshal.ReadIntPtr(_type, offset); + if (handlePtr != IntPtr.Zero) + { + GCHandle handle = GCHandle.FromIntPtr(handlePtr); + if (handle.IsAllocated) + { + handle.Free(); + } + Marshal.WriteIntPtr(_type, offset, IntPtr.Zero); + } + } } @@ -884,8 +947,7 @@ public static IntPtr CreateObjectType() } const string code = "class A(object): pass"; var resRef = Runtime.PyRun_String(code, RunFlagType.File, globals, globals); - IntPtr res = resRef.DangerousGetAddress(); - if (res == IntPtr.Zero) + if (resRef.IsNull()) { try { From f08813c7dc013bc104be6848609bb7d2279580fb Mon Sep 17 00:00:00 2001 From: amos402 Date: Thu, 22 Oct 2020 17:07:15 +0800 Subject: [PATCH 2/6] Apply unified mechanism for `GetManagedObjectType` --- src/runtime/managedtype.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/runtime/managedtype.cs b/src/runtime/managedtype.cs index e87d8a461..85bcf51b7 100644 --- a/src/runtime/managedtype.cs +++ b/src/runtime/managedtype.cs @@ -108,18 +108,12 @@ internal static ManagedType GetManagedObject(IntPtr ob) /// internal static ManagedType GetManagedObjectType(IntPtr ob) { - if (ob != IntPtr.Zero) + if (ob == IntPtr.Zero) { - IntPtr tp = Runtime.PyObject_TYPE(ob); - var flags = Util.ReadCLong(tp, TypeOffset.tp_flags); - if ((flags & TypeFlags.Managed) != 0) - { - tp = Marshal.ReadIntPtr(tp, TypeOffset.magic()); - var gc = (GCHandle)tp; - return (ManagedType)gc.Target; - } + return null; } - return null; + IntPtr tp = Runtime.PyObject_TYPE(ob); + return GetManagedObject(tp); } From b68f0a4b7353a612f5897185f6230e960df34ee1 Mon Sep 17 00:00:00 2001 From: amos402 Date: Mon, 21 Dec 2020 20:14:04 +0800 Subject: [PATCH 3/6] * Remove magic offset * Add Size field to TypeOffset * Replenish lacked used offset --- src/runtime/interop.cs | 11 ++--------- src/runtime/metatype.cs | 2 +- src/runtime/native/ABI.cs | 2 -- src/runtime/native/GeneratedTypeOffsets.cs | 4 ++++ src/runtime/native/ITypeOffsets.cs | 3 +++ src/runtime/native/TypeOffset.cs | 7 ++++++- src/runtime/typemanager.cs | 2 +- 7 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/runtime/interop.cs b/src/runtime/interop.cs index ef67bd3a9..d2b763026 100644 --- a/src/runtime/interop.cs +++ b/src/runtime/interop.cs @@ -68,22 +68,15 @@ public ModulePropertyAttribute() } } - internal static partial class TypeOffset - { - public static int magic() => ManagedDataOffsets.Magic; - } - - internal static class ManagedDataOffsets { - public static int Magic { get; internal set; } public static readonly Dictionary NameMapping = new Dictionary(); static class DataOffsets { public static readonly int ob_data = 0; public static readonly int ob_dict = 0; - public static readonly int ob_weaklist; + public static readonly int ob_weaklist = 0; static DataOffsets() { @@ -183,7 +176,7 @@ static ObjectOffset() public static int magic(IntPtr type) { - return (int)Marshal.ReadIntPtr(type, TypeOffset.members); + return (int)Marshal.ReadIntPtr(type, TypeOffset.Size); } public static int TypeDictOffset(IntPtr type) diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs index b891dc400..ced2dfcbd 100644 --- a/src/runtime/metatype.cs +++ b/src/runtime/metatype.cs @@ -360,7 +360,7 @@ struct ClrMetaTypeEx public static unsafe ClrMetaTypeEx* FromType(IntPtr type) { - return (ClrMetaTypeEx*)(type + TypeOffset.members); + return (ClrMetaTypeEx*)(type + TypeOffset.Size); } } diff --git a/src/runtime/native/ABI.cs b/src/runtime/native/ABI.cs index 76337c797..e98715a6c 100644 --- a/src/runtime/native/ABI.cs +++ b/src/runtime/native/ABI.cs @@ -25,8 +25,6 @@ internal static void Initialize(Version version, BorrowedReference pyType) throw new NotSupportedException($"Python ABI v{version} is not supported"); var typeOffsets = (ITypeOffsets)Activator.CreateInstance(typeOffsetsClass); TypeOffset.Use(typeOffsets); - - ManagedDataOffsets.Magic = Marshal.ReadInt32(pyType.DangerousGetAddress(), TypeOffset.tp_basicsize); } } } diff --git a/src/runtime/native/GeneratedTypeOffsets.cs b/src/runtime/native/GeneratedTypeOffsets.cs index bf10df124..777437622 100644 --- a/src/runtime/native/GeneratedTypeOffsets.cs +++ b/src/runtime/native/GeneratedTypeOffsets.cs @@ -7,6 +7,9 @@ namespace Python.Runtime.Native [StructLayout(LayoutKind.Sequential)] abstract class GeneratedTypeOffsets { + // XXX: Use field to trick the public property finding + public readonly int Size; + protected GeneratedTypeOffsets() { var type = this.GetType(); @@ -17,6 +20,7 @@ protected GeneratedTypeOffsets() int offset = fieldIndex * fieldSize; offsetProperties[fieldIndex].SetValue(this, offset, index: null); } + Size = offsetProperties.Length * fieldSize; } } } diff --git a/src/runtime/native/ITypeOffsets.cs b/src/runtime/native/ITypeOffsets.cs index 31344c66d..318f35eea 100644 --- a/src/runtime/native/ITypeOffsets.cs +++ b/src/runtime/native/ITypeOffsets.cs @@ -18,6 +18,7 @@ interface ITypeOffsets int nb_inplace_add { get; } int nb_inplace_subtract { get; } int ob_size { get; } + int ob_refcnt { get; } int ob_type { get; } int qualname { get; } int sq_contains { get; } @@ -37,6 +38,7 @@ interface ITypeOffsets int tp_descr_set { get; } int tp_dict { get; } int tp_dictoffset { get; } + int tp_init { get; } int tp_flags { get; } int tp_free { get; } int tp_getattro { get; } @@ -51,6 +53,7 @@ interface ITypeOffsets int tp_new { get; } int tp_repr { get; } int tp_richcompare { get; } + int tp_weaklistoffset { get; } int tp_setattro { get; } int tp_str { get; } int tp_traverse { get; } diff --git a/src/runtime/native/TypeOffset.cs b/src/runtime/native/TypeOffset.cs index 0c51110da..f415b94e9 100644 --- a/src/runtime/native/TypeOffset.cs +++ b/src/runtime/native/TypeOffset.cs @@ -25,6 +25,7 @@ static partial class TypeOffset internal static int nb_inplace_add { get; private set; } internal static int nb_inplace_subtract { get; private set; } internal static int ob_size { get; private set; } + internal static int ob_refcnt { get; private set; } internal static int ob_type { get; private set; } internal static int qualname { get; private set; } internal static int sq_contains { get; private set; } @@ -44,6 +45,7 @@ static partial class TypeOffset internal static int tp_descr_set { get; private set; } internal static int tp_dict { get; private set; } internal static int tp_dictoffset { get; private set; } + internal static int tp_init { get; private set; } internal static int tp_flags { get; private set; } internal static int tp_free { get; private set; } internal static int tp_getattro { get; private set; } @@ -58,10 +60,13 @@ static partial class TypeOffset internal static int tp_new { get; private set; } internal static int tp_repr { get; private set; } internal static int tp_richcompare { get; private set; } + internal static int tp_weaklistoffset { get; private set; } internal static int tp_setattro { get; private set; } internal static int tp_str { get; private set; } internal static int tp_traverse { get; private set; } + internal static int Size; + internal static void Use(ITypeOffsets offsets) { if (offsets is null) throw new ArgumentNullException(nameof(offsets)); @@ -76,7 +81,7 @@ internal static void Use(ITypeOffsets offsets) int value = (int)sourceProperty.GetValue(offsets, null); offsetProperty.SetValue(obj: null, value: value, index: null); } - + Size = ((GeneratedTypeOffsets)offsets).Size; ValidateUnusedTypeOffsetProperties(offsetProperties); ValidateRequiredOffsetsPresent(offsetProperties); } diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index c5b2949cb..f1fb6a0a4 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -458,7 +458,7 @@ internal static IntPtr CreateMetaType(Type impl, out SlotsHolder slotsHolder) IntPtr py_type = Runtime.PyTypeType; var heapTypeSize = (int)Marshal.ReadIntPtr(py_type, TypeOffset.tp_basicsize); - Debug.Assert(heapTypeSize == TypeOffset.members); + Debug.Assert(heapTypeSize == TypeOffset.Size); int metaSize = heapTypeSize + Marshal.SizeOf(typeof(ClrMetaTypeEx)); IntPtr type = Runtime._PyObject_GC_Calloc(new IntPtr(metaSize)); From 958a8eb337617fead14266322eb2dfc81c0d81bb Mon Sep 17 00:00:00 2001 From: amos402 Date: Fri, 19 Feb 2021 22:56:12 +0800 Subject: [PATCH 4/6] Fix Shutdown crash --- pythonnet/__init__.py | 2 +- src/runtime/classbase.cs | 5 ++--- src/runtime/classmanager.cs | 3 ++- src/runtime/extensiontype.cs | 2 +- src/runtime/managedtype.cs | 10 ++++++++++ src/runtime/metatype.cs | 4 +--- src/runtime/methodobject.cs | 2 +- src/runtime/runtime.cs | 33 +++++++++++++++++++++++++-------- 8 files changed, 43 insertions(+), 18 deletions(-) diff --git a/pythonnet/__init__.py b/pythonnet/__init__.py index 692fd5700..4f162dcd5 100644 --- a/pythonnet/__init__.py +++ b/pythonnet/__init__.py @@ -63,7 +63,7 @@ def unload(): global _RUNTIME if _LOADER_ASSEMBLY is not None: func = _LOADER_ASSEMBLY["Python.Runtime.Loader.Shutdown"] - if func(b"") != 0: + if func(b"full_shutdown") != 0: raise RuntimeError("Failed to call Python.NET shutdown") if _RUNTIME is not None: diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs index 535af9706..a29e3a9b2 100644 --- a/src/runtime/classbase.cs +++ b/src/runtime/classbase.cs @@ -356,7 +356,8 @@ public static void tp_dealloc(IntPtr ob) { Runtime.PyObject_ClearWeakRefs(ob); } - tp_clear(ob); + RemoveObjectDict(ob); + Runtime.Py_CLEAR(ref self.tpHandle); Runtime.PyObject_GC_UnTrack(ob); Runtime.PyObject_GC_Del(ob); self.FreeGCHandle(); @@ -364,9 +365,7 @@ public static void tp_dealloc(IntPtr ob) public static int tp_clear(IntPtr ob) { - ManagedType self = GetManagedObject(ob); ClearObjectDict(ob); - Runtime.Py_CLEAR(ref self.tpHandle); return 0; } diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index 1ee06e682..5cad4bd69 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -5,6 +5,7 @@ using System.Runtime.InteropServices; using System.Security; using System.Linq; +using System.Diagnostics; namespace Python.Runtime { @@ -270,7 +271,7 @@ private static void InitClassBase(Type type, ClassBase impl) // Finally, initialize the class __dict__ and return the object. var dict = new BorrowedReference(Marshal.ReadIntPtr(tp, TypeOffset.tp_dict)); - + Debug.Assert(!dict.IsNull); if (impl.dotNetMembers == null) { diff --git a/src/runtime/extensiontype.cs b/src/runtime/extensiontype.cs index a5f0f1219..e6c95ecaa 100644 --- a/src/runtime/extensiontype.cs +++ b/src/runtime/extensiontype.cs @@ -55,7 +55,7 @@ void SetupGc () /// public static void FinalizeObject(ManagedType self) { - ClearObjectDict(self.pyHandle); + RemoveObjectDict(self.pyHandle); Runtime.PyObject_GC_Del(self.pyHandle); // Not necessary for decref of `tpHandle`. self.FreeGCHandle(); diff --git a/src/runtime/managedtype.cs b/src/runtime/managedtype.cs index 143bf2eb6..12aa7104f 100644 --- a/src/runtime/managedtype.cs +++ b/src/runtime/managedtype.cs @@ -225,6 +225,16 @@ protected virtual void OnSave(InterDomainContext context) { } protected virtual void OnLoad(InterDomainContext context) { } protected static void ClearObjectDict(IntPtr ob) + { + IntPtr dict = GetObjectDict(ob); + if (dict == IntPtr.Zero) + { + return; + } + Runtime.PyDict_Clear(dict); + } + + protected static void RemoveObjectDict(IntPtr ob) { IntPtr dict = GetObjectDict(ob); if (dict == IntPtr.Zero) diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs index 26a0ecb85..4aaf6fe41 100644 --- a/src/runtime/metatype.cs +++ b/src/runtime/metatype.cs @@ -296,9 +296,7 @@ public static void tp_dealloc(IntPtr tp) public static int tp_clear(IntPtr ob) { - IntPtr type = Runtime.PyObject_TYPE(ob); - int dictOffset = ObjectOffset.TypeDictOffset(type); - Runtime.Py_SETREF(ob, dictOffset, IntPtr.Zero); + ClearObjectDict(ob); return 0; } diff --git a/src/runtime/methodobject.cs b/src/runtime/methodobject.cs index 37c01f5c5..630f286da 100644 --- a/src/runtime/methodobject.cs +++ b/src/runtime/methodobject.cs @@ -217,7 +217,7 @@ public static IntPtr tp_repr(IntPtr ob) { var self = (MethodObject)GetManagedObject(ob); self.ClearMembers(); - ClearObjectDict(ob); + RemoveObjectDict(ob); self.Dealloc(); } diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 7c927c14c..cd581d381 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -503,15 +503,19 @@ private static void PyDictTryDelItem(BorrowedReference dict, string key) private static void MoveClrInstancesOnwershipToPython() { var objs = ManagedType.GetManagedObjects(); - var copyObjs = objs.ToArray(); - foreach (var entry in copyObjs) + var copyObjs = new KeyValuePair[objs.Count]; { - ManagedType obj = entry.Key; - if (!objs.ContainsKey(obj)) + int i = 0; + foreach (var entry in objs) { - System.Diagnostics.Debug.Assert(obj.gcHandle == default); - continue; + ManagedType obj = entry.Key; + XIncref(obj.pyHandle); + copyObjs[i++] = entry; } + } + foreach (var entry in copyObjs) + { + ManagedType obj = entry.Key; if (entry.Value == ManagedType.TrackTypes.Extension) { obj.CallTypeClear(); @@ -522,8 +526,21 @@ private static void MoveClrInstancesOnwershipToPython() PyObject_GC_Track(obj.pyHandle); } } - obj.FreeGCHandle(); - Marshal.WriteIntPtr(obj.pyHandle, ObjectOffset.magic(obj.tpHandle), IntPtr.Zero); + } + foreach (var entry in copyObjs) + { + ManagedType obj = entry.Key; + if (!objs.ContainsKey(obj)) + { + System.Diagnostics.Debug.Assert(obj.gcHandle == default); + continue; + } + if (obj.RefCount > 1) + { + obj.FreeGCHandle(); + Marshal.WriteIntPtr(obj.pyHandle, ObjectOffset.magic(obj.tpHandle), IntPtr.Zero); + } + XDecref(obj.pyHandle); } ManagedType.ClearTrackedObjects(); } From 53ad8d9b24ddab3cd4c6fa03137004c529ee465b Mon Sep 17 00:00:00 2001 From: amos402 Date: Fri, 19 Feb 2021 23:52:24 +0800 Subject: [PATCH 5/6] Use PyTypeType.tp_basicsize as size of PyHeapTypeObject directly instead of calculate it --- src/runtime/native/GeneratedTypeOffsets.cs | 4 ---- src/runtime/native/TypeOffset.cs | 3 ++- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/runtime/native/GeneratedTypeOffsets.cs b/src/runtime/native/GeneratedTypeOffsets.cs index 777437622..bf10df124 100644 --- a/src/runtime/native/GeneratedTypeOffsets.cs +++ b/src/runtime/native/GeneratedTypeOffsets.cs @@ -7,9 +7,6 @@ namespace Python.Runtime.Native [StructLayout(LayoutKind.Sequential)] abstract class GeneratedTypeOffsets { - // XXX: Use field to trick the public property finding - public readonly int Size; - protected GeneratedTypeOffsets() { var type = this.GetType(); @@ -20,7 +17,6 @@ protected GeneratedTypeOffsets() int offset = fieldIndex * fieldSize; offsetProperties[fieldIndex].SetValue(this, offset, index: null); } - Size = offsetProperties.Length * fieldSize; } } } diff --git a/src/runtime/native/TypeOffset.cs b/src/runtime/native/TypeOffset.cs index 726476310..3f3d6be85 100644 --- a/src/runtime/native/TypeOffset.cs +++ b/src/runtime/native/TypeOffset.cs @@ -8,6 +8,7 @@ namespace Python.Runtime using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; + using System.Runtime.InteropServices; using Python.Runtime.Native; @@ -93,7 +94,7 @@ internal static void Use(ITypeOffsets offsets) int value = (int)sourceProperty.GetValue(offsets, null); offsetProperty.SetValue(obj: null, value: value, index: null); } - Size = ((GeneratedTypeOffsets)offsets).Size; + Size = (int)Marshal.ReadIntPtr(Runtime.PyTypeType, tp_basicsize); ; ValidateUnusedTypeOffsetProperties(offsetProperties); ValidateRequiredOffsetsPresent(offsetProperties); } From f474039170b008e660235182e0e35acdf9232b4b Mon Sep 17 00:00:00 2001 From: amos402 Date: Sun, 21 Feb 2021 16:27:22 +0800 Subject: [PATCH 6/6] * Use C# 9.0 for Python.EmbeddingTest.csproj * Remove dead code --- src/embed_tests/Python.EmbeddingTest.csproj | 1 + src/embed_tests/TestClass.cs | 14 +++++--------- src/runtime/metatype.cs | 12 +----------- src/runtime/native/TypeOffset.cs | 2 +- 4 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/embed_tests/Python.EmbeddingTest.csproj b/src/embed_tests/Python.EmbeddingTest.csproj index 67a7d3338..099d026fa 100644 --- a/src/embed_tests/Python.EmbeddingTest.csproj +++ b/src/embed_tests/Python.EmbeddingTest.csproj @@ -2,6 +2,7 @@ net472;netcoreapp3.1 + 9.0 ..\pythonnet.snk true diff --git a/src/embed_tests/TestClass.cs b/src/embed_tests/TestClass.cs index 08a0a497d..cbcb92483 100644 --- a/src/embed_tests/TestClass.cs +++ b/src/embed_tests/TestClass.cs @@ -31,18 +31,14 @@ public void Dispose() public void WeakRefForClrObject() { var obj = new MyClass(); - using (var scope = Py.CreateScope()) - { - scope.Set("clr_obj", obj); - scope.Exec(@" + using var scope = Py.CreateScope(); + scope.Set("clr_obj", obj); + scope.Exec(@" import weakref ref = weakref.ref(clr_obj) "); - using (PyObject pyobj = scope.Get("clr_obj")) - { - ValidateAttachedGCHandle(obj, pyobj.Handle); - } - } + using PyObject pyobj = scope.Get("clr_obj"); + ValidateAttachedGCHandle(obj, pyobj.Handle); } [Test] diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs index 4aaf6fe41..3f4717190 100644 --- a/src/runtime/metatype.cs +++ b/src/runtime/metatype.cs @@ -155,16 +155,6 @@ public static IntPtr tp_new(IntPtr tp, IntPtr args, IntPtr kw) flags |= TypeFlags.Subclass; flags |= TypeFlags.HaveGC; Util.WriteCLong(type, TypeOffset.tp_flags, flags); - - //TypeManager.CopySlot(base_type, type, TypeOffset.tp_dealloc); - - // Hmm - the standard subtype_traverse, clear look at ob_size to - // do things, so to allow gc to work correctly we need to move - // our hidden handle out of ob_size. Then, in theory we can - // comment this out and still not crash. - //TypeManager.CopySlot(base_type, type, TypeOffset.tp_traverse); - //TypeManager.CopySlot(base_type, type, TypeOffset.tp_clear); - // for now, move up hidden handle... unsafe { @@ -361,7 +351,7 @@ public static IntPtr __subclasscheck__(IntPtr tp, IntPtr args) [StructLayout(LayoutKind.Sequential)] struct ClrMetaTypeEx { - public IntPtr ClrHandleOffset; + public nint ClrHandleOffset; public IntPtr ClrHandle; public static unsafe ClrMetaTypeEx* FromType(IntPtr type) diff --git a/src/runtime/native/TypeOffset.cs b/src/runtime/native/TypeOffset.cs index 3f3d6be85..62bd90b4a 100644 --- a/src/runtime/native/TypeOffset.cs +++ b/src/runtime/native/TypeOffset.cs @@ -94,7 +94,7 @@ internal static void Use(ITypeOffsets offsets) int value = (int)sourceProperty.GetValue(offsets, null); offsetProperty.SetValue(obj: null, value: value, index: null); } - Size = (int)Marshal.ReadIntPtr(Runtime.PyTypeType, tp_basicsize); ; + Size = (int)Marshal.ReadIntPtr(Runtime.PyTypeType, tp_basicsize); ValidateUnusedTypeOffsetProperties(offsetProperties); ValidateRequiredOffsetsPresent(offsetProperties); }