Delay resolving super classes until referenced

src/main/c/jpy_jobj.c

@@ -726,11 +726,14 @@ int JType_InitSlots(JPy_JType* type)
 
     typeObj = JTYPE_AS_PYTYPE(type);
 
-    #if defined(JPY_COMPAT_39P)
-        Py_SET_REFCNT(typeObj, 1);
-    #else
-        Py_REFCNT(typeObj) = 1;
-    #endif
+    // This is hacky to make the caller JType_GetType able to call JPy_INCREF regardless if the type was already created,
+    // but it will cause a ref counting error when this function fails, and we need to remove the type from the registry.
+    // The proper ref counting needs to be done in JType_GetType().
+    //    #if defined(JPY_COMPAT_39P)
+    //        Py_SET_REFCNT(typeObj, 1);
+    //    #else
+    //        Py_REFCNT(typeObj) = 1;
+    //    #endif
     Py_SET_TYPE(typeObj, NULL);
     Py_SET_SIZE(typeObj, 0);
     // todo: The following lines are actually correct, but setting Py_TYPE(type) = &JType_Type results in an interpreter crash. Why?

src/main/c/jpy_jtype.c

@@ -171,16 +171,29 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
         //printf("T2: type->tp_init=%p\n", ((PyTypeObject*)type)->tp_init);
 
         // ... before we can continue processing the super type ...
-        if (JType_InitSuperType(jenv, type, resolve) < 0) {
+        // Note, at this point, the Python type has been registered-but-yet-finalized (as done in JType_InitSlots).
+        // We need to delay resolving the super classes to when they are actually referenced, so that in a cyclic
+        // reference scenario, the super classes can still be finalized (reference-able by child classes), but NOT RESOLVED
+        // (its methods/fields remain absent in the type object). This is to avoid the case where its members reference some other
+        // registered-but-yet-finalized classes in the same class hierarchy, and these classes in turn have
+        // registered-but-yet-finalized super classes, causing JType_InitSlots to fail for one of them because its super
+        // class is not finalized. When this happens, the affected classes will not be properly resolved.
+        if (JType_InitSuperType(jenv, type, JNI_FALSE) < 0) {
+            JPy_DIAG_PRINT(JPy_DIAG_F_TYPE, "JType_GetType: error: JType_InitSuperType() failed for javaName=\"%s\"\n", type->javaName);
             PyDict_DelItem(JPy_Types, typeKey);
+            JPy_DECREF(typeKey);
+            JPy_DECREF(type);
             return NULL;
         }
 
         //printf("T3: type->tp_init=%p\n", ((PyTypeObject*)type)->tp_init);
 
         // ... and processing the component type.
-        if (JType_InitComponentType(jenv, type, resolve) < 0) {
+        if (JType_InitComponentType(jenv, type, JNI_FALSE) < 0) {
+            JPy_DIAG_PRINT(JPy_DIAG_F_TYPE, "JType_GetType: error: JType_InitComponentType() failed for javaName=\"%s\"\n", type->javaName);
             PyDict_DelItem(JPy_Types, typeKey);
+            JPy_DECREF(typeKey);
+            JPy_DECREF(type);
             return NULL;
         }
 
@@ -190,10 +203,14 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
         if (JType_InitSlots(type) < 0) {
             JPy_DIAG_PRINT(JPy_DIAG_F_TYPE, "JType_GetType: error: JType_InitSlots() failed for javaName=\"%s\"\n", type->javaName);
             PyDict_DelItem(JPy_Types, typeKey);
+            JPy_DECREF(typeKey);
+            JPy_DECREF(type);
             return NULL;
         }
 
         JType_AddClassAttribute(jenv, type);
+        // 'type' will be returned as a new reference. 'typeKey' needs to be released.
+        JPy_DECREF(typeKey);
 
         //printf("T5: type->tp_init=%p\n", ((PyTypeObject*)type)->tp_init);
 
@@ -215,20 +232,22 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
             return NULL;
         }
 
-        JPy_DECREF(typeKey);
         type = (JPy_JType*) typeValue;
+        // 'type' will be returned as a new reference. 'typeKey' needs to be released.
+        JPy_INCREF(type);
+        JPy_DECREF(typeKey);
     }
 
     JPy_DIAG_PRINT(JPy_DIAG_F_TYPE, "JType_GetType: javaName=\"%s\", found=%d, resolve=%d, resolved=%d, type=%p\n", type->javaName, found, resolve, type->isResolved, type);
 
     if (!type->isResolved && resolve) {
         if (JType_ResolveType(jenv, type) < 0) {
+            JPy_DECREF(type);
             return NULL;
         }
     }
