When reflecting nested types, ensure their corresponding PyType is allocated.

lostmsu · lostmsu · commit f857d59e66bc · 2021-10-01T13:06:47.000-07:00
fixes #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.
diff --git a/src/embed_tests/ClassManagerTests.cs 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
@@ -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<ManagedType, InterDomainContext> 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
@@ -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;
         }
 
