Fixed issue calling multiple levels of derived classes with some defining a method and some not.

rmadsen-ks · rmadsen-ks · commit bf389cf1490e · 2025-10-09T10:30:37.000+02:00
Also fixed an issue causing the finalizer to be called twice.

Added unit tests to verify the fix.
diff --git 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<string>();
+            var dictKeys = new HashSet<string>();
             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<string>();
+                    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;
         }
 
+
+
         /// <summary>
         /// Add a constructor override that calls the python ctor after calling the base type constructor.
         /// </summary>
@@ -368,7 +386,8 @@ private static void AddConstructor(ConstructorInfo ctor, Type baseType, TypeBuil
         /// <param name="method">virtual method to be overridden</param>
         /// <param name="baseType">Python callable object</param>
         /// <param name="typeBuilder">TypeBuilder for the new type the method is to be added to</param>
-        private static void AddVirtualMethod(MethodInfo method, Type baseType, TypeBuilder typeBuilder)
+        /// <param name="isDeclared"></param>
+        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<PyObject>();
                 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
@@ -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
@@ -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"

Original file line number	Diff line number	Diff line change
`@@ -70,5 +70,10 @@ public static void TestObject(object obj)`
`70`	`70`	`throw new Exception("Expected ISayHello and SimpleClass instance");`
`71`	`71`	`}`
`72`	`72`	`}`
	`73`	`+`
	`74`	`+ public virtual string SayGoodbye()`
	`75`	`+ {`
	`76`	`+ return "!";`
	`77`	`+ }`
`73`	`78`	`}`
`74`	`79`	`}`