diff --git a/src/main/c/jpy_jobj.c b/src/main/c/jpy_jobj.c index d8247dc1..84880a27 100644 --- a/src/main/c/jpy_jobj.c +++ b/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? diff --git a/src/main/c/jpy_jtype.c b/src/main/c/jpy_jtype.c index 6a8dad2e..7798009d 100644 --- a/src/main/c/jpy_jtype.c +++ b/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) diff --git a/src/main/c/jpy_module.c b/src/main/c/jpy_module.c index f29cd040..3c4afe8a 100644 --- a/src/main/c/jpy_module.c +++ b/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; diff --git a/src/test/java/org/jpy/fixtures/CyclicReferenceChild1.java b/src/test/java/org/jpy/fixtures/CyclicReferenceChild1.java new file mode 100644 index 00000000..21fd6efb --- /dev/null +++ b/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; + } +} diff --git a/src/test/java/org/jpy/fixtures/CyclicReferenceChild2.java b/src/test/java/org/jpy/fixtures/CyclicReferenceChild2.java new file mode 100644 index 00000000..922e910d --- /dev/null +++ b/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(); + } +} diff --git a/src/test/java/org/jpy/fixtures/CyclicReferenceGrandParent.java b/src/test/java/org/jpy/fixtures/CyclicReferenceGrandParent.java new file mode 100644 index 00000000..92c2fe62 --- /dev/null +++ b/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; + } +} diff --git a/src/test/java/org/jpy/fixtures/CyclicReferenceParent.java b/src/test/java/org/jpy/fixtures/CyclicReferenceParent.java new file mode 100644 index 00000000..1b544ccb --- /dev/null +++ b/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; + } + +} diff --git a/src/test/java/org/jpy/fixtures/GetTypeFailureChild.java b/src/test/java/org/jpy/fixtures/GetTypeFailureChild.java new file mode 100644 index 00000000..b065b10f --- /dev/null +++ b/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); + } +} diff --git a/src/test/java/org/jpy/fixtures/GetTypeFailureParent.java b/src/test/java/org/jpy/fixtures/GetTypeFailureParent.java new file mode 100644 index 00000000..0e86a760 --- /dev/null +++ b/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!"); + } +} diff --git a/src/test/python/jpy_gettype_test.py b/src/test/python/jpy_gettype_test.py index f9b58b1c..67e9504a 100644 --- a/src/test/python/jpy_gettype_test.py +++ b/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__':