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

Initial free threaded bindings #4421

Merged
merged 31 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
2685691
add support in pyo3-build-config for free-threaded python
ngoldbaum Aug 6, 2024
e8613d5
update object.h bindings for free-threaded build
ngoldbaum Aug 6, 2024
eeac20c
Add PyMutex bindings
ngoldbaum Aug 6, 2024
8eebe43
fix po3-ffi-check with free-threaded build
ngoldbaum Aug 6, 2024
9e9d77e
error when building with limited api and Py_GIL_DISABLED
ngoldbaum Aug 6, 2024
79dd1a7
Add CI job for free-threaded build
ngoldbaum Jul 31, 2024
5aaa03a
fix issues building on older pythons
ngoldbaum Aug 6, 2024
f4219b3
ci config fixup
ngoldbaum Aug 6, 2024
4ff9a62
fix clippy on gil-enabled 3.13 build
ngoldbaum Aug 6, 2024
57e7e3d
Apply suggestions from code review
ngoldbaum Aug 13, 2024
a042e56
make PyMutex and PyObject refcounting fields atomics
ngoldbaum Aug 13, 2024
7c493eb
add new field on PyConfig in 3.13 debug ABI
ngoldbaum Aug 13, 2024
da3e50c
warn and disable abi3 on gil-disabled build
ngoldbaum Aug 13, 2024
77b4d96
fix conditional compilation for PyMutex usage
ngoldbaum Aug 13, 2024
96dd13e
temporarily skip test that deadlocks
ngoldbaum Aug 13, 2024
dbcb9d2
remove Py_GIL_DISABLED from py_sys_config cfg options
ngoldbaum Aug 13, 2024
d9dac3c
only expose PyMutex in 3.13
ngoldbaum Aug 13, 2024
9b70b07
make PyObject_HEAD_INIT a function
ngoldbaum Aug 14, 2024
3e03dd1
intialize ob_ref_local to _Py_IMMORTAL_REFCNT_LOCAL in HEAD_INIT
ngoldbaum Aug 14, 2024
f695203
Fix clippy lint about static with interior mutability
ngoldbaum Aug 14, 2024
848c793
add TODO comments about INCREF and DECREF in free-threaded build
ngoldbaum Aug 14, 2024
6e66cb0
make the _bits field of PyMutex pub(crate)
ngoldbaum Aug 14, 2024
126c97b
refactor so HEAD_INIT remains a constant
ngoldbaum Aug 14, 2024
162a65e
ignore clippy lint about interior mutability
ngoldbaum Aug 14, 2024
6103d26
revert unnecessary changes to pyo3-build-config
ngoldbaum Aug 14, 2024
8a9ef2d
add changelog entries
ngoldbaum Aug 14, 2024
e1ddd01
use derive(Debug) for PyMutex
ngoldbaum Aug 15, 2024
1a088ea
Add PhantomPinned field to PyMutex bindings
ngoldbaum Aug 15, 2024
38bc492
Update pyo3-build-config/src/impl_.rs
ngoldbaum Aug 15, 2024
a85fa4c
Update pyo3-ffi/src/object.rs
ngoldbaum Aug 15, 2024
60c26c4
fix build config again
ngoldbaum Aug 15, 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
30 changes: 30 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,35 @@ jobs:
echo PYO3_CONFIG_FILE=$PYO3_CONFIG_FILE >> $GITHUB_ENV
- run: python3 -m nox -s test

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.

Very cool to see this job running, thanks 🙏

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' }}
Expand Down Expand Up @@ -627,6 +656,7 @@ jobs:
- coverage
- emscripten
- test-debug
- test-free-threaded
- test-version-limits
- check-feature-powerset
- test-cross-compilation
Expand Down
1 change: 1 addition & 0 deletions newsfragments/4421.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Added bindings for PyMutex.
1 change: 1 addition & 0 deletions newsfragments/4421.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Updated FFI bindings for free-threaded CPython 3.13 ABI
48 changes: 46 additions & 2 deletions pyo3-build-config/src/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,19 @@ 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());
}