-
-    JPy_INCREF(type);
-    return type;
+
+     return type;
 }
 
 /**

src/main/c/jpy_module.c

@@ -800,11 +800,7 @@ PyObject* JType_CreateJavaByteBufferObj(JNIEnv* jenv, PyObject* pyObj)
 
 PyObject* JPy_byte_buffer_internal(JNIEnv* jenv, PyObject* self, PyObject* args)
 {
-    jobject byteBufferRef;
     PyObject* pyObj;
-    PyObject* newPyObj;
-    Py_buffer *pyBuffer;
-    JPy_JByteBufferObj* byteBuffer;
 
     if (!PyArg_ParseTuple(args, "O:byte_buffer", &pyObj)) {
         return NULL;

src/test/java/org/jpy/fixtures/CyclicReferenceChild1.java

@@ -0,0 +1,30 @@
+//
+// Copyright 2024 jpy-consortium
+//
+
+package org.jpy.fixtures;
+
+import java.lang.reflect.Array;
+import java.lang.reflect.Method;
+
+/**
+ * Used as a test class for the test cases in jpy_gettype_test.py
+ *
+ * @author Jianfeng Mao
+ */
+@SuppressWarnings("UnusedDeclaration")
+public class CyclicReferenceChild1 extends CyclicReferenceParent {
+    private int x;
+
+    private CyclicReferenceChild1(int x) {
+        this.x = x;
+    }
+
+    public static CyclicReferenceChild1 of(int x) {
+        return new CyclicReferenceChild1(x);
+    }
+
+    public int get_x() {
+        return this.x;
+    }
+}

src/test/java/org/jpy/fixtures/CyclicReferenceChild2.java

@@ -0,0 +1,17 @@
+//
+// Copyright 2024 jpy-consortium
+//
+
+package org.jpy.fixtures;
+
+import java.lang.reflect.Array;
+import java.lang.reflect.Method;
+
+/**
+ * Used as a test class for the test cases in jpy_gettype_test.py
+ *
+ * @author Jianfeng Mao
+ */
+@SuppressWarnings("UnusedDeclaration")
+public class CyclicReferenceChild2 extends CyclicReferenceParent {
+}

src/test/java/org/jpy/fixtures/CyclicReferenceGrandParent.java

@@ -0,0 +1,33 @@
+//
+// Copyright 2024 jpy-consortium
+//
+
+package org.jpy.fixtures;
+
+import java.lang.reflect.Array;
+import java.lang.reflect.Method;
+
+/**
+ * Used as a test class for the test cases in jpy_gettype_test.py
+ *
+ * @author Jianfeng Mao
+ */
+@SuppressWarnings("UnusedDeclaration")
+public class CyclicReferenceGrandParent {
+    private int x;
+    public int z = 100;
+
+    public CyclicReferenceGrandParent() {
+    }
+
+    public void refChild2(CyclicReferenceChild2 child2) {
+    }
+
+    public int grandParentMethod() {
+        return 888;
+    }
+
+    public int get_x() {
+        return this.x;
+    }
+}

src/test/java/org/jpy/fixtures/CyclicReferenceParent.java

@@ -0,0 +1,32 @@
+//
+// Copyright 2024 jpy-consortium
+//
+
+package org.jpy.fixtures;
+
+import java.lang.reflect.Array;
+import java.lang.reflect.Method;
+
+/**
+ * Used as a test class for the test cases in jpy_gettype_test.py
+ *
+ * @author Jianfeng Mao
+ */
+@SuppressWarnings("UnusedDeclaration")
+public abstract class CyclicReferenceParent extends CyclicReferenceGrandParent {
+    private int x;
+    public int y = 10;
+
+    public static CyclicReferenceChild1 of(int x) {
+        return CyclicReferenceChild1.of(x);
+    }
+
+    public int parentMethod() {
+        return 88;
+    }
+
+    public int get_x() {
+        return this.x;
+    }
+
+}

src/test/python/jpy_gettype_test.py

@@ -71,6 +71,21 @@ def test_issue_74(self):
             for i in range(200):
                 jpy.get_type(java_type)
 
+    def test_cyclic_reference(self):
+        """
+        Test if delaying resolving super classes breaks existing class reference pattern.
+        """
+        j_child1_class = jpy.get_type("org.jpy.fixtures.CyclicReferenceChild1")
+        j_child2_class = jpy.get_type("org.jpy.fixtures.CyclicReferenceChild2")
+        j_child2 = j_child2_class()
+
+        j_child1 = j_child1_class.of(8)
+        self.assertEqual(88, j_child1.parentMethod())
+        self.assertEqual(888, j_child1.grandParentMethod())
+        self.assertIsNone(j_child1.refChild2(j_child2))
+        self.assertEqual(8, j_child1.get_x())
+        self.assertEqual(10, j_child1.y)
+        self.assertEqual(100, j_child1.z)
 
 
 if __name__ == '__main__':