Skip to content

When reflecting nested types, ensure their corresponding PyType is allocated #1579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/embed_tests/ClassManagerTests.cs
Original file line number Diff line number Diff line change
@@ -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();
}
}
30 changes: 23 additions & 7 deletions src/runtime/classmanager.cs
Original file line number Diff line number Diff line change
@@ -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
{
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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__.
Expand Down Expand Up @@ -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;
Expand Down
43 changes: 26 additions & 17 deletions src/runtime/typemanager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

Expand Down