Skip to content

Commit

Permalink
Split the changes to keep the issue relevant ones
Browse files Browse the repository at this point in the history
  • Loading branch information
jmao-denver committed May 12, 2024
1 parent c7a4b8b commit b50f206
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/main/c/jpy_jobj.c
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ int JType_InitSlots(JPy_JType* type)

typeObj = JTYPE_AS_PYTYPE(type);

// This is hacky to make the caller JType_GetType be able to call JPy_INCREF regardless if the type was already created,
// 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
// #if defined(JPY_COMPAT_39P)
// Py_SET_REFCNT(typeObj, 1);
Expand Down
14 changes: 14 additions & 0 deletions src/main/c/jpy_jtype.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,16 +171,28 @@ 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 ...
// 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) {
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, 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;
}

Expand All @@ -190,6 +202,8 @@ 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;
}

Expand Down
30 changes: 30 additions & 0 deletions src/test/java/org/jpy/fixtures/CyclicReferenceChild1.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
17 changes: 17 additions & 0 deletions src/test/java/org/jpy/fixtures/CyclicReferenceChild2.java
Original file line number Diff line number Diff line change
@@ -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 {
}
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;
}

}
15 changes: 15 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,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 doesn't break 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__':
Expand Down

0 comments on commit b50f206

Please sign in to comment.