From e76207287059e818df826eed267403f09c54ea72 Mon Sep 17 00:00:00 2001 From: romadsen-ks Date: Wed, 8 Oct 2025 22:58:27 +0200 Subject: [PATCH 1/3] Fixed issue calling multiple levels of derived classes with some defining a method and some not. Also fixed an issue causing the finalizer to be called twice. Added unit tests to verify the fix. --- src/runtime/Types/ClassDerived.cs | 62 +++++++++++++++++++++---------- src/testing/classtest.cs | 5 +++ tests/test_subclass.py | 36 +++++++++++++++++- 3 files changed, 82 insertions(+), 21 deletions(-) diff --git a/src/runtime/Types/ClassDerived.cs b/src/runtime/Types/ClassDerived.cs index 592eefd55..0789fbb99 100644 --- a/src/runtime/Types/ClassDerived.cs +++ b/src/runtime/Types/ClassDerived.cs @@ -198,12 +198,15 @@ internal static Type CreateDerivedType(string name, // Override any properties explicitly overridden in python var pyProperties = new HashSet(); + var dictKeys = new HashSet(); if (py_dict != null && Runtime.PyDict_Check(py_dict)) { using var dict = new PyDict(py_dict); using var keys = dict.Keys(); foreach (PyObject pyKey in keys) { + var keyString = pyKey.As(); + dictKeys.Add(keyString); using var value = dict[pyKey]; if (value.HasAttr("_clr_property_type_")) { @@ -239,11 +242,18 @@ internal static Type CreateDerivedType(string name, continue; } + // if the name of the method is not in the dict keys, then the method is not explicitly + // declared in the python code and we dont need to add it here. + bool isDeclared = dictKeys.Contains(method.Name); + if (!isDeclared) + continue; + // keep track of the virtual methods redirected to the python instance virtualMethods.Add(method.Name); + // override the virtual method to call out to the python method, if there is one. - AddVirtualMethod(method, baseType, typeBuilder); + AddVirtualMethod(method, baseType, typeBuilder, isDeclared); } // Add any additional methods and properties explicitly exposed from Python. @@ -271,35 +281,43 @@ internal static Type CreateDerivedType(string name, } } - // add the destructor so the python object created in the constructor gets destroyed - MethodBuilder methodBuilder = typeBuilder.DefineMethod("Finalize", - MethodAttributes.Family | - MethodAttributes.Virtual | - MethodAttributes.HideBySig, - CallingConventions.Standard, - typeof(void), - Type.EmptyTypes); - ILGenerator il = methodBuilder.GetILGenerator(); - il.Emit(OpCodes.Ldarg_0); + + // only add finalizer if it has not allready been added on a base type. + // otherwise PyFinalize will be called multiple times for the same object, + // causing an access violation exception on some platforms. + // to see if this is the case, we can check if the base type is a IPythonDerivedType if so, it already + // has the finalizer. + if (typeof(IPythonDerivedType).IsAssignableFrom(baseType) == false) + { + // add the destructor so the python object created in the constructor gets destroyed + MethodBuilder methodBuilder = typeBuilder.DefineMethod("Finalize", + MethodAttributes.Family | + MethodAttributes.Virtual | + MethodAttributes.HideBySig, + CallingConventions.Standard, + typeof(void), + Type.EmptyTypes); + ILGenerator il = methodBuilder.GetILGenerator(); + il.Emit(OpCodes.Ldarg_0); #pragma warning disable CS0618 // PythonDerivedType is for internal use only - il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod(nameof(PyFinalize))); + il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod(nameof(PyFinalize))); #pragma warning restore CS0618 // PythonDerivedType is for internal use only - il.Emit(OpCodes.Ldarg_0); - il.Emit(OpCodes.Call, baseClass.GetMethod("Finalize", BindingFlags.NonPublic | BindingFlags.Instance)); - il.Emit(OpCodes.Ret); + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Call, baseClass.GetMethod("Finalize", BindingFlags.NonPublic | BindingFlags.Instance)); + il.Emit(OpCodes.Ret); + } Type type = typeBuilder.CreateType(); - // scan the assembly so the newly added class can be imported + // scan the assembly so the newly added class can be imported. Assembly assembly = Assembly.GetAssembly(type); AssemblyManager.ScanAssembly(assembly); - // FIXME: assemblyBuilder not used - AssemblyBuilder assemblyBuilder = assemblyBuilders[assemblyName]; - return type; } + + /// /// Add a constructor override that calls the python ctor after calling the base type constructor. /// @@ -368,7 +386,8 @@ private static void AddConstructor(ConstructorInfo ctor, Type baseType, TypeBuil /// virtual method to be overridden /// Python callable object /// TypeBuilder for the new type the method is to be added to - private static void AddVirtualMethod(MethodInfo method, Type baseType, TypeBuilder typeBuilder) + /// + private static void AddVirtualMethod(MethodInfo method, Type baseType, TypeBuilder typeBuilder, bool isDeclared) { ParameterInfo[] parameters = method.GetParameters(); Type[] parameterTypes = (from param in parameters select param.ParameterType).ToArray(); @@ -720,12 +739,15 @@ public class PythonDerivedType { var disposeList = new List(); PyGILState gs = Runtime.PyGILState_Ensure(); + + try { using var pyself = new PyObject(self.CheckRun()); using PyObject method = pyself.GetAttr(methodName, Runtime.None); if (method.Reference != Runtime.PyNone) { + // if the method hasn't been overridden then it will be a managed object ManagedType? managedMethod = ManagedType.GetManagedObject(method.Reference); if (null == managedMethod) diff --git a/src/testing/classtest.cs b/src/testing/classtest.cs index 993afdfc9..d7ca833cc 100644 --- a/src/testing/classtest.cs +++ b/src/testing/classtest.cs @@ -70,5 +70,10 @@ public static void TestObject(object obj) throw new Exception("Expected ISayHello and SimpleClass instance"); } } + + public virtual string SayGoodbye() + { + return "!"; + } } } diff --git a/tests/test_subclass.py b/tests/test_subclass.py index c6ab7650f..5070006c4 100644 --- a/tests/test_subclass.py +++ b/tests/test_subclass.py @@ -339,8 +339,42 @@ class OverloadingSubclass2(OverloadingSubclass): assert obj.VirtMethod[int](5) == 5 def test_implement_interface_and_class(): + import clr + class DualSubClass0(ISayHello1, SimpleClass): + __namespace__ = "Test0" + def SayHello(self): + return "hello" + + @clr.clrmethod(str, []) + def SayGoodbye(self): + return "bye" + super().SayGoodbye() + +def test_multi_level_subclass(): + """ + Test multi levels of subclassing. This has shown verious issues, like stack overflow + exception if a method was not implemented in the middle of the tree. + """ + import clr class DualSubClass0(ISayHello1, SimpleClass): __namespace__ = "Test" def SayHello(self): return "hello" - obj = DualSubClass0() + def SayGoodbye(self): + return "bye" + super().SayGoodbye() + class DualSubClass1(DualSubClass0): + __namespace__ = "Test" + def SayHello(self): + return super().SayHello() + " hi1" + class DualSubClass2(DualSubClass1): + __namespace__ = "Test" + class DualSubClass3(DualSubClass2): + __namespace__ = "Test" + def SayHello(self): + return super().SayHello() + " hi3" + def SayGoodbye(self): + return super().SayGoodbye() + "!" + obj = DualSubClass3() + helloResult = obj.SayHello() + goodByeResult = obj.SayGoodbye() + assert goodByeResult =="bye!!" + assert helloResult == "hello hi1 hi3" From 0c995516ac3066d6143fd33956a49a527a04ada7 Mon Sep 17 00:00:00 2001 From: romadsen-ks Date: Thu, 9 Oct 2025 16:02:48 +0200 Subject: [PATCH 2/3] Added fix for calling virtual method on baseclass which is not defined. --- src/runtime/Types/ClassDerived.cs | 2 +- src/testing/classtest.cs | 5 +++++ tests/test_subclass.py | 4 ++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/runtime/Types/ClassDerived.cs b/src/runtime/Types/ClassDerived.cs index 0789fbb99..e03252e00 100644 --- a/src/runtime/Types/ClassDerived.cs +++ b/src/runtime/Types/ClassDerived.cs @@ -396,7 +396,7 @@ private static void AddVirtualMethod(MethodInfo method, Type baseType, TypeBuild string? baseMethodName = null; if (!method.IsAbstract) { - baseMethodName = "_" + baseType.Name + "__" + method.Name; + baseMethodName = "_" + method.DeclaringType.Name + "__" + method.Name; MethodBuilder baseMethodBuilder = typeBuilder.DefineMethod(baseMethodName, MethodAttributes.Public | MethodAttributes.Final | diff --git a/src/testing/classtest.cs b/src/testing/classtest.cs index d7ca833cc..e7c224518 100644 --- a/src/testing/classtest.cs +++ b/src/testing/classtest.cs @@ -76,4 +76,9 @@ public virtual string SayGoodbye() return "!"; } } + + public class SimpleClass2 : SimpleClass + { + // this class does not override SayGoodbye. + } } diff --git a/tests/test_subclass.py b/tests/test_subclass.py index 5070006c4..57d6df2b4 100644 --- a/tests/test_subclass.py +++ b/tests/test_subclass.py @@ -9,7 +9,7 @@ import System import pytest from Python.Test import (IInterfaceTest, SubClassTest, EventArgsTest, - FunctionsTest, IGenericInterface, GenericVirtualMethodTest, SimpleClass, ISayHello1) + FunctionsTest, IGenericInterface, GenericVirtualMethodTest, SimpleClass, SimpleClass2, ISayHello1) from System.Collections.Generic import List @@ -355,7 +355,7 @@ def test_multi_level_subclass(): exception if a method was not implemented in the middle of the tree. """ import clr - class DualSubClass0(ISayHello1, SimpleClass): + class DualSubClass0(ISayHello1, SimpleClass2): __namespace__ = "Test" def SayHello(self): return "hello" From 98a740ec264b708584a8ff7cb9821e9a0934045c Mon Sep 17 00:00:00 2001 From: Rolf Madsen Date: Tue, 14 Oct 2025 10:31:31 +0200 Subject: [PATCH 3/3] minor cleanup --- src/runtime/Types/ClassDerived.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/runtime/Types/ClassDerived.cs b/src/runtime/Types/ClassDerived.cs index e03252e00..75c596fac 100644 --- a/src/runtime/Types/ClassDerived.cs +++ b/src/runtime/Types/ClassDerived.cs @@ -253,7 +253,7 @@ internal static Type CreateDerivedType(string name, // override the virtual method to call out to the python method, if there is one. - AddVirtualMethod(method, baseType, typeBuilder, isDeclared); + AddVirtualMethod(method, typeBuilder); } // Add any additional methods and properties explicitly exposed from Python. @@ -384,10 +384,8 @@ private static void AddConstructor(ConstructorInfo ctor, Type baseType, TypeBuil /// and calls it, otherwise fall back to the base class method. /// /// virtual method to be overridden - /// Python callable object /// TypeBuilder for the new type the method is to be added to - /// - private static void AddVirtualMethod(MethodInfo method, Type baseType, TypeBuilder typeBuilder, bool isDeclared) + private static void AddVirtualMethod(MethodInfo method, TypeBuilder typeBuilder) { ParameterInfo[] parameters = method.GetParameters(); Type[] parameterTypes = (from param in parameters select param.ParameterType).ToArray();