Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delay resolving super classes until referenced #146

Merged
merged 8 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions src/main/c/jpy_jobj.c
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
32 changes: 25 additions & 7 deletions src/main/c/jpy_jtype.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) {
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
JPy_DIAG_PRINT(JPy_DIAG_F_TYPE, "JType_GetType: error: JType_InitSuperType() failed for javaName=\"%s\"\n", type->javaName);
PyDict_DelItem(JPy_Types, typeKey);
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
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) {
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
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;
}

Expand All @@ -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);
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
JPy_DECREF(type);

//printf("T5: type->tp_init=%p\n", ((PyTypeObject*)type)->tp_init);

Expand All @@ -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);
Expand All @@ -226,7 +246,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
return NULL;
}
}

JPy_INCREF(type);
return type;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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)
Expand Down
4 changes: 0 additions & 4 deletions src/main/c/jpy_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
37 changes: 37 additions & 0 deletions src/test/java/org/jpy/fixtures/CyclicReferenceChild1.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
20 changes: 20 additions & 0 deletions src/test/java/org/jpy/fixtures/CyclicReferenceChild2.java
Original file line number Diff line number Diff line change
@@ -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();
}
}
33 changes: 33 additions & 0 deletions src/test/java/org/jpy/fixtures/CyclicReferenceGrandParent.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
32 changes: 32 additions & 0 deletions src/test/java/org/jpy/fixtures/CyclicReferenceParent.java
Original file line number Diff line number Diff line change
@@ -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;
}

}
26 changes: 26 additions & 0 deletions src/test/java/org/jpy/fixtures/GetTypeFailureChild.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
24 changes: 24 additions & 0 deletions src/test/java/org/jpy/fixtures/GetTypeFailureParent.java
Original file line number Diff line number Diff line change
@@ -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!");
}
}
28 changes: 28 additions & 0 deletions src/test/python/jpy_gettype_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,34 @@ 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")
print(str(cm.exception))
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved


if __name__ == '__main__':
Expand Down
Loading