From f4748103959a265f38bed80891ecea825d164564 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 16 Aug 2024 07:05:41 -0600 Subject: [PATCH] Initial free threaded bindings (#4421) * add support in pyo3-build-config for free-threaded python * update object.h bindings for free-threaded build * Add PyMutex bindings * fix po3-ffi-check with free-threaded build * error when building with limited api and Py_GIL_DISABLED * Add CI job for free-threaded build * fix issues building on older pythons * ci config fixup * fix clippy on gil-enabled 3.13 build * Apply suggestions from code review Co-authored-by: David Hewitt * make PyMutex and PyObject refcounting fields atomics * add new field on PyConfig in 3.13 debug ABI * warn and disable abi3 on gil-disabled build * fix conditional compilation for PyMutex usage * temporarily skip test that deadlocks * remove Py_GIL_DISABLED from py_sys_config cfg options * only expose PyMutex in 3.13 * make PyObject_HEAD_INIT a function * intialize ob_ref_local to _Py_IMMORTAL_REFCNT_LOCAL in HEAD_INIT * Fix clippy lint about static with interior mutability * add TODO comments about INCREF and DECREF in free-threaded build * make the _bits field of PyMutex pub(crate) * refactor so HEAD_INIT remains a constant * ignore clippy lint about interior mutability * revert unnecessary changes to pyo3-build-config * add changelog entries * use derive(Debug) for PyMutex * Add PhantomPinned field to PyMutex bindings * Update pyo3-build-config/src/impl_.rs Co-authored-by: David Hewitt * Update pyo3-ffi/src/object.rs Co-authored-by: David Hewitt * fix build config again --------- Co-authored-by: David Hewitt --- .github/workflows/ci.yml | 30 +++++++++++ newsfragments/4421.added.md | 1 + newsfragments/4421.fixed.md | 1 + pyo3-build-config/src/impl_.rs | 47 +++++++++++++++- pyo3-build-config/src/lib.rs | 1 + pyo3-ffi/build.rs | 5 ++ pyo3-ffi/src/cpython/initconfig.rs | 4 ++ pyo3-ffi/src/cpython/lock.rs | 14 +++++ pyo3-ffi/src/cpython/mod.rs | 4 ++ pyo3-ffi/src/cpython/weakrefobject.rs | 2 + pyo3-ffi/src/moduleobject.rs | 1 + pyo3-ffi/src/object.rs | 78 ++++++++++++++++++++++++--- src/impl_/pymodule.rs | 1 + tests/test_dict_iter.rs | 1 + 14 files changed, 182 insertions(+), 8 deletions(-) create mode 100644 newsfragments/4421.added.md create mode 100644 newsfragments/4421.fixed.md create mode 100644 pyo3-ffi/src/cpython/lock.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c8f4b5dcc67..2f2e093937d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -489,6 +489,35 @@ jobs: echo PYO3_CONFIG_FILE=$PYO3_CONFIG_FILE >> $GITHUB_ENV - run: python3 -m nox -s test + test-free-threaded: + if: ${{ contains(github.event.pull_request.labels.*.name, 'CI-build-full') || github.event_name != 'pull_request' }} + needs: [fmt] + runs-on: ubuntu-latest + env: + UNSAFE_PYO3_BUILD_FREE_THREADED: 1 + steps: + - uses: actions/checkout@v4 + - uses: Swatinem/rust-cache@v2 + with: + save-if: ${{ github.event_name != 'merge_group' }} + - uses: dtolnay/rust-toolchain@stable + with: + components: rust-src + # TODO: replace with setup-python when there is support + - uses: deadsnakes/action@v3.1.0 + with: + python-version: '3.13-dev' + nogil: true + - run: python3 -m sysconfig + - run: python3 -m pip install --upgrade pip && pip install nox + - run: nox -s ffi-check + - name: Run default nox sessions that should pass + run: nox -s clippy docs rustfmt ruff + - name: Run PyO3 tests with free-threaded Python (can fail) + # TODO fix the test crashes so we can unset this + continue-on-error: true + run: nox -s test + test-version-limits: needs: [fmt] if: ${{ contains(github.event.pull_request.labels.*.name, 'CI-build-full') || github.event_name != 'pull_request' }} @@ -627,6 +656,7 @@ jobs: - coverage - emscripten - test-debug + - test-free-threaded - test-version-limits - check-feature-powerset - test-cross-compilation diff --git a/newsfragments/4421.added.md b/newsfragments/4421.added.md new file mode 100644 index 00000000000..b0a85bea3ca --- /dev/null +++ b/newsfragments/4421.added.md @@ -0,0 +1 @@ +* Added bindings for PyMutex. diff --git a/newsfragments/4421.fixed.md b/newsfragments/4421.fixed.md new file mode 100644 index 00000000000..075b1fa7b5a --- /dev/null +++ b/newsfragments/4421.fixed.md @@ -0,0 +1 @@ +* Updated FFI bindings for free-threaded CPython 3.13 ABI diff --git a/pyo3-build-config/src/impl_.rs b/pyo3-build-config/src/impl_.rs index d38d41ed552..c8f68864727 100644 --- a/pyo3-build-config/src/impl_.rs +++ b/pyo3-build-config/src/impl_.rs @@ -177,12 +177,18 @@ impl InterpreterConfig { PythonImplementation::GraalPy => out.push("cargo:rustc-cfg=GraalPy".to_owned()), } - if self.abi3 { + // If Py_GIL_DISABLED is set, do not build with limited API support + if self.abi3 && !self.build_flags.0.contains(&BuildFlag::Py_GIL_DISABLED) { out.push("cargo:rustc-cfg=Py_LIMITED_API".to_owned()); } for flag in &self.build_flags.0 { - out.push(format!("cargo:rustc-cfg=py_sys_config=\"{}\"", flag)); + match flag { + BuildFlag::Py_GIL_DISABLED => { + out.push("cargo:rustc-cfg=Py_GIL_DISABLED".to_owned()) + } + flag => out.push(format!("cargo:rustc-cfg=py_sys_config=\"{}\"", flag)), + } } out @@ -2733,6 +2739,43 @@ mod tests { ); } + #[test] + fn test_build_script_outputs_gil_disabled() { + let mut build_flags = BuildFlags::default(); + build_flags.0.insert(BuildFlag::Py_GIL_DISABLED); + let interpreter_config = InterpreterConfig { + implementation: PythonImplementation::CPython, + version: PythonVersion { + major: 3, + minor: 13, + }, + shared: true, + abi3: false, + lib_name: Some("python3".into()), + lib_dir: None, + executable: None, + pointer_width: None, + build_flags, + suppress_build_script_link_lines: false, + extra_build_script_lines: vec![], + }; + + assert_eq!( + interpreter_config.build_script_outputs(), + [ + "cargo:rustc-cfg=Py_3_6".to_owned(), + "cargo:rustc-cfg=Py_3_7".to_owned(), + "cargo:rustc-cfg=Py_3_8".to_owned(), + "cargo:rustc-cfg=Py_3_9".to_owned(), + "cargo:rustc-cfg=Py_3_10".to_owned(), + "cargo:rustc-cfg=Py_3_11".to_owned(), + "cargo:rustc-cfg=Py_3_12".to_owned(), + "cargo:rustc-cfg=Py_3_13".to_owned(), + "cargo:rustc-cfg=Py_GIL_DISABLED".to_owned(), + ] + ); + } + #[test] fn test_build_script_outputs_debug() { let mut build_flags = BuildFlags::default(); diff --git a/pyo3-build-config/src/lib.rs b/pyo3-build-config/src/lib.rs index 0bc2274e0e5..033e7b46540 100644 --- a/pyo3-build-config/src/lib.rs +++ b/pyo3-build-config/src/lib.rs @@ -166,6 +166,7 @@ pub fn print_expected_cfgs() { } println!("cargo:rustc-check-cfg=cfg(Py_LIMITED_API)"); + println!("cargo:rustc-check-cfg=cfg(Py_GIL_DISABLED)"); println!("cargo:rustc-check-cfg=cfg(PyPy)"); println!("cargo:rustc-check-cfg=cfg(GraalPy)"); println!("cargo:rustc-check-cfg=cfg(py_sys_config, values(\"Py_DEBUG\", \"Py_REF_DEBUG\", \"Py_TRACE_REFS\", \"COUNT_ALLOCS\"))"); diff --git a/pyo3-ffi/build.rs b/pyo3-ffi/build.rs index 83408b31222..8b3a98bf9f7 100644 --- a/pyo3-ffi/build.rs +++ b/pyo3-ffi/build.rs @@ -135,6 +135,11 @@ fn ensure_gil_enabled(interpreter_config: &InterpreterConfig) -> Result<()> { = help: set UNSAFE_PYO3_BUILD_FREE_THREADED=1 to suppress this check and build anyway for free-threaded Python", std::env::var("CARGO_PKG_VERSION").unwrap() ); + if interpreter_config.abi3 { + warn!( + "The free-threaded build of CPython does not yet support abi3 so the build artifacts will be version-specific." + ) + } Ok(()) } diff --git a/pyo3-ffi/src/cpython/initconfig.rs b/pyo3-ffi/src/cpython/initconfig.rs index 32931415888..321d200e141 100644 --- a/pyo3-ffi/src/cpython/initconfig.rs +++ b/pyo3-ffi/src/cpython/initconfig.rs @@ -143,6 +143,8 @@ pub struct PyConfig { pub int_max_str_digits: c_int, #[cfg(Py_3_13)] pub cpu_count: c_int, + #[cfg(Py_GIL_DISABLED)] + pub enable_gil: c_int, pub pathconfig_warnings: c_int, #[cfg(Py_3_10)] pub program_name: *mut wchar_t, @@ -177,6 +179,8 @@ pub struct PyConfig { pub _is_python_build: c_int, #[cfg(all(Py_3_9, not(Py_3_10)))] pub _orig_argv: PyWideStringList, + #[cfg(all(Py_3_13, py_sys_config = "Py_DEBUG"))] + pub run_presite: *mut wchar_t, } extern "C" { diff --git a/pyo3-ffi/src/cpython/lock.rs b/pyo3-ffi/src/cpython/lock.rs new file mode 100644 index 00000000000..05778dfe573 --- /dev/null +++ b/pyo3-ffi/src/cpython/lock.rs @@ -0,0 +1,14 @@ +use std::marker::PhantomPinned; +use std::sync::atomic::AtomicU8; + +#[repr(transparent)] +#[derive(Debug)] +pub struct PyMutex { + pub(crate) _bits: AtomicU8, + pub(crate) _pin: PhantomPinned, +} + +extern "C" { + pub fn PyMutex_Lock(m: *mut PyMutex); + pub fn PyMutex_UnLock(m: *mut PyMutex); +} diff --git a/pyo3-ffi/src/cpython/mod.rs b/pyo3-ffi/src/cpython/mod.rs index 1710dbc4122..8f850104def 100644 --- a/pyo3-ffi/src/cpython/mod.rs +++ b/pyo3-ffi/src/cpython/mod.rs @@ -18,6 +18,8 @@ pub(crate) mod import; pub(crate) mod initconfig; // skipped interpreteridobject.h pub(crate) mod listobject; +#[cfg(Py_3_13)] +pub(crate) mod lock; pub(crate) mod longobject; #[cfg(all(Py_3_9, not(PyPy)))] pub(crate) mod methodobject; @@ -54,6 +56,8 @@ pub use self::import::*; #[cfg(all(Py_3_8, not(PyPy)))] pub use self::initconfig::*; pub use self::listobject::*; +#[cfg(Py_3_13)] +pub use self::lock::*; pub use self::longobject::*; #[cfg(all(Py_3_9, not(PyPy)))] pub use self::methodobject::*; diff --git a/pyo3-ffi/src/cpython/weakrefobject.rs b/pyo3-ffi/src/cpython/weakrefobject.rs index 3a232c7ed38..88bb501bcc5 100644 --- a/pyo3-ffi/src/cpython/weakrefobject.rs +++ b/pyo3-ffi/src/cpython/weakrefobject.rs @@ -8,6 +8,8 @@ pub struct _PyWeakReference { pub wr_next: *mut crate::PyWeakReference, #[cfg(Py_3_11)] pub vectorcall: Option, + #[cfg(all(Py_3_13, Py_GIL_DISABLED))] + pub weakrefs_lock: *mut crate::PyMutex, } // skipped _PyWeakref_GetWeakrefCount diff --git a/pyo3-ffi/src/moduleobject.rs b/pyo3-ffi/src/moduleobject.rs index 04b0f4ac25f..ff6458f4b15 100644 --- a/pyo3-ffi/src/moduleobject.rs +++ b/pyo3-ffi/src/moduleobject.rs @@ -60,6 +60,7 @@ pub struct PyModuleDef_Base { pub m_copy: *mut PyObject, } +#[allow(clippy::declare_interior_mutable_const)] pub const PyModuleDef_HEAD_INIT: PyModuleDef_Base = PyModuleDef_Base { ob_base: PyObject_HEAD_INIT, m_init: None, diff --git a/pyo3-ffi/src/object.rs b/pyo3-ffi/src/object.rs index 9181cb17dd2..27436c208a9 100644 --- a/pyo3-ffi/src/object.rs +++ b/pyo3-ffi/src/object.rs @@ -1,7 +1,13 @@ use crate::pyport::{Py_hash_t, Py_ssize_t}; +#[cfg(Py_GIL_DISABLED)] +use crate::PyMutex; +#[cfg(Py_GIL_DISABLED)] +use std::marker::PhantomPinned; use std::mem; use std::os::raw::{c_char, c_int, c_uint, c_ulong, c_void}; use std::ptr; +#[cfg(Py_GIL_DISABLED)] +use std::sync::atomic::{AtomicIsize, AtomicU32, AtomicU8, Ordering::Relaxed}; #[cfg(Py_LIMITED_API)] opaque_struct!(PyTypeObject); @@ -22,12 +28,33 @@ pub const _Py_IMMORTAL_REFCNT: Py_ssize_t = { } }; +#[cfg(Py_GIL_DISABLED)] +pub const _Py_IMMORTAL_REFCNT_LOCAL: u32 = u32::MAX; +#[cfg(Py_GIL_DISABLED)] +pub const _Py_REF_SHARED_SHIFT: isize = 2; + +#[allow(clippy::declare_interior_mutable_const)] pub const PyObject_HEAD_INIT: PyObject = PyObject { #[cfg(py_sys_config = "Py_TRACE_REFS")] _ob_next: std::ptr::null_mut(), #[cfg(py_sys_config = "Py_TRACE_REFS")] _ob_prev: std::ptr::null_mut(), - #[cfg(Py_3_12)] + #[cfg(Py_GIL_DISABLED)] + ob_tid: 0, + #[cfg(Py_GIL_DISABLED)] + _padding: 0, + #[cfg(Py_GIL_DISABLED)] + ob_mutex: PyMutex { + _bits: AtomicU8::new(0), + _pin: PhantomPinned, + }, + #[cfg(Py_GIL_DISABLED)] + ob_gc_bits: 0, + #[cfg(Py_GIL_DISABLED)] + ob_ref_local: AtomicU32::new(_Py_IMMORTAL_REFCNT_LOCAL), + #[cfg(Py_GIL_DISABLED)] + ob_ref_shared: AtomicIsize::new(0), + #[cfg(all(not(Py_GIL_DISABLED), Py_3_12))] ob_refcnt: PyObjectObRefcnt { ob_refcnt: 1 }, #[cfg(not(Py_3_12))] ob_refcnt: 1, @@ -67,6 +94,19 @@ pub struct PyObject { pub _ob_next: *mut PyObject, #[cfg(py_sys_config = "Py_TRACE_REFS")] pub _ob_prev: *mut PyObject, + #[cfg(Py_GIL_DISABLED)] + pub ob_tid: libc::uintptr_t, + #[cfg(Py_GIL_DISABLED)] + pub _padding: u16, + #[cfg(Py_GIL_DISABLED)] + pub ob_mutex: PyMutex, // per-object lock + #[cfg(Py_GIL_DISABLED)] + pub ob_gc_bits: u8, // gc-related state + #[cfg(Py_GIL_DISABLED)] + pub ob_ref_local: AtomicU32, // local reference count + #[cfg(Py_GIL_DISABLED)] + pub ob_ref_shared: AtomicIsize, // shared reference count + #[cfg(not(Py_GIL_DISABLED))] pub ob_refcnt: PyObjectObRefcnt, #[cfg(PyPy)] pub ob_pypy_link: Py_ssize_t, @@ -91,6 +131,18 @@ pub unsafe fn Py_Is(x: *mut PyObject, y: *mut PyObject) -> c_int { } #[inline] +#[cfg(Py_GIL_DISABLED)] +pub unsafe fn Py_REFCNT(ob: *mut PyObject) -> Py_ssize_t { + let local = (*ob).ob_ref_local.load(Relaxed); + if local == _Py_IMMORTAL_REFCNT_LOCAL { + return _Py_IMMORTAL_REFCNT; + } + let shared = (*ob).ob_ref_shared.load(Relaxed); + local as Py_ssize_t + Py_ssize_t::from(shared >> _Py_REF_SHARED_SHIFT) +} + +#[inline] +#[cfg(not(Py_GIL_DISABLED))] #[cfg(Py_3_12)] pub unsafe fn Py_REFCNT(ob: *mut PyObject) -> Py_ssize_t { (*ob).ob_refcnt.ob_refcnt @@ -134,7 +186,7 @@ pub unsafe fn Py_IS_TYPE(ob: *mut PyObject, tp: *mut PyTypeObject) -> c_int { } #[inline(always)] -#[cfg(all(Py_3_12, target_pointer_width = "64"))] +#[cfg(all(not(Py_GIL_DISABLED), Py_3_12, target_pointer_width = "64"))] pub unsafe fn _Py_IsImmortal(op: *mut PyObject) -> c_int { (((*op).ob_refcnt.ob_refcnt as crate::PY_INT32_T) < 0) as c_int } @@ -507,8 +559,14 @@ extern "C" { #[inline(always)] pub unsafe fn Py_INCREF(op: *mut PyObject) { - // On limited API or with refcount debugging, let the interpreter do refcounting - #[cfg(any(Py_LIMITED_API, py_sys_config = "Py_REF_DEBUG", GraalPy))] + // On limited API, the free-threaded build, or with refcount debugging, let the interpreter do refcounting + // TODO: reimplement the logic in the header in the free-threaded build, for a little bit of performance. + #[cfg(any( + Py_GIL_DISABLED, + Py_LIMITED_API, + py_sys_config = "Py_REF_DEBUG", + GraalPy + ))] { // _Py_IncRef was added to the ABI in 3.10; skips null checks #[cfg(all(Py_3_10, not(PyPy)))] @@ -523,7 +581,12 @@ pub unsafe fn Py_INCREF(op: *mut PyObject) { } // version-specific builds are allowed to directly manipulate the reference count - #[cfg(not(any(any(Py_LIMITED_API, py_sys_config = "Py_REF_DEBUG", GraalPy))))] + #[cfg(not(any( + Py_GIL_DISABLED, + Py_LIMITED_API, + py_sys_config = "Py_REF_DEBUG", + GraalPy + )))] { #[cfg(all(Py_3_12, target_pointer_width = "64"))] { @@ -559,9 +622,11 @@ pub unsafe fn Py_INCREF(op: *mut PyObject) { track_caller )] pub unsafe fn Py_DECREF(op: *mut PyObject) { - // On limited API or with refcount debugging, let the interpreter do refcounting + // On limited API, the free-threaded build, or with refcount debugging, let the interpreter do refcounting // On 3.12+ we implement refcount debugging to get better assertion locations on negative refcounts + // TODO: reimplement the logic in the header in the free-threaded build, for a little bit of performance. #[cfg(any( + Py_GIL_DISABLED, Py_LIMITED_API, all(py_sys_config = "Py_REF_DEBUG", not(Py_3_12)), GraalPy @@ -580,6 +645,7 @@ pub unsafe fn Py_DECREF(op: *mut PyObject) { } #[cfg(not(any( + Py_GIL_DISABLED, Py_LIMITED_API, all(py_sys_config = "Py_REF_DEBUG", not(Py_3_12)), GraalPy diff --git a/src/impl_/pymodule.rs b/src/impl_/pymodule.rs index d199bf95aac..8ce169fffa2 100644 --- a/src/impl_/pymodule.rs +++ b/src/impl_/pymodule.rs @@ -55,6 +55,7 @@ impl ModuleDef { doc: &'static CStr, initializer: ModuleInitializer, ) -> Self { + #[allow(clippy::declare_interior_mutable_const)] const INIT: ffi::PyModuleDef = ffi::PyModuleDef { m_base: ffi::PyModuleDef_HEAD_INIT, m_name: std::ptr::null(), diff --git a/tests/test_dict_iter.rs b/tests/test_dict_iter.rs index dc32eb61fd7..5b3573d10ad 100644 --- a/tests/test_dict_iter.rs +++ b/tests/test_dict_iter.rs @@ -3,6 +3,7 @@ use pyo3::types::IntoPyDict; #[test] #[cfg_attr(target_arch = "wasm32", ignore)] // Not sure why this fails. +#[cfg_attr(Py_GIL_DISABLED, ignore)] // test deadlocks in GIL-disabled build, TODO: fix deadlock fn iter_dict_nosegv() { Python::with_gil(|py| { const LEN: usize = 10_000_000;