Delay resolving super classes until referenced (#146)

* Delay resolving super classes till referenced

* Fix a ref counting issue that led to crash

in a PyType_Ready failure situation

* Split the changes to keep the issue relevant ones

* Trivial code cleanup

* Minor code refactoring and more comments

* Add a test for array component tyep resolution

* Remove unwanted comments and wrong refcounting

* Update src/test/python/jpy_gettype_test.py

src/main/c/jpy_jobj.c

@@ -726,11 +726,6 @@ 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
     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

@@ -156,7 +156,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
 
         found = JNI_FALSE;
 
-        // Create a new type instance
+        // Create a new type instance, the refcount is set to 1 if this call succeeds
         type = JType_New(jenv, classRef, resolve);
         if (type == NULL) {
             JPy_DECREF(typeKey);
@@ -166,21 +166,35 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
         //printf("T1: type->tp_init=%p\n", ((PyTypeObject*)type)->tp_init);
 
         // In order to avoid infinite recursion, we have to register the new (but yet incomplete) type first...
+        // it will increment the refcount of type
         PyDict_SetItem(JPy_Types, typeKey, (PyObject*) type);
 
         //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 +204,15 @@ 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 refcount incremented and returned as a new reference later. 'typeKey' needs to be released.
+        JPy_DECREF(typeKey);
+        JPy_DECREF(type);
 
         //printf("T5: type->tp_init=%p\n", ((PyTypeObject*)type)->tp_init);
 
@@ -215,8 +234,9 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
             return NULL;
         }
 
-        JPy_DECREF(typeKey);
         type = (JPy_JType*) typeValue;
+        // 'type' will be refcount incremented and returned as a new reference. 'typeKey' needs to be released.
+        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);
@@ -226,7 +246,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
             return NULL;
         }
     }
-    
+
     JPy_INCREF(type);
     return type;
 }
@@ -1074,7 +1094,6 @@ int JType_InitComponentType(JNIEnv* jenv, JPy_JType* type, jboolean resolve)
         if (type->componentType == NULL) {
             return -1;
         }
-        JPy_INCREF(type->componentType);
     } else {
         type->componentType = NULL;
     }
@@ -1092,7 +1111,6 @@ int JType_InitSuperType(JNIEnv* jenv, JPy_JType* type, jboolean resolve)
         if (type->superType == NULL) {
             return -1;
         }
-        JPy_INCREF(type->superType);
         JPy_DELETE_LOCAL_REF(superClassRef);
     } else if (type->isInterface && JPy_JObject != NULL) {
         // This solves the problems that java.lang.Object methods can not be called on interfaces (https://github.com/bcdev/jpy/issues/57)

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,37 @@
+//
+// 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;
+    }
+
+    public CyclicReferenceChild2[] getChild2s() {
+        CyclicReferenceChild2[] child2s = new CyclicReferenceChild2[2];
+        child2s[0] = new CyclicReferenceChild2();
+        child2s[1] = new CyclicReferenceChild2();
+        return child2s;
+    }
+}

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

@@ -0,0 +1,20 @@
+//
+// 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 {
+    public String getName() {
+        return "Child2: " + this.toString();
+    }
+}

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/java/org/jpy/fixtures/GetTypeFailureChild.java

@@ -0,0 +1,26 @@
+//
+// 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 GetTypeFailureChild extends GetTypeFailureParent {
+    private int x;
+
+    private GetTypeFailureChild(int x) {
+        this.x = x;
+    }
+
+    public static GetTypeFailureChild of(int x) {
+        return new GetTypeFailureChild(x);
+    }
+}

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

@@ -0,0 +1,24 @@
+//
+// 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 GetTypeFailureParent {
+    static {
+        toFail();
+    }
+
+    static void toFail() {
+        throw new RuntimeException("Can't be loaded!");
+    }
+}

src/test/python/jpy_gettype_test.py

@@ -71,6 +71,33 @@ 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)
+
+    def test_component_type_resolution(self):
+        j_child1_class = jpy.get_type("org.jpy.fixtures.CyclicReferenceChild1")
+        j_child1 = j_child1_class.of(8)
+        j_child2s = j_child1.getChild2s()
+        self.assertIn("[Lorg.jpy.fixtures.CyclicReferenceChild2;", repr(type(j_child2s)))
+        for j_child2 in j_child2s:
+            self.assertTrue(j_child2.getName().startswith("Child2"))
+
+    def test_fail_init_supertype(self):
+        with self.assertRaises(ValueError) as cm:
+            j_child_class = jpy.get_type("org.jpy.fixtures.GetTypeFailureChild")
 
 
 if __name__ == '__main__':