From f335468121ee3eeb9d230040a83bd4b13b4a770f Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Mon, 23 Sep 2024 10:35:55 -0600 Subject: [PATCH 01/24] Rebase to master --- setup.py | 3 +- src/main/c/jpy_jmethod.c | 4 + src/main/c/jpy_jobj.c | 2 +- src/main/c/jpy_jtype.c | 66 ++++++++++ src/main/c/jpy_module.c | 3 + .../MultiThreadedEvalTestFixture.java | 54 ++++++++ src/test/python/jpy_eval_exec_test.py | 1 + src/test/python/jpy_mt_eval_exec_test.py | 121 ++++++++++++++++++ 8 files changed, 252 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java create mode 100644 src/test/python/jpy_mt_eval_exec_test.py diff --git a/setup.py b/setup.py index 7a5520df..31a5529b 100644 --- a/setup.py +++ b/setup.py @@ -97,6 +97,7 @@ os.path.join(src_test_py_dir, 'jpy_java_embeddable_test.py'), os.path.join(src_test_py_dir, 'jpy_obj_test.py'), os.path.join(src_test_py_dir, 'jpy_eval_exec_test.py'), + os.path.join(src_test_py_dir, 'jpy_mt_eval_exec_test.py'), ] # e.g. jdk_home_dir = '/home/marta/jdk1.7.0_15' @@ -279,7 +280,7 @@ def test_python_with_java_classes(self): def test_java(self): assert test_maven() - suite.addTest(test_python_with_java_runtime) + # suite.addTest(test_python_with_java_runtime) suite.addTest(test_python_with_java_classes) # comment out because the asynchronous nature of the PyObject GC in Java makes stopPython/startPython flakey. # suite.addTest(test_java) diff --git a/src/main/c/jpy_jmethod.c b/src/main/c/jpy_jmethod.c index e4469e95..ab49bd31 100644 --- a/src/main/c/jpy_jmethod.c +++ b/src/main/c/jpy_jmethod.c @@ -799,6 +799,8 @@ JPy_JMethod* JOverloadedMethod_FindMethod0(JNIEnv* jenv, JPy_JOverloadedMethod* overloadedMethod->declaringClass->javaName, JPy_AS_UTF8(overloadedMethod->name), overloadCount, argCount); for (i = 0; i < overloadCount; i++) { + // borrowed reference but no need to replace it with PyList_GetItemRef(), because the list won't be + // changed concurrently currMethod = (JPy_JMethod*) PyList_GetItem(overloadedMethod->methodList, i); if (currMethod->isVarArgs && matchValueMax > 0 && !bestMethod->isVarArgs) { @@ -950,6 +952,8 @@ int JOverloadedMethod_AddMethod(JPy_JOverloadedMethod* overloadedMethod, JPy_JMe // we need to insert this before the first varargs method Py_ssize_t size = PyList_Size(overloadedMethod->methodList); for (ii = 0; ii < size; ii++) { + // borrowed reference but no need to replace it with PyList_GetItemRef(), because the list won't be + // changed concurrently PyObject *check = PyList_GetItem(overloadedMethod->methodList, ii); if (((JPy_JMethod *) check)->isVarArgs) { // this is the first varargs method, so we should go before it diff --git a/src/main/c/jpy_jobj.c b/src/main/c/jpy_jobj.c index 84880a27..06687d5b 100644 --- a/src/main/c/jpy_jobj.c +++ b/src/main/c/jpy_jobj.c @@ -72,7 +72,7 @@ PyObject* JObj_FromType(JNIEnv* jenv, JPy_JType* type, jobject objectRef) } -// we check the type translations dictionary for a callable for this java type name, + // we check the type translations dictionary for a callable for this java type name, // and apply the returned callable to the wrapped object callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName); if (callable != NULL) { diff --git a/src/main/c/jpy_jtype.c b/src/main/c/jpy_jtype.c index 7798009d..f1877bca 100644 --- a/src/main/c/jpy_jtype.c +++ b/src/main/c/jpy_jtype.c @@ -27,6 +27,54 @@ #include "jpy_conv.h" #include "jpy_compat.h" +#ifdef Py_GIL_DISABLED +typedef struct { + PyMutex lock; + PyThreadState* owner; + int recursion_level; +} ReentrantLock; + +static void acquire_lock(ReentrantLock* self) { + PyThreadState* current_thread = PyThreadState_Get(); + + if (self->owner == current_thread) { + self->recursion_level++; + return; + } + + PyMutex_Lock(&(self->lock)); + + self->owner = current_thread; + self->recursion_level = 1; +} + +static void release_lock(ReentrantLock* self) { + if (self->owner != PyThreadState_Get()) { + PyErr_SetString(PyExc_RuntimeError, "Lock not owned by current thread"); + return; + } + + self->recursion_level--; + if (self->recursion_level == 0) { + self->owner = NULL; + PyMutex_Unlock(&(self->lock)); + } +} + +static ReentrantLock get_type_rlock = {{0}, NULL, 0}; +static ReentrantLock resolve_type_rlock = {{0}, NULL, 0}; + +#define ACQUIRE_GET_TYPE_LOCK() acquire_lock(&get_type_rlock) +#define RELEASE_GET_TYPE_LOCK() release_lock(&get_type_rlock) +#define ACQUIRE_RESOLVE_TYPE_LOCK() acquire_lock(&resolve_type_rlock) +#define RELEASE_RESOLVE_TYPE_LOCK() release_lock(&resolve_type_rlock) + +#else +#define ACQUIRE_GET_TYPE_LOCK() +#define RELEASE_GET_TYPE_LOCK() +#define ACQUIRE_RESOLVE_TYPE_LOCK() +#define RELEASE_RESOLVE_TYPE_LOCK() +#endif JPy_JType* JType_New(JNIEnv* jenv, jclass classRef, jboolean resolve); int JType_ResolveType(JNIEnv* jenv, JPy_JType* type); @@ -52,6 +100,8 @@ static int JType_MatchVarArgPyArgAsFPType(const JPy_ParamDescriptor *paramDescri static int JType_MatchVarArgPyArgIntType(const JPy_ParamDescriptor *paramDescriptor, PyObject *pyArg, int idx, struct JPy_JType *expectedComponentType); + + JPy_JType* JType_GetTypeForObject(JNIEnv* jenv, jobject objectRef, jboolean resolve) { JPy_JType* type; @@ -151,6 +201,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve) return NULL; } + ACQUIRE_GET_TYPE_LOCK(); typeValue = PyDict_GetItem(JPy_Types, typeKey); if (typeValue == NULL) { @@ -160,6 +211,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve) type = JType_New(jenv, classRef, resolve); if (type == NULL) { JPy_DECREF(typeKey); + RELEASE_GET_TYPE_LOCK(); return NULL; } @@ -184,6 +236,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve) PyDict_DelItem(JPy_Types, typeKey); JPy_DECREF(typeKey); JPy_DECREF(type); + RELEASE_GET_TYPE_LOCK(); return NULL; } @@ -195,6 +248,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve) PyDict_DelItem(JPy_Types, typeKey); JPy_DECREF(typeKey); JPy_DECREF(type); + RELEASE_GET_TYPE_LOCK(); return NULL; } @@ -206,6 +260,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve) PyDict_DelItem(JPy_Types, typeKey); JPy_DECREF(typeKey); JPy_DECREF(type); + RELEASE_GET_TYPE_LOCK(); return NULL; } @@ -231,6 +286,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve) "jpy internal error: attributes in 'jpy.%s' must be of type '%s', but found a '%s'", JPy_MODULE_ATTR_NAME_TYPES, JType_Type.tp_name, Py_TYPE(typeValue)->tp_name); JPy_DECREF(typeKey); + RELEASE_GET_TYPE_LOCK(); return NULL; } @@ -240,6 +296,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve) } 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); + RELEASE_GET_TYPE_LOCK(); if (!type->isResolved && resolve) { if (JType_ResolveType(jenv, type) < 0) { @@ -968,7 +1025,10 @@ int JType_ResolveType(JNIEnv* jenv, JPy_JType* type) { PyTypeObject* typeObj; + ACQUIRE_RESOLVE_TYPE_LOCK(); + if (type->isResolved || type->isResolving) { + RELEASE_RESOLVE_TYPE_LOCK(); return 0; } @@ -980,6 +1040,7 @@ int JType_ResolveType(JNIEnv* jenv, JPy_JType* type) if (!baseType->isResolved) { if (JType_ResolveType(jenv, baseType) < 0) { type->isResolving = JNI_FALSE; + RELEASE_RESOLVE_TYPE_LOCK(); return -1; } } @@ -988,24 +1049,29 @@ int JType_ResolveType(JNIEnv* jenv, JPy_JType* type) //printf("JType_ResolveType 1\n"); if (JType_ProcessClassConstructors(jenv, type) < 0) { type->isResolving = JNI_FALSE; + RELEASE_RESOLVE_TYPE_LOCK(); return -1; } //printf("JType_ResolveType 2\n"); if (JType_ProcessClassMethods(jenv, type) < 0) { type->isResolving = JNI_FALSE; + RELEASE_RESOLVE_TYPE_LOCK(); return -1; } //printf("JType_ResolveType 3\n"); if (JType_ProcessClassFields(jenv, type) < 0) { type->isResolving = JNI_FALSE; + RELEASE_RESOLVE_TYPE_LOCK(); return -1; } //printf("JType_ResolveType 4\n"); type->isResolving = JNI_FALSE; type->isResolved = JNI_TRUE; + + RELEASE_RESOLVE_TYPE_LOCK(); return 0; } diff --git a/src/main/c/jpy_module.c b/src/main/c/jpy_module.c index 3c4afe8a..6ffd90d1 100644 --- a/src/main/c/jpy_module.c +++ b/src/main/c/jpy_module.c @@ -323,6 +323,9 @@ PyMODINIT_FUNC JPY_MODULE_INIT_FUNC(void) if (JPy_Module == NULL) { JPY_RETURN(NULL); } +#ifdef Py_GIL_DISABLED + PyUnstable_Module_SetGIL(JPy_Module, Py_MOD_GIL_NOT_USED); +#endif #elif defined(JPY_COMPAT_27) JPy_Module = Py_InitModule3(JPY_MODULE_NAME, JPy_Functions, JPY_MODULE_DOC); if (JPy_Module == NULL) { diff --git a/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java b/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java new file mode 100644 index 00000000..f7ca69f3 --- /dev/null +++ b/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java @@ -0,0 +1,54 @@ +package org.jpy.fixtures; + +import org.jpy.PyInputMode; +import org.jpy.PyLib; +import org.jpy.PyObject; + +import java.util.List; + +public class MultiThreadedEvalTestFixture { + + public static void expression(String expression, int numThreads) { + PyObject globals = PyLib.getCurrentGlobals(); + PyObject locals = PyLib.getCurrentLocals(); + + List threads = new java.util.ArrayList<>(); + for (int i = 0; i < numThreads; i++) { + threads.add(new Thread(() -> { + PyObject.executeCode(expression, PyInputMode.EXPRESSION, globals, locals); + })); + } + for (Thread thread : threads) { + thread.start(); + } + for (Thread thread : threads) { + try { + thread.join(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + } + + public static void script(String expression, int numThreads) { + List threads = new java.util.ArrayList<>(); + PyObject globals = PyLib.getCurrentGlobals(); + PyObject locals = PyLib.getCurrentLocals(); + for (int i = 0; i < numThreads; i++) { + threads.add(new Thread(() -> { + PyObject.executeCode(expression, PyInputMode.SCRIPT, globals, locals); + })); + } + for (Thread thread : threads) { + thread.start(); + } + for (Thread thread : threads) { + try { + thread.join(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + } + +} diff --git a/src/test/python/jpy_eval_exec_test.py b/src/test/python/jpy_eval_exec_test.py index e7bc8798..46f4530b 100644 --- a/src/test/python/jpy_eval_exec_test.py +++ b/src/test/python/jpy_eval_exec_test.py @@ -5,6 +5,7 @@ jpyutil.init_jvm(jvm_maxmem='512M', jvm_classpath=['target/classes', 'target/test-classes']) import jpy + class TestEvalExec(unittest.TestCase): def setUp(self): self.fixture = jpy.get_type("org.jpy.fixtures.EvalTestFixture") diff --git a/src/test/python/jpy_mt_eval_exec_test.py b/src/test/python/jpy_mt_eval_exec_test.py new file mode 100644 index 00000000..dba7acc0 --- /dev/null +++ b/src/test/python/jpy_mt_eval_exec_test.py @@ -0,0 +1,121 @@ +import math +import unittest + +import jpyutil + +jpyutil.init_jvm(jvm_maxmem='512M', jvm_classpath=['target/classes', 'target/test-classes']) +import jpy +# jpy.diag.flags = jpy.diag.F_TYPE + +NUM_THREADS = 20 + + +# A CPU-bound task: computing a large number of prime numbers +def is_prime(n: int) -> bool: + if n <= 1: + return False + for i in range(2, int(math.sqrt(n)) + 1): + if n % i == 0: + return False + return True + + +def count_primes(start: int, end: int) -> int: + count = 0 + for i in range(start, end): + if is_prime(i): + count += 1 + return count + + +def use_circular_java_classes(): + 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) + result = j_child1.parentMethod() + assert result == 88 + assert 888 == j_child1.grandParentMethod() + j_child1.refChild2(j_child2) + assert 8 == j_child1.get_x() + assert 10 == j_child1.y + assert 100 == j_child1.z + + +class MultiThreadedTestEvalExec(unittest.TestCase): + def setUp(self): + self.fixture = jpy.get_type("org.jpy.fixtures.MultiThreadedEvalTestFixture") + self.assertIsNotNone(self.fixture) + + def atest_inc_baz(self): + baz = 15 + self.fixture.script("baz = baz + 1; self.assertGreater(baz, 15)", NUM_THREADS) + # note: this *is* correct wrt python semantics w/ exec(code, globals(), locals()) + # https://bugs.python.org/issue4831 (Note: it's *not* a bug, is working as intended) + self.assertEqual(baz, 15) + + def atest_exec_import(self): + import sys + self.assertTrue("json" not in sys.modules) + self.fixture.script("import json", NUM_THREADS) + self.assertTrue("json" in sys.modules) + + def atest_exec_function_call(self): + self.fixture.expression("use_circular_java_classes()", NUM_THREADS) + + def test_count_primes(self): + self.fixture.expression("count_primes(1, 10000)", NUM_THREADS) + + def atest_java_threading_jpy_get_type(self): + + py_script = """ +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) +result = j_child1.parentMethod() +assert result == 88 +assert 888 == j_child1.grandParentMethod() +j_child1.refChild2(j_child2) +assert 8 == j_child1.get_x() +assert 10 == j_child1.y +assert 100 == j_child1.z + """ + self.fixture.script(py_script, NUM_THREADS) + + def atest_py_threading_jpy_get_type(self): + import threading + + test_self = self + + class MyThread(threading.Thread): + def __init__(self): + threading.Thread.__init__(self) + + def run(self): + barrier.wait() + 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) + test_self.assertEqual(88, j_child1.parentMethod()) + test_self.assertEqual(888, j_child1.grandParentMethod()) + test_self.assertIsNone(j_child1.refChild2(j_child2)) + test_self.assertEqual(8, j_child1.get_x()) + test_self.assertEqual(10, j_child1.y) + test_self.assertEqual(100, j_child1.z) + + barrier = threading.Barrier(NUM_THREADS) + threads = [] + for i in range(NUM_THREADS): + t = MyThread() + t.start() + threads.append(t) + + for t in threads: + t.join() + + +if __name__ == '__main__': + print('\nRunning ' + __file__) + unittest.main() From a1fb4569751ed1461b5ffb74ecb1e665b57a3e08 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Mon, 23 Sep 2024 14:14:28 -0600 Subject: [PATCH 02/24] More thread-safety change/refactoring --- src/main/c/jni/org_jpy_PyLib.c | 24 +++++++++++++++++++++--- src/main/c/jpy_jmethod.c | 6 ++---- src/main/c/jpy_jobj.c | 14 +++++++++++++- src/main/c/jpy_jtype.c | 15 ++++++++++++++- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/main/c/jni/org_jpy_PyLib.c b/src/main/c/jni/org_jpy_PyLib.c index 75d6d39b..51841786 100644 --- a/src/main/c/jni/org_jpy_PyLib.c +++ b/src/main/c/jni/org_jpy_PyLib.c @@ -499,12 +499,18 @@ void dumpDict(const char* dictName, PyObject* dict) size = PyDict_Size(dict); printf(">> dumpDict: %s.size = %ld\n", dictName, size); +#ifdef Py_GIL_DISABLED + Py_BEGIN_CRITICAL_SECTION(dict); +#endif while (PyDict_Next(dict, &pos, &key, &value)) { const char* name; name = JPy_AS_UTF8(key); printf(">> dumpDict: %s[%ld].name = '%s'\n", dictName, i, name); i++; } +#ifdef Py_GIL_DISABLED + Py_END_CRITICAL_SECTION(); +#endif } /** @@ -521,7 +527,7 @@ PyObject *getMainGlobals() { } pyGlobals = PyModule_GetDict(pyMainModule); // borrowed ref - JPy_INCREF(pyGlobals); + JPy_XINCREF(pyGlobals); return pyGlobals; } @@ -557,7 +563,7 @@ JNIEXPORT jobject JNICALL Java_org_jpy_PyLib_getCurrentGlobals JPy_BEGIN_GIL_STATE -#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION <= 12 +#if PY_VERSION_HEX < 0x030D0000 globals = PyEval_GetGlobals(); // borrowed ref JPy_XINCREF(globals); #else @@ -588,7 +594,7 @@ JNIEXPORT jobject JNICALL Java_org_jpy_PyLib_getCurrentLocals JPy_BEGIN_GIL_STATE -#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION <= 12 +#if PY_VERSION_HEX < 0x030D0000 locals = PyEval_GetLocals(); // borrowed ref JPy_XINCREF(locals); #else @@ -1124,7 +1130,11 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_incRef if (Py_IsInitialized()) { JPy_BEGIN_GIL_STATE +#if PY_VERSION_HEX < 0x030D0000 refCount = pyObject->ob_refcnt; +#else + refCount = Py_REFCNT(pyObject); +#endif JPy_DIAG_PRINT(JPy_DIAG_F_MEM, "Java_org_jpy_PyLib_incRef: pyObject=%p, refCount=%d, type='%s'\n", pyObject, refCount, Py_TYPE(pyObject)->tp_name); JPy_INCREF(pyObject); @@ -1150,7 +1160,11 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_decRef if (Py_IsInitialized()) { JPy_BEGIN_GIL_STATE +#if PY_VERSION_HEX < 0x030D0000 refCount = pyObject->ob_refcnt; +#else + refCount = Py_REFCNT(pyObject); +#endif if (refCount <= 0) { JPy_DIAG_PRINT(JPy_DIAG_F_ALL, "Java_org_jpy_PyLib_decRef: error: refCount <= 0: pyObject=%p, refCount=%d\n", pyObject, refCount); } else { @@ -1183,7 +1197,11 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_decRefs buf = (*jenv)->GetLongArrayElements(jenv, objIds, &isCopy); for (i = 0; i < len; i++) { pyObject = (PyObject*) buf[i]; +#if PY_VERSION_HEX < 0x030D0000 refCount = pyObject->ob_refcnt; +#else + refCount = Py_REFCNT(pyObject); +#endif if (refCount <= 0) { JPy_DIAG_PRINT(JPy_DIAG_F_ALL, "Java_org_jpy_PyLib_decRefs: error: refCount <= 0: pyObject=%p, refCount=%d\n", pyObject, refCount); } else { diff --git a/src/main/c/jpy_jmethod.c b/src/main/c/jpy_jmethod.c index ab49bd31..6548efc4 100644 --- a/src/main/c/jpy_jmethod.c +++ b/src/main/c/jpy_jmethod.c @@ -799,8 +799,7 @@ JPy_JMethod* JOverloadedMethod_FindMethod0(JNIEnv* jenv, JPy_JOverloadedMethod* overloadedMethod->declaringClass->javaName, JPy_AS_UTF8(overloadedMethod->name), overloadCount, argCount); for (i = 0; i < overloadCount; i++) { - // borrowed reference but no need to replace it with PyList_GetItemRef(), because the list won't be - // changed concurrently + // borrowed ref, no need to replace with PyList_GetItemRef() because the list won't be changed concurrently currMethod = (JPy_JMethod*) PyList_GetItem(overloadedMethod->methodList, i); if (currMethod->isVarArgs && matchValueMax > 0 && !bestMethod->isVarArgs) { @@ -952,8 +951,7 @@ int JOverloadedMethod_AddMethod(JPy_JOverloadedMethod* overloadedMethod, JPy_JMe // we need to insert this before the first varargs method Py_ssize_t size = PyList_Size(overloadedMethod->methodList); for (ii = 0; ii < size; ii++) { - // borrowed reference but no need to replace it with PyList_GetItemRef(), because the list won't be - // changed concurrently + // borrowed ref, no need to replace with PyList_GetItemRef() because the list won't be changed concurrently PyObject *check = PyList_GetItem(overloadedMethod->methodList, ii); if (((JPy_JMethod *) check)->isVarArgs) { // this is the first varargs method, so we should go before it diff --git a/src/main/c/jpy_jobj.c b/src/main/c/jpy_jobj.c index 06687d5b..7ee6877c 100644 --- a/src/main/c/jpy_jobj.c +++ b/src/main/c/jpy_jobj.c @@ -74,10 +74,20 @@ PyObject* JObj_FromType(JNIEnv* jenv, JPy_JType* type, jobject objectRef) // we check the type translations dictionary for a callable for this java type name, // and apply the returned callable to the wrapped object - callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName); +#ifdef Py_GIL_DISABLED + // if return is 1, callable is new reference + if (PyDict_GetItemStringRef(JPy_Type_Translations, type->javaName, &callable) != 1) { + callable = NULL; + } +#else + callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName); // borrowed reference + JPy_XINCREF(callable); +#endif + if (callable != NULL) { if (PyCallable_Check(callable)) { callableResult = PyObject_CallFunction(callable, "OO", type, obj); + JPy_XDECREF(callable); if (callableResult == NULL) { Py_RETURN_NONE; } else { @@ -85,6 +95,7 @@ PyObject* JObj_FromType(JNIEnv* jenv, JPy_JType* type, jobject objectRef) } } } + JPy_XDECREF(callable); return (PyObject *)obj; } @@ -103,6 +114,7 @@ int JObj_init_internal(JNIEnv* jenv, JPy_JObj* self, PyObject* args, PyObject* k type = ((PyObject*) self)->ob_type; + // borrowed ref, no need to replace with PyDict_GetItemStringRef because tp_dict won't be changed concurrently constructor = PyDict_GetItemString(type->tp_dict, JPy_JTYPE_ATTR_NAME_JINIT); if (constructor == NULL) { PyErr_SetString(PyExc_RuntimeError, "no constructor found (missing JType attribute '" JPy_JTYPE_ATTR_NAME_JINIT "')"); diff --git a/src/main/c/jpy_jtype.c b/src/main/c/jpy_jtype.c index f1877bca..29e4eb98 100644 --- a/src/main/c/jpy_jtype.c +++ b/src/main/c/jpy_jtype.c @@ -202,6 +202,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve) } ACQUIRE_GET_TYPE_LOCK(); + // borrowed ref, no need to replace with PyDict_GetItemRef because it protected by the lock typeValue = PyDict_GetItem(JPy_Types, typeKey); if (typeValue == NULL) { @@ -1081,12 +1082,21 @@ jboolean JType_AcceptMethod(JPy_JType* declaringClass, JPy_JMethod* method) PyObject* callableResult; //printf("JType_AcceptMethod: javaName='%s'\n", overloadedMethod->declaringClass->javaName); +#ifdef Py_GIL_DISABLED + // if return is 1, callable is new reference + if (PyDict_GetItemStringRef(JPy_Type_Callbacks, declaringClass->javaName, &callable) != 1) { + callable = NULL; + } +#else + callable = PyDict_GetItemString(JPy_Type_Callbacks, declaringClass->javaName); // borrowed reference + JPy_XINCREF(callable); +#endif - callable = PyDict_GetItemString(JPy_Type_Callbacks, declaringClass->javaName); if (callable != NULL) { if (PyCallable_Check(callable)) { callableResult = PyObject_CallFunction(callable, "OO", declaringClass, method); if (callableResult == Py_None || callableResult == Py_False) { + JPy_XDECREF(callable); return JNI_FALSE; } else if (callableResult == NULL) { JPy_DIAG_PRINT(JPy_DIAG_F_TYPE, "JType_AcceptMethod: warning: failed to invoke callback on method addition\n"); @@ -1094,6 +1104,7 @@ jboolean JType_AcceptMethod(JPy_JType* declaringClass, JPy_JMethod* method) } } } + JPy_XDECREF(callable); return JNI_TRUE; } @@ -1493,6 +1504,7 @@ int JType_AddMethod(JPy_JType* type, JPy_JMethod* method) return -1; } + // borrowed ref, no need to replace with PyDict_GetItemRef because typeDict won't be changed concurrently methodValue = PyDict_GetItem(typeDict, method->name); if (methodValue == NULL) { overloadedMethod = JOverloadedMethod_New(type, method->name, method); @@ -1520,6 +1532,7 @@ PyObject* JType_GetOverloadedMethod(JNIEnv* jenv, JPy_JType* type, PyObject* met return NULL; } + // borrowed ref, no need to replace with PyDict_GetItemRef because typeDict won't be changed concurrently methodValue = PyDict_GetItem(typeDict, methodName); if (methodValue == NULL) { if (useSuperClass) { From e93774a09fe9b20dce0a4e079064476fd0f60081 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Thu, 26 Sep 2024 18:18:20 -0600 Subject: [PATCH 03/24] Fix a race between PO cleanup thread and Python --- src/main/java/org/jpy/PyObject.java | 2 +- src/main/java/org/jpy/PyObjectReferences.java | 70 ++++++++++--------- .../MultiThreadedEvalTestFixture.java | 1 - 3 files changed, 39 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/jpy/PyObject.java b/src/main/java/org/jpy/PyObject.java index e0715ec7..746039d8 100644 --- a/src/main/java/org/jpy/PyObject.java +++ b/src/main/java/org/jpy/PyObject.java @@ -71,7 +71,7 @@ public static int cleanup() { PyObject(long pointer, boolean fromJNI) { state = new PyObjectState(pointer); if (fromJNI) { - if (CLEANUP_ON_INIT && PyLib.hasGil()) { + if (CLEANUP_ON_INIT) { REFERENCES.cleanupOnlyUseFromGIL(); // only performs *one* cleanup } if (CLEANUP_ON_THREAD) { diff --git a/src/main/java/org/jpy/PyObjectReferences.java b/src/main/java/org/jpy/PyObjectReferences.java index 1196cf14..6d266042 100644 --- a/src/main/java/org/jpy/PyObjectReferences.java +++ b/src/main/java/org/jpy/PyObjectReferences.java @@ -153,45 +153,51 @@ def cleanup(references): sleep_time = 0.1 if size == 1024 else 1.0 time.sleep(sleep_time) */ - - final PyObjectCleanup proxy = asProxy(); - - while (!Thread.currentThread().isInterrupted()) { - // This blocks on the GIL, acquires the GIL, and then releases the GIL. - // For linux, acquiring the GIL involves a pthread_mutex_lock, which does not provide - // any fairness guarantees. As such, we need to be mindful of other python users/code, - // and ensure we don't overly acquire the GIL causing starvation issues, especially when - // there is no cleanup work to do. - final int size = proxy.cleanupOnlyUseFromGIL(); - - - // Although, it *does* make sense to potentially take the GIL in a tight loop when there - // is a lot of real cleanup work to do. Sleeping for any amount of time may be - // detrimental to the cleanup of resources. There is a balance that we want to try to - // achieve between producers of PyObjects, and the cleanup of PyObjects (us). - - // It would be much nicer if ReferenceQueue exposed a method that blocked until the - // queue was non-empty and *doesn't* remove any items. We can potentially implement this - // by using reflection to access the internal lock of the ReferenceQueue in the future. - - if (size == buffer.length) { - if (CLEANUP_THREAD_ACTIVE_SLEEP_MILLIS == 0) { - Thread.yield(); + try { + final PyObjectCleanup proxy = asProxy(); + + while (!Thread.currentThread().isInterrupted()) { + // This blocks on the GIL, acquires the GIL, and then releases the GIL. + // For linux, acquiring the GIL involves a pthread_mutex_lock, which does not provide + // any fairness guarantees. As such, we need to be mindful of other python users/code, + // and ensure we don't overly acquire the GIL causing starvation issues, especially when + // there is no cleanup work to do. + final int size = proxy.cleanupOnlyUseFromGIL(); + + + // Although, it *does* make sense to potentially take the GIL in a tight loop when there + // is a lot of real cleanup work to do. Sleeping for any amount of time may be + // detrimental to the cleanup of resources. There is a balance that we want to try to + // achieve between producers of PyObjects, and the cleanup of PyObjects (us). + + // It would be much nicer if ReferenceQueue exposed a method that blocked until the + // queue was non-empty and *doesn't* remove any items. We can potentially implement this + // by using reflection to access the internal lock of the ReferenceQueue in the future. + + if (size == buffer.length) { + if (CLEANUP_THREAD_ACTIVE_SLEEP_MILLIS == 0) { + Thread.yield(); + } else { + try { + Thread.sleep(CLEANUP_THREAD_ACTIVE_SLEEP_MILLIS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + } } else { try { - Thread.sleep(CLEANUP_THREAD_ACTIVE_SLEEP_MILLIS); + Thread.sleep(CLEANUP_THREAD_PASSIVE_SLEEP_MILLIS); } catch (InterruptedException e) { Thread.currentThread().interrupt(); return; } } - } else { - try { - Thread.sleep(CLEANUP_THREAD_PASSIVE_SLEEP_MILLIS); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - return; - } + } + } + catch (RuntimeException e) { + if (!e.getMessage().contains("PyLib not initialized")) { + throw e; } } } diff --git a/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java b/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java index f7ca69f3..1a7ed56b 100644 --- a/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java +++ b/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java @@ -50,5 +50,4 @@ public static void script(String expression, int numThreads) { } } } - } From e65f8ffa48c4703c9e8c501641cbce4ec8c39ec0 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Mon, 21 Oct 2024 10:18:50 -0600 Subject: [PATCH 04/24] Add free-threaded build in GH action --- .github/workflows/build.yml | 1 - .github/workflows/build_ft_wheel.yml | 105 +++++++++++++++++++++++++++ .github/workflows/check.yml | 27 +++++++ 3 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/build_ft_wheel.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c82a040a..78d1e303 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -79,7 +79,6 @@ jobs: distribution: 'temurin' java-version: '8' - - run: pip install setuptools - run: ${{ matrix.info.cmd }} - uses: actions/upload-artifact@v4 diff --git a/.github/workflows/build_ft_wheel.yml b/.github/workflows/build_ft_wheel.yml new file mode 100644 index 00000000..9bee2ba7 --- /dev/null +++ b/.github/workflows/build_ft_wheel.yml @@ -0,0 +1,105 @@ +name: Build Free-Threaded JPY distributions + +on: + pull_request: + branches: [ 'master', 'release/v*' ] + push: + branches: [ 'master', 'release/v*' ] + +jobs: + bdist-wheels: + runs-on: ${{ matrix.info.machine }} + strategy: + fail-fast: false + matrix: + info: + - { machine: 'ubuntu-20.04', python: '3.13t', arch: 'amd64', cmd: '.github/env/Linux/bdist-wheel.sh' } + - { machine: 'macos-13', python: '3.13t', arch: 'amd64', cmd: '.github/env/macOS/bdist-wheel.sh' } + - { machine: 'macos-latest', python: '3.13t', arch: 'arm64', cmd: '.github/env/macOS/bdist-wheel.sh' } + + steps: + - uses: actions/checkout@v4 + + - uses: astral-sh/setup-uv@v3 + - run: | + uv python install ${{ matrix.info.python }} + uv venv --python ${{ matrix.info.python }} + source .venv/bin/activate + uv pip install pip + echo $JAVA_HOME + echo PATH=$PATH >> $GITHUB_ENV + + - run: ${{ matrix.info.cmd }} + + - uses: actions/upload-artifact@v4 + with: + name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }} + path: dist/*.whl + retention-days: 1 + + bdist-wheels-windows: + runs-on: ${{ matrix.info.machine }} + strategy: + fail-fast: false + matrix: + info: + - { machine: 'windows-2022', python: '3.13t', arch: 'amd64', cmd: '.\.github\env\Windows\bdist-wheel.ps1' } + + steps: + - uses: actions/checkout@v4 + + - uses: astral-sh/setup-uv@v3 + - run: | + uv python install ${{ matrix.info.python }} + uv venv --python ${{ matrix.info.python }} + .venv\Scripts\Activate.ps1 + uv pip install pip + echo PATH=%PATH% >> $GITHUB_ENV + ${{ matrix.info.cmd }} + + - uses: actions/upload-artifact@v4 + with: + name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }} + path: dist/*.whl + retention-days: 1 + + bdist-wheels-linux-aarch64: + runs-on: ${{ matrix.info.machine }} + strategy: + fail-fast: false + matrix: + info: + - { machine: 'ubuntu-20.04', python: '3.13t', arch: 'aarch64', cmd: '.github/env/Linux/bdist-wheel.sh' } + + steps: + - uses: actions/checkout@v4 + + - name: Set up QEMU + uses: docker/setup-qemu-action@v3 + + - name: Build wheels + uses: pypa/cibuildwheel@v2.21.3 + env: + CIBW_FREE_THREADED_SUPPORT: true + CIBW_ARCHS_LINUX: "aarch64" + CIBW_BUILD: "cp313t-*" + CIBW_SKIP: "cp313t-musllinux_aarch64" + CIBW_BEFORE_ALL_LINUX: > + yum install -y java-11-openjdk-devel && + yum install -y wget && + wget https://www.apache.org/dist/maven/maven-3/3.8.8/binaries/apache-maven-3.8.8-bin.tar.gz -P /tmp && + tar xf /tmp/apache-maven-3.8.8-bin.tar.gz -C /opt && + ln -s /opt/apache-maven-3.8.8/bin/mvn /usr/bin/mvn + CIBW_ENVIRONMENT: JAVA_HOME=/etc/alternatives/jre_11_openjdk + CIBW_REPAIR_WHEEL_COMMAND_LINUX: 'auditwheel repair --exclude libjvm.so -w {dest_dir} {wheel}' + + with: + package-dir: . + output-dir: dist + + - uses: actions/upload-artifact@v4 + with: + name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }} + path: dist/*.whl + retention-days: 1 + diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index dcdf1e5e..3e07e41a 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -29,3 +29,30 @@ jobs: - name: Run Test run: python setup.py test + + test-free-threaded: + runs-on: ubuntu-22.04 + strategy: + matrix: + java: ['8', '11', '17', '21', '23'] + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-java@v4 + with: + distribution: 'temurin' + java-version: ${{ matrix.java }} + + - uses: astral-sh/setup-uv@v3 + - run: | + uv python install 3.13t + uv venv --python 3.13t + source .venv/bin/activate + uv pip install pip + echo PATH=$PATH >> $GITHUB_ENV + + - run: pip install "setuptools < 72" + + - name: Run Free-threaded Test + run: python setup.py test + From 6a0f8fe195b2f0027b2d86df24d0f8025dfb3587 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Thu, 24 Oct 2024 13:35:40 -0600 Subject: [PATCH 05/24] Fix dll names for FT/debug builds --- .github/workflows/check.yml | 6 ++++-- jpyutil.py | 30 ++++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 3e07e41a..0ca19d6b 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -34,6 +34,7 @@ jobs: runs-on: ubuntu-22.04 strategy: matrix: + python: ['3.13t'] java: ['8', '11', '17', '21', '23'] steps: - uses: actions/checkout@v4 @@ -45,10 +46,11 @@ jobs: - uses: astral-sh/setup-uv@v3 - run: | - uv python install 3.13t - uv venv --python 3.13t + uv python install ${{ matrix.python }} + uv venv --python ${{ matrix.python }} source .venv/bin/activate uv pip install pip + echo $JAVA_HOME echo PATH=$PATH >> $GITHUB_ENV - run: pip install "setuptools < 72" diff --git a/jpyutil.py b/jpyutil.py index 1f7ede20..ecf008f7 100644 --- a/jpyutil.py +++ b/jpyutil.py @@ -43,6 +43,7 @@ # This way importing jpyutil does not interfere with logging in other modules logger = logging.getLogger('jpyutil') # Get log level from environment variable JPY_LOG_LEVEL. Default to INFO +os.environ['JPY_LOG_LEVEL'] = 'DEBUG' log_level = os.getenv('JPY_LOG_LEVEL', 'INFO') try: logger.setLevel(getattr(logging, log_level)) @@ -303,18 +304,17 @@ def _find_python_dll_file(fail=False): logger.debug("Searching for Python shared library file") # Prepare list of search directories - search_dirs = [sys.prefix] + installed_base = sysconfig.get_config_var('installed_base') + if installed_base: + search_dirs.append(os.path.join(installed_base, "lib")) + extra_search_dirs = [sysconfig.get_config_var(name) for name in PYTHON_LIB_DIR_CONFIG_VAR_NAMES] for extra_dir in extra_search_dirs: if extra_dir and extra_dir not in search_dirs and os.path.exists(extra_dir): search_dirs.append(extra_dir) - if platform.system() == 'Windows': - extra_search_dirs = _get_existing_subdirs(search_dirs, "DLLs") - search_dirs = extra_search_dirs + search_dirs - multi_arch_sub_dir = sysconfig.get_config_var('multiarchsubdir') if multi_arch_sub_dir: while multi_arch_sub_dir.startswith('/'): @@ -326,18 +326,32 @@ def _find_python_dll_file(fail=False): # Prepare list of possible library file names + # account for Python debug builds + try: + sys.gettotalrefcount() + debug_build = True + except AttributeError: + debug_build = False + + # account for Python 3.13+ with GIL disabled + dll_suffix = '' + if sys.version_info >= (3, 13): + if not sys._is_gil_enabled(): + dll_suffix = 't' + dll_suffix += 'd' if debug_build else '' + vmaj = str(sys.version_info.major) vmin = str(sys.version_info.minor) if platform.system() == 'Windows': - versions = (vmaj + vmin, vmaj, '') + versions = (vmaj + vmin, vmaj, vmaj + vmin + dll_suffix, '') file_names = ['python' + v + '.dll' for v in versions] elif platform.system() == 'Darwin': - versions = (vmaj + "." + vmin, vmaj, '') + versions = (vmaj + "." + vmin, vmaj, vmaj + "." + vmin + dll_suffix, '') file_names = ['libpython' + v + '.dylib' for v in versions] + \ ['libpython' + v + '.so' for v in versions] else: - versions = (vmaj + "." + vmin, vmaj, '') + versions = (vmaj + "." + vmin, vmaj, vmaj + "." + vmin + dll_suffix, '') file_names = ['libpython' + v + '.so' for v in versions] logger.debug("Potential Python shared library file names: %s" % repr(file_names)) From 2cb33144e7ef83ed41973690a6dd37b237d0a605 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Thu, 24 Oct 2024 14:02:11 -0600 Subject: [PATCH 06/24] Cleanup and more comments --- jpyutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jpyutil.py b/jpyutil.py index ecf008f7..f73167fa 100644 --- a/jpyutil.py +++ b/jpyutil.py @@ -43,7 +43,6 @@ # This way importing jpyutil does not interfere with logging in other modules logger = logging.getLogger('jpyutil') # Get log level from environment variable JPY_LOG_LEVEL. Default to INFO -os.environ['JPY_LOG_LEVEL'] = 'DEBUG' log_level = os.getenv('JPY_LOG_LEVEL', 'INFO') try: logger.setLevel(getattr(logging, log_level)) @@ -306,6 +305,7 @@ def _find_python_dll_file(fail=False): # Prepare list of search directories search_dirs = [sys.prefix] + # installed_base/lib needs to be added to the search path for Python 3.13t files installed_base = sysconfig.get_config_var('installed_base') if installed_base: search_dirs.append(os.path.join(installed_base, "lib")) From fe97888884d1df8967f617f680b67c46501af1e9 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Thu, 24 Oct 2024 14:19:17 -0600 Subject: [PATCH 07/24] Rollback temp change and add clarifying comments --- setup.py | 2 +- src/main/java/org/jpy/PyObjectReferences.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 31a5529b..b2a26356 100644 --- a/setup.py +++ b/setup.py @@ -280,7 +280,7 @@ def test_python_with_java_classes(self): def test_java(self): assert test_maven() - # suite.addTest(test_python_with_java_runtime) + suite.addTest(test_python_with_java_runtime) suite.addTest(test_python_with_java_classes) # comment out because the asynchronous nature of the PyObject GC in Java makes stopPython/startPython flakey. # suite.addTest(test_java) diff --git a/src/main/java/org/jpy/PyObjectReferences.java b/src/main/java/org/jpy/PyObjectReferences.java index 6d266042..68cc7fe5 100644 --- a/src/main/java/org/jpy/PyObjectReferences.java +++ b/src/main/java/org/jpy/PyObjectReferences.java @@ -153,6 +153,8 @@ def cleanup(references): sleep_time = 0.1 if size == 1024 else 1.0 time.sleep(sleep_time) */ + // try-catch block to handle PyLib not initialized exception when a race condition occurs in the free-threaded + // mode that the cleanup thread starts running after Python is already finalized. try { final PyObjectCleanup proxy = asProxy(); From ab38dcba0dd7c087a70138548211e31005d9d7cb Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Thu, 24 Oct 2024 14:51:21 -0600 Subject: [PATCH 08/24] Complete release action for FT builds --- .github/workflows/build_ft_wheel.yml | 32 +++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build_ft_wheel.yml b/.github/workflows/build_ft_wheel.yml index 9bee2ba7..9e0d8f23 100644 --- a/.github/workflows/build_ft_wheel.yml +++ b/.github/workflows/build_ft_wheel.yml @@ -63,7 +63,7 @@ jobs: path: dist/*.whl retention-days: 1 - bdist-wheels-linux-aarch64: + bdist-wheels-linux-arm64: runs-on: ${{ matrix.info.machine }} strategy: fail-fast: false @@ -103,3 +103,33 @@ jobs: path: dist/*.whl retention-days: 1 + collect-artifacts: + runs-on: ubuntu-22.04 + needs: ['bdist-wheels', 'bdist-wheels-windows', 'bdist-wheels-linux-arm64'] + steps: + - uses: actions/checkout@v4 + - uses: actions/download-artifact@v4 + with: + path: download-artifacts + - name: collect-artifacts.sh + run: .github/scripts/collect-artifacts.sh + - uses: actions/upload-artifact@v4 + with: + name: jpy-ft + path: collect-artifacts + + release-artifacts: + if: ${{ startsWith(github.ref, 'refs/heads/release/v') }} + runs-on: ubuntu-22.04 + needs: ['collect-artifacts'] + steps: + - uses: actions/download-artifact@v4 + with: + name: jpy-ft + path: dist + - name: Publish package to PyPI + uses: pypa/gh-action-pypi-publish@release/v1 + with: + user: __token__ + password: ${{ secrets.PYPI_API_TOKEN }} + From 16ba89a32b569568036f9f904aadb418de7a7c9a Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Thu, 24 Oct 2024 15:07:23 -0600 Subject: [PATCH 09/24] Merge ft builds into build.yml --- .github/workflows/build.yml | 98 ++++++++++++++++++- .github/workflows/build_ft_wheel.yml | 135 --------------------------- 2 files changed, 97 insertions(+), 136 deletions(-) delete mode 100644 .github/workflows/build_ft_wheel.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 78d1e303..e5b3127b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -160,9 +160,105 @@ jobs: path: /tmp/dist/*.whl retention-days: 1 + bdist-wheels-t: + runs-on: ${{ matrix.info.machine }} + strategy: + fail-fast: false + matrix: + info: + - { machine: 'ubuntu-20.04', python: '3.13t', arch: 'amd64', cmd: '.github/env/Linux/bdist-wheel.sh' } + - { machine: 'macos-13', python: '3.13t', arch: 'amd64', cmd: '.github/env/macOS/bdist-wheel.sh' } + - { machine: 'macos-latest', python: '3.13t', arch: 'arm64', cmd: '.github/env/macOS/bdist-wheel.sh' } + + steps: + - uses: actions/checkout@v4 + + - uses: astral-sh/setup-uv@v3 + - run: | + uv python install ${{ matrix.info.python }} + uv venv --python ${{ matrix.info.python }} + source .venv/bin/activate + uv pip install pip + echo $JAVA_HOME + echo PATH=$PATH >> $GITHUB_ENV + + - run: ${{ matrix.info.cmd }} + + - uses: actions/upload-artifact@v4 + with: + name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }} + path: dist/*.whl + retention-days: 1 + + bdist-wheels-windows-t: + runs-on: ${{ matrix.info.machine }} + strategy: + fail-fast: false + matrix: + info: + - { machine: 'windows-2022', python: '3.13t', arch: 'amd64', cmd: '.\.github\env\Windows\bdist-wheel.ps1' } + + steps: + - uses: actions/checkout@v4 + + - uses: astral-sh/setup-uv@v3 + - run: | + uv python install ${{ matrix.info.python }} + uv venv --python ${{ matrix.info.python }} + .venv\Scripts\Activate.ps1 + uv pip install pip + echo PATH=%PATH% >> $GITHUB_ENV + ${{ matrix.info.cmd }} + + - uses: actions/upload-artifact@v4 + with: + name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }} + path: dist/*.whl + retention-days: 1 + + bdist-wheels-linux-arm64-t: + runs-on: ${{ matrix.info.machine }} + strategy: + fail-fast: false + matrix: + info: + - { machine: 'ubuntu-20.04', python: '3.13t', arch: 'aarch64', cmd: '.github/env/Linux/bdist-wheel.sh' } + + steps: + - uses: actions/checkout@v4 + + - name: Set up QEMU + uses: docker/setup-qemu-action@v3 + + - name: Build wheels + uses: pypa/cibuildwheel@v2.21.3 + env: + CIBW_FREE_THREADED_SUPPORT: true + CIBW_ARCHS_LINUX: "aarch64" + CIBW_BUILD: "cp313t-*" + CIBW_SKIP: "cp313t-musllinux_aarch64" + CIBW_BEFORE_ALL_LINUX: > + yum install -y java-11-openjdk-devel && + yum install -y wget && + wget https://www.apache.org/dist/maven/maven-3/3.8.8/binaries/apache-maven-3.8.8-bin.tar.gz -P /tmp && + tar xf /tmp/apache-maven-3.8.8-bin.tar.gz -C /opt && + ln -s /opt/apache-maven-3.8.8/bin/mvn /usr/bin/mvn + CIBW_ENVIRONMENT: JAVA_HOME=/etc/alternatives/jre_11_openjdk + CIBW_REPAIR_WHEEL_COMMAND_LINUX: 'auditwheel repair --exclude libjvm.so -w {dest_dir} {wheel}' + + with: + package-dir: . + output-dir: dist + + - uses: actions/upload-artifact@v4 + with: + name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }} + path: dist/*.whl + retention-days: 1 + collect-artifacts: runs-on: ubuntu-22.04 - needs: ['sdist', 'bdist-wheel', 'bdist-wheel-universal2-hack', 'bdist-wheels-linux-arm64'] + needs: ['sdist', 'bdist-wheel', 'bdist-wheel-universal2-hack', 'bdist-wheels-linux-arm64', 'bdist-wheels-t', 'bdist-wheels-windows-t', 'bdist-wheels-linux-arm64-t'] steps: - uses: actions/checkout@v4 - uses: actions/download-artifact@v4 diff --git a/.github/workflows/build_ft_wheel.yml b/.github/workflows/build_ft_wheel.yml deleted file mode 100644 index 9e0d8f23..00000000 --- a/.github/workflows/build_ft_wheel.yml +++ /dev/null @@ -1,135 +0,0 @@ -name: Build Free-Threaded JPY distributions - -on: - pull_request: - branches: [ 'master', 'release/v*' ] - push: - branches: [ 'master', 'release/v*' ] - -jobs: - bdist-wheels: - runs-on: ${{ matrix.info.machine }} - strategy: - fail-fast: false - matrix: - info: - - { machine: 'ubuntu-20.04', python: '3.13t', arch: 'amd64', cmd: '.github/env/Linux/bdist-wheel.sh' } - - { machine: 'macos-13', python: '3.13t', arch: 'amd64', cmd: '.github/env/macOS/bdist-wheel.sh' } - - { machine: 'macos-latest', python: '3.13t', arch: 'arm64', cmd: '.github/env/macOS/bdist-wheel.sh' } - - steps: - - uses: actions/checkout@v4 - - - uses: astral-sh/setup-uv@v3 - - run: | - uv python install ${{ matrix.info.python }} - uv venv --python ${{ matrix.info.python }} - source .venv/bin/activate - uv pip install pip - echo $JAVA_HOME - echo PATH=$PATH >> $GITHUB_ENV - - - run: ${{ matrix.info.cmd }} - - - uses: actions/upload-artifact@v4 - with: - name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }} - path: dist/*.whl - retention-days: 1 - - bdist-wheels-windows: - runs-on: ${{ matrix.info.machine }} - strategy: - fail-fast: false - matrix: - info: - - { machine: 'windows-2022', python: '3.13t', arch: 'amd64', cmd: '.\.github\env\Windows\bdist-wheel.ps1' } - - steps: - - uses: actions/checkout@v4 - - - uses: astral-sh/setup-uv@v3 - - run: | - uv python install ${{ matrix.info.python }} - uv venv --python ${{ matrix.info.python }} - .venv\Scripts\Activate.ps1 - uv pip install pip - echo PATH=%PATH% >> $GITHUB_ENV - ${{ matrix.info.cmd }} - - - uses: actions/upload-artifact@v4 - with: - name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }} - path: dist/*.whl - retention-days: 1 - - bdist-wheels-linux-arm64: - runs-on: ${{ matrix.info.machine }} - strategy: - fail-fast: false - matrix: - info: - - { machine: 'ubuntu-20.04', python: '3.13t', arch: 'aarch64', cmd: '.github/env/Linux/bdist-wheel.sh' } - - steps: - - uses: actions/checkout@v4 - - - name: Set up QEMU - uses: docker/setup-qemu-action@v3 - - - name: Build wheels - uses: pypa/cibuildwheel@v2.21.3 - env: - CIBW_FREE_THREADED_SUPPORT: true - CIBW_ARCHS_LINUX: "aarch64" - CIBW_BUILD: "cp313t-*" - CIBW_SKIP: "cp313t-musllinux_aarch64" - CIBW_BEFORE_ALL_LINUX: > - yum install -y java-11-openjdk-devel && - yum install -y wget && - wget https://www.apache.org/dist/maven/maven-3/3.8.8/binaries/apache-maven-3.8.8-bin.tar.gz -P /tmp && - tar xf /tmp/apache-maven-3.8.8-bin.tar.gz -C /opt && - ln -s /opt/apache-maven-3.8.8/bin/mvn /usr/bin/mvn - CIBW_ENVIRONMENT: JAVA_HOME=/etc/alternatives/jre_11_openjdk - CIBW_REPAIR_WHEEL_COMMAND_LINUX: 'auditwheel repair --exclude libjvm.so -w {dest_dir} {wheel}' - - with: - package-dir: . - output-dir: dist - - - uses: actions/upload-artifact@v4 - with: - name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }} - path: dist/*.whl - retention-days: 1 - - collect-artifacts: - runs-on: ubuntu-22.04 - needs: ['bdist-wheels', 'bdist-wheels-windows', 'bdist-wheels-linux-arm64'] - steps: - - uses: actions/checkout@v4 - - uses: actions/download-artifact@v4 - with: - path: download-artifacts - - name: collect-artifacts.sh - run: .github/scripts/collect-artifacts.sh - - uses: actions/upload-artifact@v4 - with: - name: jpy-ft - path: collect-artifacts - - release-artifacts: - if: ${{ startsWith(github.ref, 'refs/heads/release/v') }} - runs-on: ubuntu-22.04 - needs: ['collect-artifacts'] - steps: - - uses: actions/download-artifact@v4 - with: - name: jpy-ft - path: dist - - name: Publish package to PyPI - uses: pypa/gh-action-pypi-publish@release/v1 - with: - user: __token__ - password: ${{ secrets.PYPI_API_TOKEN }} - From 40ef816297065848a4c4ebb2e8d1f0e2f831f3a6 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Tue, 29 Oct 2024 13:45:00 -0600 Subject: [PATCH 10/24] Use GITHUB_PATH --- .github/workflows/build.yml | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e5b3127b..3552b458 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -173,14 +173,19 @@ jobs: steps: - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 + id: setup-java + with: + distribution: 'temurin' + java-version: '8' + - uses: astral-sh/setup-uv@v3 - run: | uv python install ${{ matrix.info.python }} uv venv --python ${{ matrix.info.python }} source .venv/bin/activate uv pip install pip - echo $JAVA_HOME - echo PATH=$PATH >> $GITHUB_ENV + echo PATH=$PATH >> $GITHUB_PATH - run: ${{ matrix.info.cmd }} @@ -201,14 +206,21 @@ jobs: steps: - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 + id: setup-java + with: + distribution: 'temurin' + java-version: '8' + - uses: astral-sh/setup-uv@v3 - run: | uv python install ${{ matrix.info.python }} uv venv --python ${{ matrix.info.python }} .venv\Scripts\Activate.ps1 uv pip install pip - echo PATH=%PATH% >> $GITHUB_ENV - ${{ matrix.info.cmd }} + echo PATH=%PATH% >> $GITHUB_PATH + + - run: ${{ matrix.info.cmd }} - uses: actions/upload-artifact@v4 with: From a5718c170723cfa4d834270dca90e0556b6eb850 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Tue, 29 Oct 2024 14:18:33 -0600 Subject: [PATCH 11/24] Squash windows-t build, add Java ver in matrix --- .github/workflows/build.yml | 54 +++++++++---------------------------- 1 file changed, 13 insertions(+), 41 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3552b458..a4693584 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -166,9 +166,10 @@ jobs: fail-fast: false matrix: info: - - { machine: 'ubuntu-20.04', python: '3.13t', arch: 'amd64', cmd: '.github/env/Linux/bdist-wheel.sh' } - - { machine: 'macos-13', python: '3.13t', arch: 'amd64', cmd: '.github/env/macOS/bdist-wheel.sh' } - - { machine: 'macos-latest', python: '3.13t', arch: 'arm64', cmd: '.github/env/macOS/bdist-wheel.sh' } + - { machine: 'ubuntu-20.04', python: '3.13t', java: '8', arch: 'amd64', cmd: '.github/env/Linux/bdist-wheel.sh' } + - { machine: 'windows-2022', python: '3.13t', java: '8', arch: 'amd64', cmd: '.\.github\env\Windows\bdist-wheel.ps1' } + - { machine: 'macos-13', python: '3.13t', java: '8', arch: 'amd64', cmd: '.github/env/macOS/bdist-wheel.sh' } + - { machine: 'macos-latest', python: '3.13t', java: '11', arch: 'arm64', cmd: '.github/env/macOS/bdist-wheel.sh' } steps: - uses: actions/checkout@v4 @@ -177,13 +178,17 @@ jobs: id: setup-java with: distribution: 'temurin' - java-version: '8' + java-version: ${{ matrix.info.java }} - uses: astral-sh/setup-uv@v3 - run: | uv python install ${{ matrix.info.python }} uv venv --python ${{ matrix.info.python }} - source .venv/bin/activate + if [ ${{ matrix.info.machine }} == 'windows-2022' ]; then + .venv\Scripts\Activate.ps1 + else + source .venv/bin/activate + fi uv pip install pip echo PATH=$PATH >> $GITHUB_PATH @@ -195,46 +200,13 @@ jobs: path: dist/*.whl retention-days: 1 - bdist-wheels-windows-t: - runs-on: ${{ matrix.info.machine }} - strategy: - fail-fast: false - matrix: - info: - - { machine: 'windows-2022', python: '3.13t', arch: 'amd64', cmd: '.\.github\env\Windows\bdist-wheel.ps1' } - - steps: - - uses: actions/checkout@v4 - - - uses: actions/setup-java@v4 - id: setup-java - with: - distribution: 'temurin' - java-version: '8' - - - uses: astral-sh/setup-uv@v3 - - run: | - uv python install ${{ matrix.info.python }} - uv venv --python ${{ matrix.info.python }} - .venv\Scripts\Activate.ps1 - uv pip install pip - echo PATH=%PATH% >> $GITHUB_PATH - - - run: ${{ matrix.info.cmd }} - - - uses: actions/upload-artifact@v4 - with: - name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }} - path: dist/*.whl - retention-days: 1 - bdist-wheels-linux-arm64-t: runs-on: ${{ matrix.info.machine }} strategy: fail-fast: false matrix: info: - - { machine: 'ubuntu-20.04', python: '3.13t', arch: 'aarch64', cmd: '.github/env/Linux/bdist-wheel.sh' } + - { machine: 'ubuntu-20.04', python: '3.13t', java: '11', arch: 'aarch64', cmd: '.github/env/Linux/bdist-wheel.sh' } steps: - uses: actions/checkout@v4 @@ -250,7 +222,7 @@ jobs: CIBW_BUILD: "cp313t-*" CIBW_SKIP: "cp313t-musllinux_aarch64" CIBW_BEFORE_ALL_LINUX: > - yum install -y java-11-openjdk-devel && + yum install -y java-${{ matrix.info.java }}-openjdk-devel && yum install -y wget && wget https://www.apache.org/dist/maven/maven-3/3.8.8/binaries/apache-maven-3.8.8-bin.tar.gz -P /tmp && tar xf /tmp/apache-maven-3.8.8-bin.tar.gz -C /opt && @@ -270,7 +242,7 @@ jobs: collect-artifacts: runs-on: ubuntu-22.04 - needs: ['sdist', 'bdist-wheel', 'bdist-wheel-universal2-hack', 'bdist-wheels-linux-arm64', 'bdist-wheels-t', 'bdist-wheels-windows-t', 'bdist-wheels-linux-arm64-t'] + needs: ['sdist', 'bdist-wheel', 'bdist-wheel-universal2-hack', 'bdist-wheels-linux-arm64', 'bdist-wheels-t', 'bdist-wheels-linux-arm64-t'] steps: - uses: actions/checkout@v4 - uses: actions/download-artifact@v4 From 9aa82508bb117e62c181286ffcb6b4d8b51f9a49 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Tue, 29 Oct 2024 15:02:10 -0600 Subject: [PATCH 12/24] Fix for invalid syntax for powershell --- .github/workflows/build.yml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a4693584..dcf7e013 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -181,14 +181,17 @@ jobs: java-version: ${{ matrix.info.java }} - uses: astral-sh/setup-uv@v3 - - run: | + run: | uv python install ${{ matrix.info.python }} uv venv --python ${{ matrix.info.python }} - if [ ${{ matrix.info.machine }} == 'windows-2022' ]; then - .venv\Scripts\Activate.ps1 - else - source .venv/bin/activate - fi + - if : ${{ startsWith(matrix.info.machine, 'windows')}} + run: | + .venv\Scripts\Activate.ps1 + uv pip install pip + echo PATH=$PATH >> $GITHUB_PATH + - if : ${{ ! startsWith(matrix.info.machine, 'windows')}} + run: | + source .venv/bin/activate uv pip install pip echo PATH=$PATH >> $GITHUB_PATH From 752a0a729ff9d748a5b393d8535c2aa33b7a5a0a Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Tue, 29 Oct 2024 15:36:06 -0600 Subject: [PATCH 13/24] Add upload action for JVM core/log --- .github/workflows/check.yml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 0ca19d6b..6f8f0b42 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -30,6 +30,16 @@ jobs: - name: Run Test run: python setup.py test + - name: Upload JVM Error Logs + uses: actions/upload-artifact@v4 + if: failure() + with: + name: check-ci-jvm-err + path: | + **/*_pid*.log + **/core.* + if-no-files-found: ignore + test-free-threaded: runs-on: ubuntu-22.04 strategy: @@ -58,3 +68,12 @@ jobs: - name: Run Free-threaded Test run: python setup.py test + - name: Upload JVM Error Logs + uses: actions/upload-artifact@v4 + if: failure() + with: + name: check-ci-jvm-err + path: | + **/*_pid*.log + **/core.* + if-no-files-found: ignore From e1f841bf8485f50a1f26b622ccfd2160c207632a Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Tue, 29 Oct 2024 15:41:04 -0600 Subject: [PATCH 14/24] debug build.yml --- .github/workflows/build.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index dcf7e013..b9003194 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -181,16 +181,17 @@ jobs: java-version: ${{ matrix.info.java }} - uses: astral-sh/setup-uv@v3 + - if : ${{ startsWith(matrix.info.machine, 'windows')}} run: | uv python install ${{ matrix.info.python }} uv venv --python ${{ matrix.info.python }} - - if : ${{ startsWith(matrix.info.machine, 'windows')}} - run: | .venv\Scripts\Activate.ps1 uv pip install pip echo PATH=$PATH >> $GITHUB_PATH - if : ${{ ! startsWith(matrix.info.machine, 'windows')}} run: | + uv python install ${{ matrix.info.python }} + uv venv --python ${{ matrix.info.python }} source .venv/bin/activate uv pip install pip echo PATH=$PATH >> $GITHUB_PATH From 01659211fbd6e123d751ca0d01a71ff5eabfd00a Mon Sep 17 00:00:00 2001 From: Jianfeng Mao <4297243+jmao-denver@users.noreply.github.com> Date: Tue, 29 Oct 2024 15:58:11 -0600 Subject: [PATCH 15/24] Apply suggestions from code review Co-authored-by: Chip Kent <5250374+chipkent@users.noreply.github.com> --- src/main/c/jpy_jobj.c | 1 + src/main/c/jpy_jtype.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/c/jpy_jobj.c b/src/main/c/jpy_jobj.c index 7ee6877c..a23c0570 100644 --- a/src/main/c/jpy_jobj.c +++ b/src/main/c/jpy_jobj.c @@ -80,6 +80,7 @@ PyObject* JObj_FromType(JNIEnv* jenv, JPy_JType* type, jobject objectRef) callable = NULL; } #else + // borrowed ref, no need to replace with PyDict_GetItemStringRef() because the dict won't be changed concurrently callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName); // borrowed reference JPy_XINCREF(callable); #endif diff --git a/src/main/c/jpy_jtype.c b/src/main/c/jpy_jtype.c index 29e4eb98..e32b3061 100644 --- a/src/main/c/jpy_jtype.c +++ b/src/main/c/jpy_jtype.c @@ -202,7 +202,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve) } ACQUIRE_GET_TYPE_LOCK(); - // borrowed ref, no need to replace with PyDict_GetItemRef because it protected by the lock + // borrowed ref, no need to replace with PyDict_GetItemRef because it is protected by the lock typeValue = PyDict_GetItem(JPy_Types, typeKey); if (typeValue == NULL) { @@ -1088,7 +1088,8 @@ jboolean JType_AcceptMethod(JPy_JType* declaringClass, JPy_JMethod* method) callable = NULL; } #else - callable = PyDict_GetItemString(JPy_Type_Callbacks, declaringClass->javaName); // borrowed reference + // borrowed ref, no need to replace with PyDict_GetItemStringRef because it is protected by the lock + callable = PyDict_GetItemString(JPy_Type_Callbacks, declaringClass->javaName); JPy_XINCREF(callable); #endif From 31b9010dcbcd862e2b65b058820819aa0a23f501 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Tue, 29 Oct 2024 16:00:10 -0600 Subject: [PATCH 16/24] Restore a block of code deleted by mistake --- jpyutil.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jpyutil.py b/jpyutil.py index f73167fa..e3777657 100644 --- a/jpyutil.py +++ b/jpyutil.py @@ -315,6 +315,10 @@ def _find_python_dll_file(fail=False): if extra_dir and extra_dir not in search_dirs and os.path.exists(extra_dir): search_dirs.append(extra_dir) + if platform.system() == 'Windows': + extra_search_dirs = _get_existing_subdirs(search_dirs, "DLLs") + search_dirs = extra_search_dirs + search_dirs + multi_arch_sub_dir = sysconfig.get_config_var('multiarchsubdir') if multi_arch_sub_dir: while multi_arch_sub_dir.startswith('/'): From ccc3253c87e42e6fded627ba300beecdcc45e86a Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Tue, 29 Oct 2024 16:30:39 -0600 Subject: [PATCH 17/24] debug Windows FT build --- .github/workflows/build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b9003194..044429e6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -188,6 +188,7 @@ jobs: .venv\Scripts\Activate.ps1 uv pip install pip echo PATH=$PATH >> $GITHUB_PATH + ${{ matrix.info.cmd }} - if : ${{ ! startsWith(matrix.info.machine, 'windows')}} run: | uv python install ${{ matrix.info.python }} @@ -195,8 +196,7 @@ jobs: source .venv/bin/activate uv pip install pip echo PATH=$PATH >> $GITHUB_PATH - - - run: ${{ matrix.info.cmd }} + ${{ matrix.info.cmd }} - uses: actions/upload-artifact@v4 with: From d972d96a8ea0b9eace97c1ac1d1267504de8bcbf Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Wed, 30 Oct 2024 13:55:35 -0600 Subject: [PATCH 18/24] Add comments, use version directives --- src/main/c/jni/org_jpy_PyLib.c | 18 ++------ src/main/c/jpy_jobj.c | 14 ++++--- src/main/c/jpy_jtype.c | 42 +++++++++++-------- src/main/java/org/jpy/PyObjectReferences.java | 3 +- 4 files changed, 38 insertions(+), 39 deletions(-) diff --git a/src/main/c/jni/org_jpy_PyLib.c b/src/main/c/jni/org_jpy_PyLib.c index 51841786..af347f09 100644 --- a/src/main/c/jni/org_jpy_PyLib.c +++ b/src/main/c/jni/org_jpy_PyLib.c @@ -500,6 +500,8 @@ void dumpDict(const char* dictName, PyObject* dict) size = PyDict_Size(dict); printf(">> dumpDict: %s.size = %ld\n", dictName, size); #ifdef Py_GIL_DISABLED + // PyDict_Next is not thread-safe, so we need to protect it with a critical section + // https://docs.python.org/3/howto/free-threading-extensions.html#pydict-next Py_BEGIN_CRITICAL_SECTION(dict); #endif while (PyDict_Next(dict, &pos, &key, &value)) { @@ -563,7 +565,7 @@ JNIEXPORT jobject JNICALL Java_org_jpy_PyLib_getCurrentGlobals JPy_BEGIN_GIL_STATE -#if PY_VERSION_HEX < 0x030D0000 +#if PY_VERSION_HEX < 0x030D0000 // < 3.13 globals = PyEval_GetGlobals(); // borrowed ref JPy_XINCREF(globals); #else @@ -594,7 +596,7 @@ JNIEXPORT jobject JNICALL Java_org_jpy_PyLib_getCurrentLocals JPy_BEGIN_GIL_STATE -#if PY_VERSION_HEX < 0x030D0000 +#if PY_VERSION_HEX < 0x030D0000 // < 3.13 locals = PyEval_GetLocals(); // borrowed ref JPy_XINCREF(locals); #else @@ -1130,11 +1132,7 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_incRef if (Py_IsInitialized()) { JPy_BEGIN_GIL_STATE -#if PY_VERSION_HEX < 0x030D0000 - refCount = pyObject->ob_refcnt; -#else refCount = Py_REFCNT(pyObject); -#endif JPy_DIAG_PRINT(JPy_DIAG_F_MEM, "Java_org_jpy_PyLib_incRef: pyObject=%p, refCount=%d, type='%s'\n", pyObject, refCount, Py_TYPE(pyObject)->tp_name); JPy_INCREF(pyObject); @@ -1160,11 +1158,7 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_decRef if (Py_IsInitialized()) { JPy_BEGIN_GIL_STATE -#if PY_VERSION_HEX < 0x030D0000 - refCount = pyObject->ob_refcnt; -#else refCount = Py_REFCNT(pyObject); -#endif if (refCount <= 0) { JPy_DIAG_PRINT(JPy_DIAG_F_ALL, "Java_org_jpy_PyLib_decRef: error: refCount <= 0: pyObject=%p, refCount=%d\n", pyObject, refCount); } else { @@ -1197,11 +1191,7 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_decRefs buf = (*jenv)->GetLongArrayElements(jenv, objIds, &isCopy); for (i = 0; i < len; i++) { pyObject = (PyObject*) buf[i]; -#if PY_VERSION_HEX < 0x030D0000 - refCount = pyObject->ob_refcnt; -#else refCount = Py_REFCNT(pyObject); -#endif if (refCount <= 0) { JPy_DIAG_PRINT(JPy_DIAG_F_ALL, "Java_org_jpy_PyLib_decRefs: error: refCount <= 0: pyObject=%p, refCount=%d\n", pyObject, refCount); } else { diff --git a/src/main/c/jpy_jobj.c b/src/main/c/jpy_jobj.c index a23c0570..6d73d85d 100644 --- a/src/main/c/jpy_jobj.c +++ b/src/main/c/jpy_jobj.c @@ -74,21 +74,23 @@ PyObject* JObj_FromType(JNIEnv* jenv, JPy_JType* type, jobject objectRef) // we check the type translations dictionary for a callable for this java type name, // and apply the returned callable to the wrapped object -#ifdef Py_GIL_DISABLED - // if return is 1, callable is new reference +#if PY_VERSION_HEX < 0x030D0000 // < 3.13 + // borrowed ref + callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName); // borrowed reference + JPy_XINCREF(callable); +#else + // https://docs.python.org/3/howto/free-threading-extensions.html#borrowed-references + // PyDict_GetItemStringRef() is a thread safe version of PyDict_GetItemString() and returns a new reference if (PyDict_GetItemStringRef(JPy_Type_Translations, type->javaName, &callable) != 1) { callable = NULL; } -#else - // borrowed ref, no need to replace with PyDict_GetItemStringRef() because the dict won't be changed concurrently - callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName); // borrowed reference - JPy_XINCREF(callable); #endif if (callable != NULL) { if (PyCallable_Check(callable)) { callableResult = PyObject_CallFunction(callable, "OO", type, obj); JPy_XDECREF(callable); + JPy_XDECREF(obj); if (callableResult == NULL) { Py_RETURN_NONE; } else { diff --git a/src/main/c/jpy_jtype.c b/src/main/c/jpy_jtype.c index e32b3061..3a877251 100644 --- a/src/main/c/jpy_jtype.c +++ b/src/main/c/jpy_jtype.c @@ -28,36 +28,41 @@ #include "jpy_compat.h" #ifdef Py_GIL_DISABLED +// Reentrant lock for the recursive JType_GetType() and JType_ResolveType() in free-threaded environments +// Note that in order to avoid a fatal circular reference issues, JType_InitSuperType() no longer tries to resolve +// the super types. This means it is impossible for a thread to hold a get_type lock while trying to acquire the +// resolve_type lock but it can still hold a resolve_type lock while trying to acquire the get_type lock. This allows +// maximum concurrency but also avoids deadlocks at the same time. typedef struct { PyMutex lock; PyThreadState* owner; int recursion_level; } ReentrantLock; -static void acquire_lock(ReentrantLock* self) { +static void acquire_lock(ReentrantLock* rlock) { PyThreadState* current_thread = PyThreadState_Get(); - if (self->owner == current_thread) { - self->recursion_level++; + if (rlock->owner == current_thread) { + rlock->recursion_level++; return; } - PyMutex_Lock(&(self->lock)); + PyMutex_Lock(&(rlock->lock)); - self->owner = current_thread; - self->recursion_level = 1; + rlock->owner = current_thread; + rlock->recursion_level = 1; } -static void release_lock(ReentrantLock* self) { - if (self->owner != PyThreadState_Get()) { +static void release_lock(ReentrantLock* rlock) { + if (rlock->owner != PyThreadState_Get()) { PyErr_SetString(PyExc_RuntimeError, "Lock not owned by current thread"); return; } - self->recursion_level--; - if (self->recursion_level == 0) { - self->owner = NULL; - PyMutex_Unlock(&(self->lock)); + rlock->recursion_level--; + if (rlock->recursion_level == 0) { + rlock->owner = NULL; + PyMutex_Unlock(&(rlock->lock)); } } @@ -1082,15 +1087,16 @@ jboolean JType_AcceptMethod(JPy_JType* declaringClass, JPy_JMethod* method) PyObject* callableResult; //printf("JType_AcceptMethod: javaName='%s'\n", overloadedMethod->declaringClass->javaName); -#ifdef Py_GIL_DISABLED - // if return is 1, callable is new reference +#if PY_VERSION_HEX < 0x030D0000 // < 3.13 + // borrowed ref + callable = PyDict_GetItemString(JPy_Type_Callbacks, declaringClass->javaName); + JPy_XINCREF(callable); +#else + // https://docs.python.org/3/howto/free-threading-extensions.html#borrowed-references + // PyDict_GetItemStringRef() is a thread safe version of PyDict_GetItemString() and returns a new reference if (PyDict_GetItemStringRef(JPy_Type_Callbacks, declaringClass->javaName, &callable) != 1) { callable = NULL; } -#else - // borrowed ref, no need to replace with PyDict_GetItemStringRef because it is protected by the lock - callable = PyDict_GetItemString(JPy_Type_Callbacks, declaringClass->javaName); - JPy_XINCREF(callable); #endif if (callable != NULL) { diff --git a/src/main/java/org/jpy/PyObjectReferences.java b/src/main/java/org/jpy/PyObjectReferences.java index 68cc7fe5..9eec1903 100644 --- a/src/main/java/org/jpy/PyObjectReferences.java +++ b/src/main/java/org/jpy/PyObjectReferences.java @@ -198,7 +198,8 @@ def cleanup(references): } } catch (RuntimeException e) { - if (!e.getMessage().contains("PyLib not initialized")) { + String msg; + if ((msg = e.getMessage()) != null && !msg.contains("PyLib not initialized")) { throw e; } } From 40586384c5e73bf3a8ad6b8a8d0dbc2a17be5162 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Thu, 31 Oct 2024 13:47:43 -0600 Subject: [PATCH 19/24] Code cleanup --- .github/workflows/build.yml | 10 +++---- src/main/java/org/jpy/PyObject.java | 2 +- .../MultiThreadedEvalTestFixture.java | 29 +++++-------------- src/test/python/jpy_mt_eval_exec_test.py | 11 ++++--- 4 files changed, 19 insertions(+), 33 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 044429e6..04db875b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -134,7 +134,7 @@ jobs: path: dist/*.whl retention-days: 1 - bdist-wheels-linux-arm64: + bdist-wheel-linux-arm64: runs-on: 'ubuntu-22.04' steps: - uses: actions/checkout@v4 @@ -156,11 +156,11 @@ jobs: - uses: actions/upload-artifact@v4 with: - name: bdist-wheels-linux-arm64 + name: bdist-wheel-linux-arm64 path: /tmp/dist/*.whl retention-days: 1 - bdist-wheels-t: + bdist-wheel-t: runs-on: ${{ matrix.info.machine }} strategy: fail-fast: false @@ -204,7 +204,7 @@ jobs: path: dist/*.whl retention-days: 1 - bdist-wheels-linux-arm64-t: + bdist-wheel-linux-arm64-t: runs-on: ${{ matrix.info.machine }} strategy: fail-fast: false @@ -246,7 +246,7 @@ jobs: collect-artifacts: runs-on: ubuntu-22.04 - needs: ['sdist', 'bdist-wheel', 'bdist-wheel-universal2-hack', 'bdist-wheels-linux-arm64', 'bdist-wheels-t', 'bdist-wheels-linux-arm64-t'] + needs: ['sdist', 'bdist-wheel', 'bdist-wheel-universal2-hack', 'bdist-wheel-linux-arm64', 'bdist-wheel-t', 'bdist-wheel-linux-arm64-t'] steps: - uses: actions/checkout@v4 - uses: actions/download-artifact@v4 diff --git a/src/main/java/org/jpy/PyObject.java b/src/main/java/org/jpy/PyObject.java index 746039d8..e0715ec7 100644 --- a/src/main/java/org/jpy/PyObject.java +++ b/src/main/java/org/jpy/PyObject.java @@ -71,7 +71,7 @@ public static int cleanup() { PyObject(long pointer, boolean fromJNI) { state = new PyObjectState(pointer); if (fromJNI) { - if (CLEANUP_ON_INIT) { + if (CLEANUP_ON_INIT && PyLib.hasGil()) { REFERENCES.cleanupOnlyUseFromGIL(); // only performs *one* cleanup } if (CLEANUP_ON_THREAD) { diff --git a/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java b/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java index 1a7ed56b..b5b6b49a 100644 --- a/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java +++ b/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java @@ -9,34 +9,20 @@ public class MultiThreadedEvalTestFixture { public static void expression(String expression, int numThreads) { - PyObject globals = PyLib.getCurrentGlobals(); - PyObject locals = PyLib.getCurrentLocals(); - - List threads = new java.util.ArrayList<>(); - for (int i = 0; i < numThreads; i++) { - threads.add(new Thread(() -> { - PyObject.executeCode(expression, PyInputMode.EXPRESSION, globals, locals); - })); - } - for (Thread thread : threads) { - thread.start(); - } - for (Thread thread : threads) { - try { - thread.join(); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - } + execute(PyInputMode.EXPRESSION, expression, numThreads); } public static void script(String expression, int numThreads) { + execute(PyInputMode.SCRIPT, expression, numThreads); + } + + private static void execute(PyInputMode mode, String expression, int numThreads) { List threads = new java.util.ArrayList<>(); PyObject globals = PyLib.getCurrentGlobals(); PyObject locals = PyLib.getCurrentLocals(); for (int i = 0; i < numThreads; i++) { threads.add(new Thread(() -> { - PyObject.executeCode(expression, PyInputMode.SCRIPT, globals, locals); + PyObject.executeCode(expression, mode, globals, locals); })); } for (Thread thread : threads) { @@ -46,8 +32,9 @@ public static void script(String expression, int numThreads) { try { thread.join(); } catch (InterruptedException e) { - throw new RuntimeException(e); + Thread.currentThread().interrupt(); } } } + } diff --git a/src/test/python/jpy_mt_eval_exec_test.py b/src/test/python/jpy_mt_eval_exec_test.py index dba7acc0..1e738b0f 100644 --- a/src/test/python/jpy_mt_eval_exec_test.py +++ b/src/test/python/jpy_mt_eval_exec_test.py @@ -5,7 +5,6 @@ jpyutil.init_jvm(jvm_maxmem='512M', jvm_classpath=['target/classes', 'target/test-classes']) import jpy -# jpy.diag.flags = jpy.diag.F_TYPE NUM_THREADS = 20 @@ -47,26 +46,26 @@ def setUp(self): self.fixture = jpy.get_type("org.jpy.fixtures.MultiThreadedEvalTestFixture") self.assertIsNotNone(self.fixture) - def atest_inc_baz(self): + def test_inc_baz(self): baz = 15 self.fixture.script("baz = baz + 1; self.assertGreater(baz, 15)", NUM_THREADS) # note: this *is* correct wrt python semantics w/ exec(code, globals(), locals()) # https://bugs.python.org/issue4831 (Note: it's *not* a bug, is working as intended) self.assertEqual(baz, 15) - def atest_exec_import(self): + def test_exec_import(self): import sys self.assertTrue("json" not in sys.modules) self.fixture.script("import json", NUM_THREADS) self.assertTrue("json" in sys.modules) - def atest_exec_function_call(self): + def test_exec_function_call(self): self.fixture.expression("use_circular_java_classes()", NUM_THREADS) def test_count_primes(self): self.fixture.expression("count_primes(1, 10000)", NUM_THREADS) - def atest_java_threading_jpy_get_type(self): + def test_java_threading_jpy_get_type(self): py_script = """ j_child1_class = jpy.get_type("org.jpy.fixtures.CyclicReferenceChild1") @@ -83,7 +82,7 @@ def atest_java_threading_jpy_get_type(self): """ self.fixture.script(py_script, NUM_THREADS) - def atest_py_threading_jpy_get_type(self): + def test_py_threading_jpy_get_type(self): import threading test_self = self From 173592d4015f3a539e207ede8300e55ba8ea1149 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Fri, 1 Nov 2024 21:32:37 -0600 Subject: [PATCH 20/24] Fix a bug in convert() for java array type --- src/main/c/jpy_module.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/src/main/c/jpy_module.c b/src/main/c/jpy_module.c index 6ffd90d1..e332562a 100644 --- a/src/main/c/jpy_module.c +++ b/src/main/c/jpy_module.c @@ -628,6 +628,7 @@ PyObject* JPy_convert_internal(JNIEnv* jenv, PyObject* self, PyObject* args) if (targetTypeParsed == NULL) { return NULL; } + JPy_DECREF(targetTypeParsed); } else if (JType_Check(targetTypeArg)) { targetTypeParsed = (JPy_JType*) targetTypeArg; } else { @@ -649,23 +650,7 @@ PyObject* JPy_convert_internal(JNIEnv* jenv, PyObject* self, PyObject* args) return NULL; } - // Create a global reference for the objectRef (so it is valid after we exit this frame) - objectRef = (*jenv)->NewGlobalRef(jenv, objectRef); - if (objectRef == NULL) { - PyErr_NoMemory(); - return NULL; - } - - // Create a PyObject (JObj) to hold the result - resultObj = (JPy_JObj*) PyObject_New(JPy_JObj, JTYPE_AS_PYTYPE(targetTypeParsed)); - if (resultObj == NULL) { - (*jenv)->DeleteGlobalRef(jenv, objectRef); - return NULL; - } - // Store the reference to the converted object in the result JObj - ((JPy_JObj*) resultObj)->objectRef = objectRef; - - return (PyObject*) resultObj; + return JObj_FromType(jenv, targetTypeParsed, objectRef); } From eea87e7474c7cba2881df4c0b5f8bf4cef7aaa2f Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Mon, 4 Nov 2024 09:43:29 -0700 Subject: [PATCH 21/24] Remove duplicate comment --- src/main/c/jpy_jobj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/c/jpy_jobj.c b/src/main/c/jpy_jobj.c index 6d73d85d..310ebde6 100644 --- a/src/main/c/jpy_jobj.c +++ b/src/main/c/jpy_jobj.c @@ -76,7 +76,7 @@ PyObject* JObj_FromType(JNIEnv* jenv, JPy_JType* type, jobject objectRef) // and apply the returned callable to the wrapped object #if PY_VERSION_HEX < 0x030D0000 // < 3.13 // borrowed ref - callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName); // borrowed reference + callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName); JPy_XINCREF(callable); #else // https://docs.python.org/3/howto/free-threading-extensions.html#borrowed-references From c9d7bfacb7091168ed52eba1af912b408e55874d Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Mon, 4 Nov 2024 16:29:14 -0700 Subject: [PATCH 22/24] Add clarifying comment --- src/main/c/jpy_module.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/c/jpy_module.c b/src/main/c/jpy_module.c index e332562a..c4e9976d 100644 --- a/src/main/c/jpy_module.c +++ b/src/main/c/jpy_module.c @@ -628,6 +628,7 @@ PyObject* JPy_convert_internal(JNIEnv* jenv, PyObject* self, PyObject* args) if (targetTypeParsed == NULL) { return NULL; } + // new ref, persists in the global JPy_Types dict and will never be removed, so safe to DECREF JPy_DECREF(targetTypeParsed); } else if (JType_Check(targetTypeArg)) { targetTypeParsed = (JPy_JType*) targetTypeArg; From 1c51917ba58c87fc5e4c293faf772a284179c263 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Tue, 5 Nov 2024 23:19:35 -0500 Subject: [PATCH 23/24] Respond to review comments --- .github/workflows/build.yml | 2 +- .github/workflows/check.yml | 2 +- jpyutil.py | 7 ++----- src/main/c/jni/org_jpy_PyLib.c | 4 ++-- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 04db875b..dacf7df6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -169,7 +169,7 @@ jobs: - { machine: 'ubuntu-20.04', python: '3.13t', java: '8', arch: 'amd64', cmd: '.github/env/Linux/bdist-wheel.sh' } - { machine: 'windows-2022', python: '3.13t', java: '8', arch: 'amd64', cmd: '.\.github\env\Windows\bdist-wheel.ps1' } - { machine: 'macos-13', python: '3.13t', java: '8', arch: 'amd64', cmd: '.github/env/macOS/bdist-wheel.sh' } - - { machine: 'macos-latest', python: '3.13t', java: '11', arch: 'arm64', cmd: '.github/env/macOS/bdist-wheel.sh' } + - { machine: 'macos-14', python: '3.13t', java: '11', arch: 'arm64', cmd: '.github/env/macOS/bdist-wheel.sh' } steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 6f8f0b42..92f3eae7 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -61,7 +61,7 @@ jobs: source .venv/bin/activate uv pip install pip echo $JAVA_HOME - echo PATH=$PATH >> $GITHUB_ENV + echo PATH=$PATH >> $GITHUB_PATH - run: pip install "setuptools < 72" diff --git a/jpyutil.py b/jpyutil.py index e3777657..35557af5 100644 --- a/jpyutil.py +++ b/jpyutil.py @@ -331,11 +331,8 @@ def _find_python_dll_file(fail=False): # Prepare list of possible library file names # account for Python debug builds - try: - sys.gettotalrefcount() - debug_build = True - except AttributeError: - debug_build = False + + debug_build = sysconfig.get_config_var('Py_DEBUG') # account for Python 3.13+ with GIL disabled dll_suffix = '' diff --git a/src/main/c/jni/org_jpy_PyLib.c b/src/main/c/jni/org_jpy_PyLib.c index af347f09..3100470f 100644 --- a/src/main/c/jni/org_jpy_PyLib.c +++ b/src/main/c/jni/org_jpy_PyLib.c @@ -499,7 +499,7 @@ void dumpDict(const char* dictName, PyObject* dict) size = PyDict_Size(dict); printf(">> dumpDict: %s.size = %ld\n", dictName, size); -#ifdef Py_GIL_DISABLED +#if PY_VERSION_HEX >= 0x030D0000 // >=3.13 // PyDict_Next is not thread-safe, so we need to protect it with a critical section // https://docs.python.org/3/howto/free-threading-extensions.html#pydict-next Py_BEGIN_CRITICAL_SECTION(dict); @@ -510,7 +510,7 @@ void dumpDict(const char* dictName, PyObject* dict) printf(">> dumpDict: %s[%ld].name = '%s'\n", dictName, i, name); i++; } -#ifdef Py_GIL_DISABLED +#if PY_VERSION_HEX >= 0x030D0000 // >=3.13 Py_END_CRITICAL_SECTION(); #endif } From 99f130aec3ac16f230aa42df5d2c599ec43f5cb1 Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Wed, 6 Nov 2024 09:30:49 -0500 Subject: [PATCH 24/24] Naming change to be more applicable in FT mode --- src/main/java/org/jpy/PyObject.java | 6 +++--- src/main/java/org/jpy/PyObjectCleanup.java | 2 +- src/main/java/org/jpy/PyObjectReferences.java | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/jpy/PyObject.java b/src/main/java/org/jpy/PyObject.java index e0715ec7..44f07409 100644 --- a/src/main/java/org/jpy/PyObject.java +++ b/src/main/java/org/jpy/PyObject.java @@ -58,7 +58,7 @@ private static void startCleanupThread() { } public static int cleanup() { - return REFERENCES.asProxy().cleanupOnlyUseFromGIL(); + return REFERENCES.asProxy().threadSafeCleanup(); } private final PyObjectState state; @@ -71,8 +71,8 @@ public static int cleanup() { PyObject(long pointer, boolean fromJNI) { state = new PyObjectState(pointer); if (fromJNI) { - if (CLEANUP_ON_INIT && PyLib.hasGil()) { - REFERENCES.cleanupOnlyUseFromGIL(); // only performs *one* cleanup + if (CLEANUP_ON_INIT) { + REFERENCES.threadSafeCleanup(); // only performs *one* cleanup } if (CLEANUP_ON_THREAD) { // ensures that we've only started after python has been started, and we know there is something to cleanup diff --git a/src/main/java/org/jpy/PyObjectCleanup.java b/src/main/java/org/jpy/PyObjectCleanup.java index cda84557..7285e3a2 100644 --- a/src/main/java/org/jpy/PyObjectCleanup.java +++ b/src/main/java/org/jpy/PyObjectCleanup.java @@ -1,5 +1,5 @@ package org.jpy; interface PyObjectCleanup { - int cleanupOnlyUseFromGIL(); + int threadSafeCleanup(); } diff --git a/src/main/java/org/jpy/PyObjectReferences.java b/src/main/java/org/jpy/PyObjectReferences.java index 9eec1903..4afea8a0 100644 --- a/src/main/java/org/jpy/PyObjectReferences.java +++ b/src/main/java/org/jpy/PyObjectReferences.java @@ -76,11 +76,11 @@ private Reference asRef(PyObject pyObject) { /** * This should *only* be invoked through the proxy, or when we *know* we have the GIL. */ - public int cleanupOnlyUseFromGIL() { - return cleanupOnlyUseFromGIL(buffer); + public int threadSafeCleanup() { + return threadSafeCleanup(buffer); } - private int cleanupOnlyUseFromGIL(long[] buffer) { + private int threadSafeCleanup(long[] buffer) { return PyLib.ensureGil(() -> { int index = 0; while (index < buffer.length) { @@ -164,7 +164,7 @@ def cleanup(references): // any fairness guarantees. As such, we need to be mindful of other python users/code, // and ensure we don't overly acquire the GIL causing starvation issues, especially when // there is no cleanup work to do. - final int size = proxy.cleanupOnlyUseFromGIL(); + final int size = proxy.threadSafeCleanup(); // Although, it *does* make sense to potentially take the GIL in a tight loop when there