if self.build_flags.0.contains(&BuildFlag::Py_GIL_DISABLED) {
out.push("cargo:rustc-cfg=Py_GIL_DISABLED".to_owned());
}

for flag in &self.build_flags.0 {
out.push(format!("cargo:rustc-cfg=py_sys_config=\"{}\"", flag));
if flag != &BuildFlag::Py_GIL_DISABLED {
out.push(format!("cargo:rustc-cfg=py_sys_config=\"{}\"", flag));
}
ngoldbaum marked this conversation as resolved.
Show resolved Hide resolved
}

out
Expand Down Expand Up @@ -2733,6 +2740,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();
Expand Down
1 change: 1 addition & 0 deletions pyo3-build-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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\"))");
Expand Down
5 changes: 5 additions & 0 deletions pyo3-ffi/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
4 changes: 4 additions & 0 deletions pyo3-ffi/src/cpython/initconfig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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" {
Expand Down
18 changes: 18 additions & 0 deletions pyo3-ffi/src/cpython/lock.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
use std::fmt;
use std::sync::atomic::AtomicU8;

#[repr(C)]
pub struct PyMutex {
ngoldbaum marked this conversation as resolved.
Show resolved Hide resolved
pub(crate) _bits: AtomicU8,
}

Choose a reason for hiding this comment

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

Looking at Rust's built-in mutex types, I think we'd want a constructor like:

impl PyMutex {
    pub const fn new() -> PyMutex {
        PyMutex { _bits: AtomicU8::new(0) }
    }
}


impl fmt::Debug for PyMutex {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("PyMutex").finish()
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This can be a #[derive(Debug)] now that the field is an atomic.


extern "C" {
pub fn PyMutex_Lock(m: *mut PyMutex);
pub fn PyMutex_UnLock(m: *mut PyMutex);
}
4 changes: 4 additions & 0 deletions pyo3-ffi/src/cpython/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::*;
Expand Down
2 changes: 2 additions & 0 deletions pyo3-ffi/src/cpython/weakrefobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ pub struct _PyWeakReference {
pub wr_next: *mut crate::PyWeakReference,
#[cfg(Py_3_11)]
pub vectorcall: Option<crate::vectorcallfunc>,
#[cfg(all(Py_3_13, Py_GIL_DISABLED))]
pub weakrefs_lock: *mut crate::PyMutex,
}

// skipped _PyWeakref_GetWeakrefCount
Expand Down
1 change: 1 addition & 0 deletions pyo3-ffi/src/moduleobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,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,
Expand Down
75 changes: 69 additions & 6 deletions pyo3-ffi/src/object.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
use crate::pyport::{Py_hash_t, Py_ssize_t};
#[cfg(Py_GIL_DISABLED)]
use crate::PyMutex;
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);
Expand All @@ -22,12 +26,32 @@ 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),
},
#[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,
Expand Down Expand Up @@ -67,6 +91,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: usize,
ngoldbaum marked this conversation as resolved.
Show resolved Hide resolved
#[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,
Expand All @@ -91,6 +128,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);
Comment on lines +136 to +140
Copy link
Member

Choose a reason for hiding this comment

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

I checked the CPython source to verify these are indeed relaxed loads 👍

local as Py_ssize_t + Py_ssize_t::from(shared >> _Py_REF_SHARED_SHIFT)
davidhewitt marked this conversation as resolved.
Show resolved Hide resolved
}

#[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
Expand Down Expand Up @@ -134,7 +183,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
}
Expand Down Expand Up @@ -507,8 +556,14 @@ extern "C" {

#[inline(always)]
pub unsafe fn Py_INCREF(op: *mut PyObject) {
ngoldbaum marked this conversation as resolved.
Show resolved Hide resolved
// 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)))]
Expand All @@ -523,7 +578,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"))]
{
Expand Down Expand Up @@ -559,9 +619,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
Expand All @@ -580,6 +642,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
Expand Down
1 change: 1 addition & 0 deletions src/impl_/pymodule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,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(),
Expand Down
1 change: 1 addition & 0 deletions tests/test_dict_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading