Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: free-threaded Python (3.13.0+) support #165

Merged
merged 24 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f335468
Rebase to master
jmao-denver Sep 23, 2024
a1fb456
More thread-safety change/refactoring
jmao-denver Sep 23, 2024
e93774a
Fix a race between PO cleanup thread and Python
jmao-denver Sep 27, 2024
e65f8ff
Add free-threaded build in GH action
jmao-denver Oct 21, 2024
6a0f8fe
Fix dll names for FT/debug builds
jmao-denver Oct 24, 2024
2cb3314
Cleanup and more comments
jmao-denver Oct 24, 2024
fe97888
Rollback temp change and add clarifying comments
jmao-denver Oct 24, 2024
ab38dcb
Complete release action for FT builds
jmao-denver Oct 24, 2024
16ba89a
Merge ft builds into build.yml
jmao-denver Oct 24, 2024
40ef816
Use GITHUB_PATH
jmao-denver Oct 29, 2024
a5718c1
Squash windows-t build, add Java ver in matrix
jmao-denver Oct 29, 2024
9aa8250
Fix for invalid syntax for powershell
jmao-denver Oct 29, 2024
752a0a7
Add upload action for JVM core/log
jmao-denver Oct 29, 2024
e1f841b
debug build.yml
jmao-denver Oct 29, 2024
0165921
Apply suggestions from code review
jmao-denver Oct 29, 2024
31b9010
Restore a block of code deleted by mistake
jmao-denver Oct 29, 2024
ccc3253
debug Windows FT build
jmao-denver Oct 29, 2024
d972d96
Add comments, use version directives
jmao-denver Oct 30, 2024
4058638
Code cleanup
jmao-denver Oct 31, 2024
173592d
Fix a bug in convert() for java array type
jmao-denver Nov 2, 2024
eea87e7
Remove duplicate comment
jmao-denver Nov 4, 2024
c9d7bfa
Add clarifying comment
jmao-denver Nov 4, 2024
1c51917
Respond to review comments
jmao-denver Nov 6, 2024
99f130a
Naming change to be more applicable in FT mode
jmao-denver Nov 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 87 additions & 4 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ jobs:
distribution: 'temurin'
java-version: '8'

- run: pip install setuptools
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
- run: ${{ matrix.info.cmd }}

- uses: actions/upload-artifact@v4
Expand Down Expand Up @@ -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
Expand All @@ -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-latest', python: '3.13t', java: '11', arch: 'arm64', cmd: '.github/env/macOS/bdist-wheel.sh' }
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved

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 }}
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved

- 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
Comment on lines +218 to +222
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting, this is different workflow for building aarch64 wheels than we use for the other aarch64 wheels, which are built via aarch64 emulation in docker. If we think think this is a better way, we may consider migrating the other ones over to cibuildwheel in the future (and potentially amd64?).

The nice thing about the docker emulation is that users can build everything locally without needing to rely on CI.

Copy link
Contributor Author

@jmao-denver jmao-denver Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still uses emulation but the nice thing about CBW under aarch64 simulation is that it can build both amd64 and aarch64 wheels. Followup ticket: #173

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
Expand Down
48 changes: 48 additions & 0 deletions .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see windows-specific and mac-specific code, but I don't see CI checks for the platforms. There do appear to be windows runners. https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners
It isn't the highest priority, but I am noting it.

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_ENV
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it is still using GITHUB_ENV?


- 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
26 changes: 22 additions & 4 deletions jpyutil.py
devinrsmith marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -326,18 +330,32 @@ def _find_python_dll_file(fail=False):

# Prepare list of possible library file names

# account for Python debug builds
try:
sys.gettotalrefcount()
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
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))
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
20 changes: 14 additions & 6 deletions src/main/c/jni/org_jpy_PyLib.c
Original file line number Diff line number Diff line change
Expand Up @@ -499,12 +499,20 @@ void dumpDict(const char* dictName, PyObject* dict)

size = PyDict_Size(dict);
printf(">> dumpDict: %s.size = %ld\n", dictName, size);
#ifdef Py_GIL_DISABLED
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
// 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++;
}
#ifdef Py_GIL_DISABLED
Py_END_CRITICAL_SECTION();
#endif
}

/**
Expand All @@ -521,7 +529,7 @@ PyObject *getMainGlobals() {
}

pyGlobals = PyModule_GetDict(pyMainModule); // borrowed ref
JPy_INCREF(pyGlobals);
JPy_XINCREF(pyGlobals);

return pyGlobals;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
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);

Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions src/main/c/jpy_jmethod.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
17 changes: 16 additions & 1 deletion src/main/c/jpy_jobj.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,19 +72,33 @@ 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
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
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);
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
JPy_XDECREF(obj);
if (callableResult == NULL) {
Py_RETURN_NONE;
} else {
return callableResult;
}
}
}
JPy_XDECREF(callable);

return (PyObject *)obj;
}
Expand All @@ -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 "')");
Expand Down
Loading
Loading