diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c82a040a..dacf7df6 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 @@ -135,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 @@ -157,13 +156,97 @@ 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-wheel-t: + runs-on: ${{ matrix.info.machine }} + strategy: + fail-fast: false + matrix: + info: + - { 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-14', python: '3.13t', java: '11', arch: 'arm64', cmd: '.github/env/macOS/bdist-wheel.sh' } + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-java@v4 + id: setup-java + with: + distribution: 'temurin' + 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 }} + .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 }} + uv venv --python ${{ matrix.info.python }} + source .venv/bin/activate + uv pip install pip + echo PATH=$PATH >> $GITHUB_PATH + ${{ 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-wheel-linux-arm64-t: + runs-on: ${{ matrix.info.machine }} + strategy: + fail-fast: false + matrix: + info: + - { machine: 'ubuntu-20.04', python: '3.13t', java: '11', 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-${{ 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 && + 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-wheel-linux-arm64', 'bdist-wheel-t', 'bdist-wheel-linux-arm64-t'] steps: - uses: actions/checkout@v4 - uses: actions/download-artifact@v4 diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index dcdf1e5e..92f3eae7 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -29,3 +29,51 @@ 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: + matrix: + python: ['3.13t'] + 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 ${{ matrix.python }} + uv venv --python ${{ matrix.python }} + source .venv/bin/activate + uv pip install pip + echo $JAVA_HOME + echo PATH=$PATH >> $GITHUB_PATH + + - run: pip install "setuptools < 72" + + - 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 diff --git a/jpyutil.py b/jpyutil.py index 1f7ede20..35557af5 100644 --- a/jpyutil.py +++ b/jpyutil.py @@ -303,9 +303,13 @@ 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/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")) + 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): @@ -326,18 +330,29 @@ def _find_python_dll_file(fail=False): # Prepare list of possible library file names + # account for Python debug builds + + debug_build = sysconfig.get_config_var('Py_DEBUG') + + # 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)) diff --git a/setup.py b/setup.py index 7a5520df..b2a26356 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' diff --git a/src/main/c/jni/org_jpy_PyLib.c b/src/main/c/jni/org_jpy_PyLib.c index 75d6d39b..3100470f 100644 --- a/src/main/c/jni/org_jpy_PyLib.c +++ b/src/main/c/jni/org_jpy_PyLib.c @@ -499,12 +499,20 @@ void dumpDict(const char* dictName, PyObject* dict) size = PyDict_Size(dict); printf(">> dumpDict: %s.size = %ld\n", dictName, size); +#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); +#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++; } +#if PY_VERSION_HEX >= 0x030D0000 // >=3.13 + Py_END_CRITICAL_SECTION(); +#endif } /** @@ -521,7 +529,7 @@ PyObject *getMainGlobals() { } pyGlobals = PyModule_GetDict(pyMainModule); // borrowed ref - JPy_INCREF(pyGlobals); + JPy_XINCREF(pyGlobals); return pyGlobals; } @@ -557,7 +565,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 // < 3.13 globals = PyEval_GetGlobals(); // borrowed ref JPy_XINCREF(globals); #else @@ -588,7 +596,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 // < 3.13 locals = PyEval_GetLocals(); // borrowed ref JPy_XINCREF(locals); #else @@ -1124,7 +1132,7 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_incRef if (Py_IsInitialized()) { JPy_BEGIN_GIL_STATE - refCount = pyObject->ob_refcnt; + refCount = Py_REFCNT(pyObject); 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 +1158,7 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_decRef if (Py_IsInitialized()) { JPy_BEGIN_GIL_STATE - refCount = pyObject->ob_refcnt; + refCount = Py_REFCNT(pyObject); 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 +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]; - refCount = pyObject->ob_refcnt; + refCount = Py_REFCNT(pyObject); 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 e4469e95..6548efc4 100644 --- a/src/main/c/jpy_jmethod.c +++ b/src/main/c/jpy_jmethod.c @@ -799,6 +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 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) { @@ -950,6 +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 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 84880a27..310ebde6 100644 --- a/src/main/c/jpy_jobj.c +++ b/src/main/c/jpy_jobj.c @@ -72,12 +72,25 @@ 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 +#if PY_VERSION_HEX < 0x030D0000 // < 3.13 + // borrowed ref callable = PyDict_GetItemString(JPy_Type_Translations, type->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_Translations, type->javaName, &callable) != 1) { + callable = NULL; + } +#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 { @@ -85,6 +98,7 @@ PyObject* JObj_FromType(JNIEnv* jenv, JPy_JType* type, jobject objectRef) } } } + JPy_XDECREF(callable); return (PyObject *)obj; } @@ -103,6 +117,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 7798009d..3a877251 100644 --- a/src/main/c/jpy_jtype.c +++ b/src/main/c/jpy_jtype.c @@ -27,6 +27,59 @@ #include "jpy_conv.h" #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* rlock) { + PyThreadState* current_thread = PyThreadState_Get(); + + if (rlock->owner == current_thread) { + rlock->recursion_level++; + return; + } + + PyMutex_Lock(&(rlock->lock)); + + rlock->owner = current_thread; + rlock->recursion_level = 1; +} + +static void release_lock(ReentrantLock* rlock) { + if (rlock->owner != PyThreadState_Get()) { + PyErr_SetString(PyExc_RuntimeError, "Lock not owned by current thread"); + return; + } + + rlock->recursion_level--; + if (rlock->recursion_level == 0) { + rlock->owner = NULL; + PyMutex_Unlock(&(rlock->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 +105,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 +206,8 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve) return NULL; } + ACQUIRE_GET_TYPE_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) { @@ -160,6 +217,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 +242,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 +254,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 +266,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 +292,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 +302,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 +1031,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 +1046,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 +1055,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; } @@ -1015,12 +1087,23 @@ jboolean JType_AcceptMethod(JPy_JType* declaringClass, JPy_JMethod* method) PyObject* callableResult; //printf("JType_AcceptMethod: javaName='%s'\n", overloadedMethod->declaringClass->javaName); - +#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; + } +#endif + 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"); @@ -1028,6 +1111,7 @@ jboolean JType_AcceptMethod(JPy_JType* declaringClass, JPy_JMethod* method) } } } + JPy_XDECREF(callable); return JNI_TRUE; } @@ -1427,6 +1511,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); @@ -1454,6 +1539,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) { diff --git a/src/main/c/jpy_module.c b/src/main/c/jpy_module.c index 3c4afe8a..c4e9976d 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) { @@ -625,6 +628,8 @@ 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; } else { @@ -646,23 +651,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); } 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 1196cf14..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) { @@ -153,45 +153,54 @@ 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-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(); + + 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.threadSafeCleanup(); + + + // 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) { + String msg; + if ((msg = e.getMessage()) != null && !msg.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 new file mode 100644 index 00000000..b5b6b49a --- /dev/null +++ b/src/test/java/org/jpy/fixtures/MultiThreadedEvalTestFixture.java @@ -0,0 +1,40 @@ +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) { + 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, mode, globals, locals); + })); + } + for (Thread thread : threads) { + thread.start(); + } + for (Thread thread : threads) { + try { + thread.join(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + +} 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..1e738b0f --- /dev/null +++ b/src/test/python/jpy_mt_eval_exec_test.py @@ -0,0 +1,120 @@ +import math +import unittest + +import jpyutil + +jpyutil.init_jvm(jvm_maxmem='512M', jvm_classpath=['target/classes', 'target/test-classes']) +import jpy + +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 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 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 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 test_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 test_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()