From 71e465fd45b936bd64e48bb520b3e43f67a8e434 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Fri, 1 Oct 2021 11:54:01 -0700 Subject: [PATCH] When reflecting nested types, ensure their corresponding PyType is allocated. fixes https://github.com/pythonnet/pythonnet/issues/1414 Without this fix attempting to use a nested .NET class that derives from its parent would cause a crash due to false mutual dependency. --- src/embed_tests/ClassManagerTests.cs | 40 ++++++++++++++++++++++++++ src/runtime/classmanager.cs | 30 ++++++++++++++----- src/runtime/typemanager.cs | 43 +++++++++++++++++----------- 3 files changed, 89 insertions(+), 24 deletions(-) create mode 100644 src/embed_tests/ClassManagerTests.cs diff --git a/src/embed_tests/ClassManagerTests.cs b/src/embed_tests/ClassManagerTests.cs new file mode 100644 index 000000000..72025a28b --- /dev/null +++ b/src/embed_tests/ClassManagerTests.cs @@ -0,0 +1,40 @@ +using NUnit.Framework; + +using Python.Runtime; + +namespace Python.EmbeddingTest +{ + public class ClassManagerTests + { + [OneTimeSetUp] + public void SetUp() + { + PythonEngine.Initialize(); + } + + [OneTimeTearDown] + public void Dispose() + { + PythonEngine.Shutdown(); + } + + [Test] + public void NestedClassDerivingFromParent() + { + var f = new NestedTestContainer().ToPython(); + f.GetAttr(nameof(NestedTestContainer.Bar)); + } + } + + public class NestedTestParent + { + public class Nested : NestedTestParent + { + } + } + + public class NestedTestContainer + { + public NestedTestParent Bar = new NestedTestParent.Nested(); + } +} diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index 55c330af7..589ac0ad1 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -1,10 +1,11 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; using System.Reflection; using System.Runtime.InteropServices; using System.Security; -using System.Linq; namespace Python.Runtime { @@ -151,8 +152,12 @@ internal static Dictionary RestoreRuntimeData(R invalidClasses.Add(pair); continue; } + // Ensure, that matching Python type exists first. + // It is required for self-referential classes + // (e.g. with members, that refer to the same class) + var pyType = InitPyType(pair.Key.Value, pair.Value); // re-init the class - InitClassBase(pair.Key.Value, pair.Value); + InitClassBase(pair.Key.Value, pair.Value, pyType); // We modified the Type object, notify it we did. Runtime.PyType_Modified(pair.Value.TypeReference); var context = contexts[pair.Value.pyHandle]; @@ -184,9 +189,13 @@ internal static ClassBase GetClass(Type type) } cb = CreateClass(type); cache.Add(type, cb); + // Ensure, that matching Python type exists first. + // It is required for self-referential classes + // (e.g. with members, that refer to the same class) + var pyType = InitPyType(type, cb); // Initialize the object later, as this might call this GetClass method // recursively (for example when a nested class inherits its declaring class...) - InitClassBase(type, cb); + InitClassBase(type, cb, pyType); return cb; } @@ -249,16 +258,18 @@ private static ClassBase CreateClass(Type type) return impl; } - private static void InitClassBase(Type type, ClassBase impl) + private static PyType InitPyType(Type type, ClassBase impl) { - // Ensure, that matching Python type exists first. - // It is required for self-referential classes - // (e.g. with members, that refer to the same class) var pyType = TypeManager.GetOrCreateClass(type); // Set the handle attributes on the implementing instance. impl.tpHandle = impl.pyHandle = pyType.Handle; + return pyType; + } + + private static void InitClassBase(Type type, ClassBase impl, PyType pyType) + { // First, we introspect the managed type and build some class // information, including generating the member descriptors // that we'll be putting in the Python class __dict__. @@ -549,6 +560,11 @@ private static ClassInfo GetClassInfo(Type type) } // Note the given instance might be uninitialized ob = GetClass(tp); + if (ob.pyHandle == IntPtr.Zero && ob is ClassObject) + { + ob.pyHandle = ob.tpHandle = TypeManager.GetOrCreateClass(tp).Handle; + } + Debug.Assert(ob.pyHandle != IntPtr.Zero); // GetClass returns a Borrowed ref. ci.members owns the reference. ob.IncrRefCount(); ci.members[mi.Name] = ob; diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index 8db3516ac..1d6321791 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -146,7 +146,17 @@ internal static PyType GetOrCreateClass(Type type) { if (!cache.TryGetValue(type, out var pyType)) { - pyType = CreateClass(type); + pyType = AllocateClass(type); + cache.Add(type, pyType); + try + { + InitializeClass(type, pyType); + } + catch + { + cache.Remove(type); + throw; + } } return pyType; } @@ -209,12 +219,25 @@ internal static unsafe PyType CreateType(Type impl) } - static PyType CreateClass(Type clrType) + static void InitializeClass(Type clrType, PyType pyType) { - string name = GetPythonTypeName(clrType); + if (pyType.BaseReference != null) + { + return; + } using var baseTuple = GetBaseTypeTuple(clrType); + InitializeBases(pyType, baseTuple); + // core fields must be initialized in partially constructed classes, + // otherwise it would be impossible to manipulate GCHandle and check type size + InitializeCoreFields(pyType); + } + + static PyType AllocateClass(Type clrType) + { + string name = GetPythonTypeName(clrType); + IntPtr type = AllocateTypeObject(name, Runtime.PyCLRMetaType); var pyType = new PyType(StolenReference.DangerousFromPointer(type)); pyType.Flags = TypeFlags.Default @@ -223,20 +246,6 @@ static PyType CreateClass(Type clrType) | TypeFlags.BaseType | TypeFlags.HaveGC; - cache.Add(clrType, pyType); - try - { - InitializeBases(pyType, baseTuple); - // core fields must be initialized in partically constructed classes, - // otherwise it would be impossible to manipulate GCHandle and check type size - InitializeCoreFields(pyType); - } - catch - { - cache.Remove(clrType); - throw; - } - return pyType; }