From 0d3af59f68cbcc3d4a3fd9e90cfbe61424d37bef Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 4 Aug 2025 16:35:04 -0600 Subject: [PATCH 01/32] WIP: attempt to add with_critical_section_mutex --- pyo3-ffi/src/cpython/critical_section.rs | 9 +- src/sync.rs | 170 ++++++++++++++++++++--- src/types/mutex.rs | 4 +- 3 files changed, 157 insertions(+), 26 deletions(-) diff --git a/pyo3-ffi/src/cpython/critical_section.rs b/pyo3-ffi/src/cpython/critical_section.rs index 808dba870c6..d63a1cd43a8 100644 --- a/pyo3-ffi/src/cpython/critical_section.rs +++ b/pyo3-ffi/src/cpython/critical_section.rs @@ -1,4 +1,3 @@ -#[cfg(Py_GIL_DISABLED)] use crate::PyMutex; use crate::PyObject; @@ -24,7 +23,15 @@ opaque_struct!(pub PyCriticalSection2); extern "C" { pub fn PyCriticalSection_Begin(c: *mut PyCriticalSection, op: *mut PyObject); + #[cfg(Py_3_14)] + pub fn PyCriticalSection_BeginMutex(c: *mut PyCriticalSection, m: *mut PyMutex); pub fn PyCriticalSection_End(c: *mut PyCriticalSection); pub fn PyCriticalSection2_Begin(c: *mut PyCriticalSection2, a: *mut PyObject, b: *mut PyObject); + #[cfg(Py_3_14)] + pub fn PyCriticalSection2_BeginMutex( + c: *mut PyCriticalSection2, + m1: *mut PyMutex, + m2: *mut PyMutex, + ); pub fn PyCriticalSection2_End(c: *mut PyCriticalSection2); } diff --git a/src/sync.rs b/src/sync.rs index 40d77d712a9..4ed9dd98595 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -12,7 +12,7 @@ use crate::{ internal::state::SuspendAttach, sealed::Sealed, - types::{any::PyAnyMethods, PyAny, PyString}, + types::{any::PyAnyMethods, PyAny, PyMutex, PyString}, Bound, Py, PyResult, PyTypeCheck, Python, }; use std::{ @@ -460,6 +460,30 @@ impl Interned { } } +#[cfg(Py_GIL_DISABLED)] +struct CSGuard(crate::ffi::PyCriticalSection); + +#[cfg(Py_GIL_DISABLED)] +impl Drop for CSGuard { + fn drop(&mut self) { + unsafe { + crate::ffi::PyCriticalSection_End(&mut self.0); + } + } +} + +#[cfg(Py_GIL_DISABLED)] +struct CS2Guard(crate::ffi::PyCriticalSection2); + +#[cfg(Py_GIL_DISABLED)] +impl Drop for CS2Guard { + fn drop(&mut self) { + unsafe { + crate::ffi::PyCriticalSection2_End(&mut self.0); + } + } +} + /// Executes a closure with a Python critical section held on an object. /// /// Acquires the per-object lock for the object `op` that is held @@ -487,17 +511,7 @@ where { #[cfg(Py_GIL_DISABLED)] { - struct Guard(crate::ffi::PyCriticalSection); - - impl Drop for Guard { - fn drop(&mut self) { - unsafe { - crate::ffi::PyCriticalSection_End(&mut self.0); - } - } - } - - let mut guard = Guard(unsafe { std::mem::zeroed() }); + let mut guard = CSGuard(unsafe { std::mem::zeroed() }); unsafe { crate::ffi::PyCriticalSection_Begin(&mut guard.0, object.as_ptr()) }; f() } @@ -534,17 +548,7 @@ where { #[cfg(Py_GIL_DISABLED)] { - struct Guard(crate::ffi::PyCriticalSection2); - - impl Drop for Guard { - fn drop(&mut self) { - unsafe { - crate::ffi::PyCriticalSection2_End(&mut self.0); - } - } - } - - let mut guard = Guard(unsafe { std::mem::zeroed() }); + let mut guard = CS2Guard(unsafe { std::mem::zeroed() }); unsafe { crate::ffi::PyCriticalSection2_Begin(&mut guard.0, a.as_ptr(), b.as_ptr()) }; f() } @@ -554,6 +558,100 @@ where } } +/// Executes a closure with a Python critical section held on mutex. +/// +/// Acquires the per-object lock for the mutex `mutex` that is held +/// until the closure `f` is finished. +/// +/// This is structurally equivalent to the use of the paired +/// Py_BEGIN_CRITICAL_SECTION_MUTEX and Py_END_CRITICAL_SECTION C-API macros. +/// +/// A no-op on GIL-enabled builds, where the critical section API is exposed as +/// a no-op by the Python C API. +/// +/// Provides weaker locking guarantees than traditional locks, but can in some +/// cases be used to provide guarantees similar to the GIL without the risk of +/// deadlocks associated with traditional locks. +/// +/// This variant with a mutex as an argument is particularly useful when paired +/// with a static `PyMutex`. This can be used in situations where a `PyMutex` would +/// deadlock with the interpreter. +/// +/// Many CPython C API functions do not acquire the per-object lock on objects +/// passed to Python. You should not expect critical sections applied to +/// built-in types to prevent concurrent modification. This API is most useful +/// for user-defined types with full control over how the internal state for the +/// type is managed. +/// +/// Only available on Python 3.14 and newer. +#[cfg(Py_3_14)] +#[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] +pub fn with_critical_section_mutex(mutex: &mut PyMutex, f: F) -> R +where + F: FnOnce(&mut T) -> R, +{ + #[cfg(Py_GIL_DISABLED)] + { + let mut guard = CSGuard(unsafe { std::mem::zeroed() }); + unsafe { crate::ffi::PyCriticalSection_BeginMutex(&mut guard.0, &mut *mutex.mutex.get()) }; + f(mutex.data.get_mut()) + } + #[cfg(not(Py_GIL_DISABLED))] + { + f() + } +} + +/// Executes a closure with a Python critical section held on two mutexes. +/// +/// Simultaneously acquires the mutexes `m1` and `m2` and holds them +/// until the closure `f` is finished. +/// +/// This is structurally equivalent to the use of the paired +/// Py_BEGIN_CRITICAL_SECTION2_MUTEX and Py_END_CRITICAL_SECTION2 C-API macros. +/// +/// A no-op on GIL-enabled builds, where the critical section API is exposed as +/// a no-op by the Python C API. +/// +/// Provides weaker locking guarantees than traditional locks, but can in some +/// cases be used to provide guarantees similar to the GIL without the risk of +/// deadlocks associated with traditional locks. +/// +/// Many CPython C API functions do not acquire the per-object lock on objects +/// passed to Python. You should not expect critical sections applied to +/// built-in types to prevent concurrent modification. This API is most useful +/// for user-defined types with full control over how the internal state for the +/// type is managed. +/// +/// Only available on Python 3.14 and newer. +#[cfg(Py_3_14)] +#[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] +pub fn with_critical_section_mutex2( + m1: &mut PyMutex, + m2: &mut PyMutex, + f: F, +) -> R +where + F: FnOnce(&mut T1, &mut T2) -> R, +{ + #[cfg(Py_GIL_DISABLED)] + { + let mut guard = CS2Guard(unsafe { std::mem::zeroed() }); + unsafe { + crate::ffi::PyCriticalSection2_BeginMutex( + &mut guard.0, + &mut *m1.mutex.get(), + &mut *m2.mutex.get(), + ) + }; + f(m1.data.get_mut(), m2.data.get_mut()) + } + #[cfg(not(Py_GIL_DISABLED))] + { + f() + } +} + mod once_lock_ext_sealed { pub trait Sealed {} impl Sealed for std::sync::OnceLock {} @@ -1152,6 +1250,32 @@ mod tests { }); } + #[cfg(feature = "macros")] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section_mutex() { + let barrier = Barrier::new(2); + + let bool_wrapper = PyMutex::new(BoolWrapper(AtomicBool::new(false))); + + std::thread::scope(|s| { + s.spawn(|| { + with_critical_section_mutex(&mut bool_wrapper, |b| { + barrier.wait(); + std::thread::sleep(std::time::Duration::from_millis(10)); + b.0.store(true, Ordering::Release); + }); + }); + s.spawn(|| { + barrier.wait(); + // this blocks until the other thread's critical section finishes + with_critical_section_mutex(&mut bool_wrapper, |b| { + assert!(b.0.load(Ordering::Acquire)); + }); + }); + }); + } + #[cfg(feature = "macros")] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] diff --git a/src/types/mutex.rs b/src/types/mutex.rs index fe9f9e436f3..f72115431d8 100644 --- a/src/types/mutex.rs +++ b/src/types/mutex.rs @@ -117,9 +117,9 @@ pub(crate) struct Guard { /// the guard that would have otherwise been returned on a successful lock. This /// allows access to the data, despite the lock being poisoned. pub struct PyMutex { - mutex: UnsafeCell, + pub(crate) mutex: UnsafeCell, poison: Flag, - data: UnsafeCell, + pub(crate) data: UnsafeCell, } /// RAII guard to handle releasing a PyMutex lock. From 37aa45672fd109a349197d415fd0bbbe45c4aac8 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 22 Aug 2025 10:49:39 -0600 Subject: [PATCH 02/32] WIP: use an UnsafeCell --- src/sync.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 4ed9dd98595..34e5d837974 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -586,15 +586,16 @@ where /// Only available on Python 3.14 and newer. #[cfg(Py_3_14)] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section_mutex(mutex: &mut PyMutex, f: F) -> R +pub fn with_critical_section_mutex(mutex: &PyMutex, f: F) -> R where - F: FnOnce(&mut T) -> R, + F: FnOnce(&UnsafeCell) -> R, { #[cfg(Py_GIL_DISABLED)] { let mut guard = CSGuard(unsafe { std::mem::zeroed() }); unsafe { crate::ffi::PyCriticalSection_BeginMutex(&mut guard.0, &mut *mutex.mutex.get()) }; - f(mutex.data.get_mut()) + let cell = unsafe { &*(mutex.data.get() as *mut T as *const UnsafeCell) }; + f(&cell) } #[cfg(not(Py_GIL_DISABLED))] { @@ -632,7 +633,7 @@ pub fn with_critical_section_mutex2( f: F, ) -> R where - F: FnOnce(&mut T1, &mut T2) -> R, + F: FnOnce(&UnsafeCell, &UnsafeCell) -> R, { #[cfg(Py_GIL_DISABLED)] { @@ -644,7 +645,9 @@ where &mut *m2.mutex.get(), ) }; - f(m1.data.get_mut(), m2.data.get_mut()) + let cell1 = unsafe { &*(m1.mutex.get() as *mut T1 as *const UnsafeCell) }; + let cell2 = unsafe { &*(m2.mutex.get() as *mut T2 as *const UnsafeCell) }; + f(&cell1, &cell2) } #[cfg(not(Py_GIL_DISABLED))] { @@ -1263,14 +1266,14 @@ mod tests { with_critical_section_mutex(&mut bool_wrapper, |b| { barrier.wait(); std::thread::sleep(std::time::Duration::from_millis(10)); - b.0.store(true, Ordering::Release); + unsafe { (*b.get()).0.store(true, Ordering::Release) }; }); }); s.spawn(|| { barrier.wait(); // this blocks until the other thread's critical section finishes with_critical_section_mutex(&mut bool_wrapper, |b| { - assert!(b.0.load(Ordering::Acquire)); + assert!(b.get_mut().0.load(Ordering::Acquire)); }); }); }); From 86664b11702204dd758f17118af4bf8d0750626e Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 21 Nov 2025 12:34:05 -0700 Subject: [PATCH 03/32] finish UnsafeCell rewrite --- src/sync.rs | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 34e5d837974..421efc053aa 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -560,8 +560,10 @@ where /// Executes a closure with a Python critical section held on mutex. /// -/// Acquires the per-object lock for the mutex `mutex` that is held -/// until the closure `f` is finished. +/// Acquires the mutex `mutex` until the closure `f` is finishes. The mutex +/// may be temporarily released and re-acquired if closure calls back into +/// the interpreter, in a similar way to how the GIL can be released during +/// blocking calls. /// /// This is structurally equivalent to the use of the paired /// Py_BEGIN_CRITICAL_SECTION_MUTEX and Py_END_CRITICAL_SECTION C-API macros. @@ -574,8 +576,8 @@ where /// deadlocks associated with traditional locks. /// /// This variant with a mutex as an argument is particularly useful when paired -/// with a static `PyMutex`. This can be used in situations where a `PyMutex` would -/// deadlock with the interpreter. +/// with a static `PyMutex` to create a "local GIL" to protect global state +/// in an extension without introducing any deadlock risks. /// /// Many CPython C API functions do not acquire the per-object lock on objects /// passed to Python. You should not expect critical sections applied to @@ -586,7 +588,11 @@ where /// Only available on Python 3.14 and newer. #[cfg(Py_3_14)] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section_mutex(mutex: &PyMutex, f: F) -> R +pub unsafe fn with_critical_section_mutex<'py, F, R, T>( + _py: Python<'py>, + mutex: &PyMutex, + f: F, +) -> R where F: FnOnce(&UnsafeCell) -> R, { @@ -594,8 +600,7 @@ where { let mut guard = CSGuard(unsafe { std::mem::zeroed() }); unsafe { crate::ffi::PyCriticalSection_BeginMutex(&mut guard.0, &mut *mutex.mutex.get()) }; - let cell = unsafe { &*(mutex.data.get() as *mut T as *const UnsafeCell) }; - f(&cell) + f(&mutex.data) } #[cfg(not(Py_GIL_DISABLED))] { @@ -1259,21 +1264,27 @@ mod tests { fn test_critical_section_mutex() { let barrier = Barrier::new(2); - let bool_wrapper = PyMutex::new(BoolWrapper(AtomicBool::new(false))); + let mutex = PyMutex::new(false); std::thread::scope(|s| { - s.spawn(|| { - with_critical_section_mutex(&mut bool_wrapper, |b| { - barrier.wait(); - std::thread::sleep(std::time::Duration::from_millis(10)); - unsafe { (*b.get()).0.store(true, Ordering::Release) }; + s.spawn(|| unsafe { + Python::attach(|py| { + with_critical_section_mutex(py, &mutex, |b: &UnsafeCell| { + barrier.wait(); + std::thread::sleep(std::time::Duration::from_millis(10)); + (*b.get()) = true; + }); }); }); s.spawn(|| { - barrier.wait(); - // this blocks until the other thread's critical section finishes - with_critical_section_mutex(&mut bool_wrapper, |b| { - assert!(b.get_mut().0.load(Ordering::Acquire)); + Python::attach(|py| { + barrier.wait(); + // blocks until the other thread enters a critical section + unsafe { + with_critical_section_mutex(py, &mutex, |b: &UnsafeCell| { + assert!(*b.get() == true); + }); + } }); }); }); From 22023d1bdc3951b94f089c71b16e0e4291d01978 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 21 Nov 2025 13:15:38 -0700 Subject: [PATCH 04/32] finish and tweak docs --- src/sync.rs | 156 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 112 insertions(+), 44 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 421efc053aa..ef8a7e46514 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -487,7 +487,11 @@ impl Drop for CS2Guard { /// Executes a closure with a Python critical section held on an object. /// /// Acquires the per-object lock for the object `op` that is held -/// until the closure `f` is finished. +/// while the closure `f` is executing. The critical section may be temporarily +/// released and re-acquired if the closure calls back into the interpreter in +/// a manner that would block. This is similar to how the GIL can be released +/// during blocking calls. See the safety notes below for caveats about +/// releasing critical sections. /// /// This is structurally equivalent to the use of the paired /// Py_BEGIN_CRITICAL_SECTION and Py_END_CRITICAL_SECTION C-API macros. @@ -499,6 +503,16 @@ impl Drop for CS2Guard { /// cases be used to provide guarantees similar to the GIL without the risk of /// deadlocks associated with traditional locks. /// +/// The caller must ensure the closure cannot implicitly release the critical +/// section. If a multithreaded program calls back into the Python interpreter +/// in a manner that would cause the critical section to be released, the +/// per-object lock will be released and the state of the object may be read +/// from or modified by another thread. Concurrent modifications are still +/// impossible, but the state of object may change "underneath" a suspended +/// thread in possibly surprising ways. Note that many operations on Python +/// objects may call back into the interpreter in a blocking manner because +/// many C API calls can trigger the execution of arbitrary Python code. +/// /// Many CPython C API functions do not acquire the per-object lock on objects /// passed to Python. You should not expect critical sections applied to /// built-in types to prevent concurrent modification. This API is most useful @@ -524,7 +538,11 @@ where /// Executes a closure with a Python critical section held on two objects. /// /// Acquires the per-object lock for the objects `a` and `b` that are held -/// until the closure `f` is finished. +/// while the closure `f` is executing. The critical section may be temporarily +/// released and re-acquired if the closure calls back into the interpreter in +/// a manner that would block. This is similar to how the GIL can be released +/// during blocking calls. See the safety notes below for caveats about +/// releasing critical sections. /// /// This is structurally equivalent to the use of the paired /// Py_BEGIN_CRITICAL_SECTION2 and Py_END_CRITICAL_SECTION2 C-API macros. @@ -536,6 +554,16 @@ where /// cases be used to provide guarantees similar to the GIL without the risk of /// deadlocks associated with traditional locks. /// +/// The caller must ensure the closure cannot implicitly release the critical +/// section. If a multithreaded program calls back into the Python interpreter +/// in a manner that would cause the critical section to be released, the +/// per-object lock will be released and the state of the object may be read +/// from or modified by another thread. Concurrent modifications are still +/// impossible, but the state of object may change "underneath" a suspended +/// thread in possibly surprising ways. Note that many operations on Python +/// objects may call back into the interpreter in a blocking manner because +/// many C API calls can trigger the execution of arbitrary Python code. +/// /// Many CPython C API functions do not acquire the per-object lock on objects /// passed to Python. You should not expect critical sections applied to /// built-in types to prevent concurrent modification. This API is most useful @@ -558,12 +586,13 @@ where } } -/// Executes a closure with a Python critical section held on mutex. +/// Executes a closure with a Python critical section held on a `PyMutex`. /// -/// Acquires the mutex `mutex` until the closure `f` is finishes. The mutex -/// may be temporarily released and re-acquired if closure calls back into -/// the interpreter, in a similar way to how the GIL can be released during -/// blocking calls. +/// Acquires the mutex `mutex` until the closure `f` finishes. The mutex may be +/// temporarily released and re-acquired if the closure calls back into the +/// interpreter in a manner that would block. This is similar to how the GIL +/// can be released during blocking calls. See the safety notes below for +/// caveats about releasing critical sections. /// /// This is structurally equivalent to the use of the paired /// Py_BEGIN_CRITICAL_SECTION_MUTEX and Py_END_CRITICAL_SECTION C-API macros. @@ -571,28 +600,29 @@ where /// A no-op on GIL-enabled builds, where the critical section API is exposed as /// a no-op by the Python C API. /// +/// This variant is particularly useful when paired with a global `PyMutex` to +/// create a "local GIL" to protect global state in an extension in an +/// analogous manner to the GIL without introducing any deadlock risks or +/// affecting runtime behavior on the GIL-enabled build. +/// +/// # Safety +/// /// Provides weaker locking guarantees than traditional locks, but can in some /// cases be used to provide guarantees similar to the GIL without the risk of /// deadlocks associated with traditional locks. /// -/// This variant with a mutex as an argument is particularly useful when paired -/// with a static `PyMutex` to create a "local GIL" to protect global state -/// in an extension without introducing any deadlock risks. -/// -/// Many CPython C API functions do not acquire the per-object lock on objects -/// passed to Python. You should not expect critical sections applied to -/// built-in types to prevent concurrent modification. This API is most useful -/// for user-defined types with full control over how the internal state for the -/// type is managed. +/// The caller must ensure the closure cannot implicitly release the critical +/// section. If a multithreaded program calls back into the Python interpreter +/// in a manner that would cause the critical section to be released, the +/// `PyMutex` will be released and the resource protected by the `PyMutex` may +/// be read from or modified by another thread. Concurrent modifications are +/// still impossible, but the state of the resource may change "underneath" a +/// suspended thread in possibly surprising ways. /// /// Only available on Python 3.14 and newer. #[cfg(Py_3_14)] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub unsafe fn with_critical_section_mutex<'py, F, R, T>( - _py: Python<'py>, - mutex: &PyMutex, - f: F, -) -> R +pub fn with_critical_section_mutex<'py, F, R, T>(_py: Python<'py>, mutex: &PyMutex, f: F) -> R where F: FnOnce(&UnsafeCell) -> R, { @@ -604,14 +634,18 @@ where } #[cfg(not(Py_GIL_DISABLED))] { - f() + f(&mutex.data) } } -/// Executes a closure with a Python critical section held on two mutexes. +/// Executes a closure with a Python critical section held on two `PyMutex` instances. /// /// Simultaneously acquires the mutexes `m1` and `m2` and holds them -/// until the closure `f` is finished. +/// until the closure `f` is finished. The mutexes may be +/// temporarily released and re-acquired if the closure calls back into the +/// interpreter in a manner that would block. This is similar to how the GIL +/// can be released during blocking calls. See the safety notes below for +/// caveats about releasing critical sections. /// /// This is structurally equivalent to the use of the paired /// Py_BEGIN_CRITICAL_SECTION2_MUTEX and Py_END_CRITICAL_SECTION2 C-API macros. @@ -619,22 +653,27 @@ where /// A no-op on GIL-enabled builds, where the critical section API is exposed as /// a no-op by the Python C API. /// +/// # Safety +/// /// Provides weaker locking guarantees than traditional locks, but can in some /// cases be used to provide guarantees similar to the GIL without the risk of /// deadlocks associated with traditional locks. /// -/// Many CPython C API functions do not acquire the per-object lock on objects -/// passed to Python. You should not expect critical sections applied to -/// built-in types to prevent concurrent modification. This API is most useful -/// for user-defined types with full control over how the internal state for the -/// type is managed. +/// The caller must ensure the closure cannot implicitly release the critical +/// section. If a multithreaded program calls back into the Python interpreter +/// in a manner that would cause the critical section to be released, the +/// `PyMutex` will be released and the resource protected by the `PyMutex` may +/// be read from or modified by another thread. Concurrent modifications are +/// still impossible, but the state of the resource may change "underneath" a +/// suspended thread in possibly surprising ways. /// /// Only available on Python 3.14 and newer. #[cfg(Py_3_14)] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section_mutex2( - m1: &mut PyMutex, - m2: &mut PyMutex, +pub fn with_critical_section_mutex2<'py, F, R, T1, T2>( + _py: Python<'py>, + m1: &PyMutex, + m2: &PyMutex, f: F, ) -> R where @@ -650,13 +689,11 @@ where &mut *m2.mutex.get(), ) }; - let cell1 = unsafe { &*(m1.mutex.get() as *mut T1 as *const UnsafeCell) }; - let cell2 = unsafe { &*(m2.mutex.get() as *mut T2 as *const UnsafeCell) }; - f(&cell1, &cell2) + f(&m1.data, &m2.data) } #[cfg(not(Py_GIL_DISABLED))] { - f() + f(&m1.data, &m2.data) } } @@ -1267,12 +1304,12 @@ mod tests { let mutex = PyMutex::new(false); std::thread::scope(|s| { - s.spawn(|| unsafe { + s.spawn(|| { Python::attach(|py| { - with_critical_section_mutex(py, &mutex, |b: &UnsafeCell| { + with_critical_section_mutex(py, &mutex, |b| { barrier.wait(); std::thread::sleep(std::time::Duration::from_millis(10)); - (*b.get()) = true; + unsafe { (*b.get()) = true }; }); }); }); @@ -1280,11 +1317,9 @@ mod tests { Python::attach(|py| { barrier.wait(); // blocks until the other thread enters a critical section - unsafe { - with_critical_section_mutex(py, &mutex, |b: &UnsafeCell| { - assert!(*b.get() == true); - }); - } + with_critical_section_mutex(py, &mutex, |b| { + assert!(unsafe { *b.get() } == true); + }); }); }); }); @@ -1339,6 +1374,39 @@ mod tests { }); } + #[cfg(feature = "macros")] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section2_mutex() { + let barrier = Barrier::new(2); + + let m1 = PyMutex::new(false); + let m2 = PyMutex::new(false); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + with_critical_section_mutex2(py, &m1, &m2, |b1, b2| { + barrier.wait(); + std::thread::sleep(std::time::Duration::from_millis(10)); + unsafe { (*b1.get()) = true }; + unsafe { (*b2.get()) = true }; + }); + }); + }); + s.spawn(|| { + Python::attach(|py| { + barrier.wait(); + // blocks until the other thread enters a critical section + with_critical_section_mutex2(py, &m1, &m2, |b1, b2| { + assert!(unsafe { *b1.get() } == true); + assert!(unsafe { *b2.get() } == true); + }); + }); + }); + }); + } + #[cfg(feature = "macros")] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] From f88228dc2093d6a5d45e21761302216010751c2b Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 21 Nov 2025 13:47:41 -0700 Subject: [PATCH 05/32] fix limited API builds --- src/sync.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index ef8a7e46514..b3770747b70 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -9,10 +9,12 @@ //! interpreter. //! //! This module provides synchronization primitives which are able to synchronize under these conditions. +#[cfg(not(Py_LIMITED_API))] +use crate::types::PyMutex; use crate::{ internal::state::SuspendAttach, sealed::Sealed, - types::{any::PyAnyMethods, PyAny, PyMutex, PyString}, + types::{any::PyAnyMethods, PyAny, PyString}, Bound, Py, PyResult, PyTypeCheck, Python, }; use std::{ @@ -620,7 +622,7 @@ where /// suspended thread in possibly surprising ways. /// /// Only available on Python 3.14 and newer. -#[cfg(Py_3_14)] +#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] pub fn with_critical_section_mutex<'py, F, R, T>(_py: Python<'py>, mutex: &PyMutex, f: F) -> R where @@ -668,7 +670,7 @@ where /// suspended thread in possibly surprising ways. /// /// Only available on Python 3.14 and newer. -#[cfg(Py_3_14)] +#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] pub fn with_critical_section_mutex2<'py, F, R, T1, T2>( _py: Python<'py>, @@ -1261,7 +1263,6 @@ mod tests { }); } - #[cfg(feature = "macros")] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section() { @@ -1295,7 +1296,7 @@ mod tests { }); } - #[cfg(feature = "macros")] + #[cfg(all(not(Py_LIMITED_API), Py_3_14))] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section_mutex() { @@ -1374,10 +1375,10 @@ mod tests { }); } - #[cfg(feature = "macros")] + #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] - fn test_critical_section2_mutex() { + fn test_critical_section_mutex2() { let barrier = Barrier::new(2); let m1 = PyMutex::new(false); From 858e7ab9b591560c39c489958ee5f6193264a71b Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 10:46:23 -0700 Subject: [PATCH 06/32] fix deadlocks and abi3 compilation errors --- src/sync.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index b3770747b70..1f879d5d7a4 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -1174,13 +1174,11 @@ mod tests { use crate::types::{PyDict, PyDictMethods}; #[cfg(not(target_arch = "wasm32"))] - use std::sync::Mutex; - #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "macros")] - use std::sync::{ - atomic::{AtomicBool, Ordering}, - Barrier, - }; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::Barrier; + #[cfg(not(target_arch = "wasm32"))] + use std::sync::Mutex; #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "macros")] @@ -1263,6 +1261,7 @@ mod tests { }); } + #[cfg(feature = "macros")] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section() { @@ -1315,8 +1314,8 @@ mod tests { }); }); s.spawn(|| { + barrier.wait(); Python::attach(|py| { - barrier.wait(); // blocks until the other thread enters a critical section with_critical_section_mutex(py, &mutex, |b| { assert!(unsafe { *b.get() } == true); @@ -1396,8 +1395,8 @@ mod tests { }); }); s.spawn(|| { + barrier.wait(); Python::attach(|py| { - barrier.wait(); // blocks until the other thread enters a critical section with_critical_section_mutex2(py, &m1, &m2, |b1, b2| { assert!(unsafe { *b1.get() } == true); From 5f825b77919f657b4f4f36d80ffe3d77b9d71d52 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 10:47:25 -0700 Subject: [PATCH 07/32] remove unnecessary comparisons with true --- src/sync.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 1f879d5d7a4..e150248a8ca 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -1318,7 +1318,7 @@ mod tests { Python::attach(|py| { // blocks until the other thread enters a critical section with_critical_section_mutex(py, &mutex, |b| { - assert!(unsafe { *b.get() } == true); + assert!(unsafe { *b.get() }); }); }); }); @@ -1399,8 +1399,8 @@ mod tests { Python::attach(|py| { // blocks until the other thread enters a critical section with_critical_section_mutex2(py, &m1, &m2, |b1, b2| { - assert!(unsafe { *b1.get() } == true); - assert!(unsafe { *b2.get() } == true); + assert!(unsafe { *b1.get() }); + assert!(unsafe { *b2.get() }); }); }); }); From a05fa1242f5b0771376007214f09125775c9601d Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 12:45:20 -0700 Subject: [PATCH 08/32] add two more tests --- src/sync.rs | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/src/sync.rs b/src/sync.rs index e150248a8ca..4ff20e336ab 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -1441,6 +1441,36 @@ mod tests { }); } + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section_mutex2_same_object_no_deadlock() { + let barrier = Barrier::new(2); + + let m = PyMutex::new(false); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + with_critical_section_mutex2(py, &m, &m, |b1, b2| { + barrier.wait(); + std::thread::sleep(std::time::Duration::from_millis(10)); + unsafe { (*b1.get()) = true }; + assert!(unsafe { *b2.get() }); + }); + }); + }); + s.spawn(|| { + barrier.wait(); + Python::attach(|py| { + // this blocks until the other thread's critical section finishes + with_critical_section_mutex(py, &m, |b| { + assert!(unsafe { *b.get() }); + }); + }); + }); + }); + } + #[cfg(feature = "macros")] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] @@ -1496,6 +1526,53 @@ mod tests { }); } + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section_mutex2_two_containers() { + let (m1, m2) = (PyMutex::new(vec![1, 2, 3]), PyMutex::new(vec![4, 5])); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + with_critical_section_mutex2(py, &m1, &m2, |v1, v2| { + // v1.extend(v1) + let vec1 = unsafe { &mut *v1.get() }; + let vec2 = unsafe { &*v2.get() }; + vec1.extend(vec2.iter()); + }) + }); + }); + s.spawn(|| { + Python::attach(|py| { + with_critical_section_mutex2(py, &m1, &m2, |v1, v2| { + // v2.extend(v1) + let vec1 = unsafe { &*v1.get() }; + let vec2 = unsafe { &mut *v2.get() }; + vec2.extend(vec1.iter()); + }) + }); + }); + }); + + // execution order is not guaranteed, so we need to check both + // NB: extend should be atomic, items must not be interleaved + // v1.extend(v2) + // v2.extend(v1) + let expected1_vec1 = vec![1, 2, 3, 4, 5]; + let expected1_vec2 = vec![4, 5, 1, 2, 3, 4, 5]; + // v2.extend(v1) + // v1.extend(v2) + let expected2_vec1 = vec![1, 2, 3, 4, 5, 1, 2, 3]; + let expected2_vec2 = vec![4, 5, 1, 2, 3]; + + let v1 = m1.lock().unwrap(); + let v2 = m2.lock().unwrap(); + assert!( + ((*v1).eq(&expected1_vec1) && (*v2).eq(&expected1_vec2)) + || ((*v1).eq(&expected2_vec1) && (*v2).eq(&expected2_vec2)) + ); + } + #[test] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled fn test_once_ext() { From 53d1e2d652f8f18114ee5bd22e48d5dda5f3f962 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 12:47:40 -0700 Subject: [PATCH 09/32] add changelog entry --- newsfragments/5642.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/5642.added.md diff --git a/newsfragments/5642.added.md b/newsfragments/5642.added.md new file mode 100644 index 00000000000..8d7571ff206 --- /dev/null +++ b/newsfragments/5642.added.md @@ -0,0 +1 @@ +added an unsafe api wrapping the `Py_BEGIN_CRITICAL_SECTION_MUTEX` and `Py_BEGIN_CRITICAL_SECTION_MUTEX2` variants for the critical section API available in Python 3.14. \ No newline at end of file From f178d123b468d1de7bf7be78593a6a472b48e56f Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 12:58:33 -0700 Subject: [PATCH 10/32] add missing conditional compilation for new tests --- src/sync.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sync.rs b/src/sync.rs index 4ff20e336ab..4c3ad2b9b04 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -1441,6 +1441,7 @@ mod tests { }); } + #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section_mutex2_same_object_no_deadlock() { @@ -1526,6 +1527,7 @@ mod tests { }); } + #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section_mutex2_two_containers() { From 7de02eb6e27de89df341d501c07844852adb96b7 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 13:09:16 -0700 Subject: [PATCH 11/32] add missing conditional compilation for new tests --- src/sync.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sync.rs b/src/sync.rs index 4c3ad2b9b04..9a799af3e92 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -1176,6 +1176,8 @@ mod tests { #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "macros")] use std::sync::atomic::{AtomicBool, Ordering}; + #[cfg(not(target_arch = "wasm32"))] + #[cfg(any(feature = "macros", all(not(Py_LIMITED_API), Py_3_14)))] use std::sync::Barrier; #[cfg(not(target_arch = "wasm32"))] use std::sync::Mutex; From caae6d36e97ce35ec0b978f703fb85b4d27b5d93 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 13:36:33 -0700 Subject: [PATCH 12/32] fix conditional compilation in pyo3::sync --- src/sync.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sync.rs b/src/sync.rs index 9a799af3e92..6310074ac28 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -9,7 +9,7 @@ //! interpreter. //! //! This module provides synchronization primitives which are able to synchronize under these conditions. -#[cfg(not(Py_LIMITED_API))] +#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] use crate::types::PyMutex; use crate::{ internal::state::SuspendAttach, From 625be8deb59a7786e618a9e34a1136d3d4ea6ec0 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 13:52:04 -0700 Subject: [PATCH 13/32] fix Python 3.13 clippy --- pyo3-ffi/src/cpython/critical_section.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/pyo3-ffi/src/cpython/critical_section.rs b/pyo3-ffi/src/cpython/critical_section.rs index d63a1cd43a8..01bd186b7e9 100644 --- a/pyo3-ffi/src/cpython/critical_section.rs +++ b/pyo3-ffi/src/cpython/critical_section.rs @@ -1,3 +1,4 @@ +#[cfg(any(Py_3_14, Py_GIL_DISABLED))] use crate::PyMutex; use crate::PyObject; From 02e2f1dc0b9da47f11b52f2a5b99beacc5333d6c Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 16:39:45 -0700 Subject: [PATCH 14/32] Update newsfragments/5642.added.md --- newsfragments/5642.added.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/5642.added.md b/newsfragments/5642.added.md index 8d7571ff206..e61ebf53414 100644 --- a/newsfragments/5642.added.md +++ b/newsfragments/5642.added.md @@ -1 +1 @@ -added an unsafe api wrapping the `Py_BEGIN_CRITICAL_SECTION_MUTEX` and `Py_BEGIN_CRITICAL_SECTION_MUTEX2` variants for the critical section API available in Python 3.14. \ No newline at end of file +added FFI definitions and an unsafe Rust API wrapping `Py_BEGIN_CRITICAL_SECTION_MUTEX` and `Py_BEGIN_CRITICAL_SECTION_MUTEX2`. \ No newline at end of file From 34fdfd0c762a49aadfc5843260a8946781984d34 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 16:35:14 -0700 Subject: [PATCH 15/32] refactor so to use EnteredCriticalSection struct --- src/sync.rs | 111 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 86 insertions(+), 25 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 6310074ac28..ed7279e1d5e 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -486,6 +486,54 @@ impl Drop for CS2Guard { } } +/// Allows access to data protected by a PyMutex in a critical section +/// +/// Used with the `with_critical_section_mutex` and +/// `with_critical_section_mutex2` functions. See the documentation of those +/// functions for more details. +#[cfg(all(Py_3_14, Py_GIL_DISABLED))] +pub struct EnteredCriticalSection<'a, T>(&'a UnsafeCell); + +impl EnteredCriticalSection<'_, T> { + /// Get a mutable reference to the data wrapped by a PyMutex + /// + /// # Safety + /// + /// The caller must ensure the critical section is not released while the + /// reference is alive. If a multithreaded program calls back into the + /// Python interpreter in a manner that would cause the critical section to + /// be released, the `PyMutex` will be released and the resource protected + /// by the `PyMutex` may be read from or modified by another thread while + /// the critical section is suspended and the thread that owns the reference + /// is blocked. Concurrent modifications are impossible, but races are + /// possible and the state of an object may change "underneath" a suspended + /// thread in possibly surprising ways. Note that many operations on Python + /// objects may call back into the interpreter in a blocking manner because + /// many C API calls can trigger the execution of arbitrary Python code. + pub unsafe fn get_mut(&mut self) -> &mut T { + return unsafe { &mut *(self.0.get()) }; + } + + /// Get a immutable reference to the value wrapped by a PyMutex + /// + /// # Safety + /// + /// The caller must ensure the critical section is not released while the + /// reference is alive. If a multithreaded program calls back into the + /// Python interpreter in a manner that would cause the critical section to + /// be released, the `PyMutex` will be released and the resource protected + /// by the `PyMutex` may be read from or modified by another thread while + /// the critical section is suspended and the thread that owns the reference + /// is blocked. Concurrent modifications are impossible, but races are + /// possible and the state of an object may change "underneath" a suspended + /// thread in possibly surprising ways. Note that many operations on Python + /// objects may call back into the interpreter in a blocking manner because + /// many C API calls can trigger the execution of arbitrary Python code. + pub unsafe fn get(&self) -> &T { + return unsafe { &*(self.0.get()) }; + } +} + /// Executes a closure with a Python critical section held on an object. /// /// Acquires the per-object lock for the object `op` that is held @@ -618,25 +666,32 @@ where /// in a manner that would cause the critical section to be released, the /// `PyMutex` will be released and the resource protected by the `PyMutex` may /// be read from or modified by another thread. Concurrent modifications are -/// still impossible, but the state of the resource may change "underneath" a -/// suspended thread in possibly surprising ways. +/// impossible, but races are possible and the state of an object may change +/// "underneath" a suspended thread in possibly surprising ways. Note that many +/// operations on Python objects may call back into the interpreter in a +/// blocking manner because many C API calls can trigger the execution of +/// arbitrary Python code. /// /// Only available on Python 3.14 and newer. #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section_mutex<'py, F, R, T>(_py: Python<'py>, mutex: &PyMutex, f: F) -> R +pub fn with_critical_section_mutex<'py, 'a, F, R, T>( + _py: Python<'py>, + mutex: &'a PyMutex, + f: F, +) -> R where - F: FnOnce(&UnsafeCell) -> R, + F: FnOnce(EnteredCriticalSection<'a, T>) -> R, { #[cfg(Py_GIL_DISABLED)] { let mut guard = CSGuard(unsafe { std::mem::zeroed() }); unsafe { crate::ffi::PyCriticalSection_BeginMutex(&mut guard.0, &mut *mutex.mutex.get()) }; - f(&mutex.data) + f(EnteredCriticalSection(&mutex.data)) } #[cfg(not(Py_GIL_DISABLED))] { - f(&mutex.data) + f(EnteredCriticalSection(&mutex.data)) } } @@ -672,14 +727,14 @@ where /// Only available on Python 3.14 and newer. #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section_mutex2<'py, F, R, T1, T2>( +pub fn with_critical_section_mutex2<'py, 'a, F, R, T1, T2>( _py: Python<'py>, - m1: &PyMutex, - m2: &PyMutex, + m1: &'a PyMutex, + m2: &'a PyMutex, f: F, ) -> R where - F: FnOnce(&UnsafeCell, &UnsafeCell) -> R, + F: FnOnce(EnteredCriticalSection<'a, T1>, EnteredCriticalSection<'a, T2>) -> R, { #[cfg(Py_GIL_DISABLED)] { @@ -691,11 +746,17 @@ where &mut *m2.mutex.get(), ) }; - f(&m1.data, &m2.data) + f( + EnteredCriticalSection(&m1.data), + EnteredCriticalSection(&m2.data), + ) } #[cfg(not(Py_GIL_DISABLED))] { - f(&m1.data, &m2.data) + f( + EnteredCriticalSection(&m1.data), + EnteredCriticalSection(&m2.data), + ) } } @@ -1308,10 +1369,10 @@ mod tests { std::thread::scope(|s| { s.spawn(|| { Python::attach(|py| { - with_critical_section_mutex(py, &mutex, |b| { + with_critical_section_mutex(py, &mutex, |mut b| { barrier.wait(); std::thread::sleep(std::time::Duration::from_millis(10)); - unsafe { (*b.get()) = true }; + *(unsafe { b.get_mut() }) = true; }); }); }); @@ -1388,11 +1449,11 @@ mod tests { std::thread::scope(|s| { s.spawn(|| { Python::attach(|py| { - with_critical_section_mutex2(py, &m1, &m2, |b1, b2| { + with_critical_section_mutex2(py, &m1, &m2, |mut b1, mut b2| { barrier.wait(); std::thread::sleep(std::time::Duration::from_millis(10)); - unsafe { (*b1.get()) = true }; - unsafe { (*b2.get()) = true }; + unsafe { (*b1.get_mut()) = true }; + unsafe { (*b2.get_mut()) = true }; }); }); }); @@ -1454,10 +1515,10 @@ mod tests { std::thread::scope(|s| { s.spawn(|| { Python::attach(|py| { - with_critical_section_mutex2(py, &m, &m, |b1, b2| { + with_critical_section_mutex2(py, &m, &m, |mut b1, b2| { barrier.wait(); std::thread::sleep(std::time::Duration::from_millis(10)); - unsafe { (*b1.get()) = true }; + unsafe { (*b1.get_mut()) = true }; assert!(unsafe { *b2.get() }); }); }); @@ -1538,20 +1599,20 @@ mod tests { std::thread::scope(|s| { s.spawn(|| { Python::attach(|py| { - with_critical_section_mutex2(py, &m1, &m2, |v1, v2| { + with_critical_section_mutex2(py, &m1, &m2, |mut v1, v2| { // v1.extend(v1) - let vec1 = unsafe { &mut *v1.get() }; - let vec2 = unsafe { &*v2.get() }; + let vec1 = unsafe { v1.get_mut() }; + let vec2 = unsafe { v2.get() }; vec1.extend(vec2.iter()); }) }); }); s.spawn(|| { Python::attach(|py| { - with_critical_section_mutex2(py, &m1, &m2, |v1, v2| { + with_critical_section_mutex2(py, &m1, &m2, |v1, mut v2| { // v2.extend(v1) - let vec1 = unsafe { &*v1.get() }; - let vec2 = unsafe { &mut *v2.get() }; + let vec1 = unsafe { v1.get() }; + let vec2 = unsafe { v2.get_mut() }; vec2.extend(vec1.iter()); }) }); From 2294f755780b1dde331a803de4eafeb1bc26e90d Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 16:35:18 -0700 Subject: [PATCH 16/32] tweak docstrings --- src/sync.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index ed7279e1d5e..708e3f593b0 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -557,11 +557,11 @@ impl EnteredCriticalSection<'_, T> { /// section. If a multithreaded program calls back into the Python interpreter /// in a manner that would cause the critical section to be released, the /// per-object lock will be released and the state of the object may be read -/// from or modified by another thread. Concurrent modifications are still -/// impossible, but the state of object may change "underneath" a suspended -/// thread in possibly surprising ways. Note that many operations on Python -/// objects may call back into the interpreter in a blocking manner because -/// many C API calls can trigger the execution of arbitrary Python code. +/// from or modified by another thread. Concurrent modifications are impossible, +/// but races are possible and the state of an object may change "underneath" a +/// suspended thread in possibly surprising ways. Note that many operations on +/// Python objects may call back into the interpreter in a blocking manner +/// because many C API calls can trigger the execution of arbitrary Python code. /// /// Many CPython C API functions do not acquire the per-object lock on objects /// passed to Python. You should not expect critical sections applied to @@ -608,11 +608,11 @@ where /// section. If a multithreaded program calls back into the Python interpreter /// in a manner that would cause the critical section to be released, the /// per-object lock will be released and the state of the object may be read -/// from or modified by another thread. Concurrent modifications are still -/// impossible, but the state of object may change "underneath" a suspended -/// thread in possibly surprising ways. Note that many operations on Python -/// objects may call back into the interpreter in a blocking manner because -/// many C API calls can trigger the execution of arbitrary Python code. +/// from or modified by another thread. Concurrent modifications are impossible, +/// but races are possible and the state of an object may change "underneath" a +/// suspended thread in possibly surprising ways. Note that many operations on +/// Python objects may call back into the interpreter in a blocking manner +/// because many C API calls can trigger the execution of arbitrary Python code. /// /// Many CPython C API functions do not acquire the per-object lock on objects /// passed to Python. You should not expect critical sections applied to @@ -721,8 +721,11 @@ where /// in a manner that would cause the critical section to be released, the /// `PyMutex` will be released and the resource protected by the `PyMutex` may /// be read from or modified by another thread. Concurrent modifications are -/// still impossible, but the state of the resource may change "underneath" a -/// suspended thread in possibly surprising ways. +/// impossible, but races are possible and the state of an object may change +/// "underneath" a suspended thread in possibly surprising ways. Note that many +/// operations on Python objects may call back into the interpreter in a +/// blocking manner because many C API calls can trigger the execution of +/// arbitrary Python code. /// /// Only available on Python 3.14 and newer. #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] From 084f6669bbd9ee7b10df32b56bd350b224bbada9 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 16:39:18 -0700 Subject: [PATCH 17/32] Add SAFETY comments in tests --- src/sync.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/sync.rs b/src/sync.rs index 708e3f593b0..d7c52f39360 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -1375,6 +1375,7 @@ mod tests { with_critical_section_mutex(py, &mutex, |mut b| { barrier.wait(); std::thread::sleep(std::time::Duration::from_millis(10)); + // SAFETY: we never call back into the python interpreter inside this critical section *(unsafe { b.get_mut() }) = true; }); }); @@ -1384,6 +1385,7 @@ mod tests { Python::attach(|py| { // blocks until the other thread enters a critical section with_critical_section_mutex(py, &mutex, |b| { + // SAFETY: we never call back into the python interpreter inside this critical section assert!(unsafe { *b.get() }); }); }); @@ -1455,6 +1457,7 @@ mod tests { with_critical_section_mutex2(py, &m1, &m2, |mut b1, mut b2| { barrier.wait(); std::thread::sleep(std::time::Duration::from_millis(10)); + // SAFETY: we never call back into the python interpreter inside this critical section unsafe { (*b1.get_mut()) = true }; unsafe { (*b2.get_mut()) = true }; }); @@ -1465,6 +1468,7 @@ mod tests { Python::attach(|py| { // blocks until the other thread enters a critical section with_critical_section_mutex2(py, &m1, &m2, |b1, b2| { + // SAFETY: we never call back into the python interpreter inside this critical section assert!(unsafe { *b1.get() }); assert!(unsafe { *b2.get() }); }); @@ -1521,6 +1525,7 @@ mod tests { with_critical_section_mutex2(py, &m, &m, |mut b1, b2| { barrier.wait(); std::thread::sleep(std::time::Duration::from_millis(10)); + // SAFETY: we never call back into the python interpreter inside this critical section unsafe { (*b1.get_mut()) = true }; assert!(unsafe { *b2.get() }); }); @@ -1531,6 +1536,7 @@ mod tests { Python::attach(|py| { // this blocks until the other thread's critical section finishes with_critical_section_mutex(py, &m, |b| { + // SAFETY: we never call back into the python interpreter inside this critical section assert!(unsafe { *b.get() }); }); }); @@ -1604,6 +1610,7 @@ mod tests { Python::attach(|py| { with_critical_section_mutex2(py, &m1, &m2, |mut v1, v2| { // v1.extend(v1) + // SAFETY: we never call back into the python interpreter inside this critical section let vec1 = unsafe { v1.get_mut() }; let vec2 = unsafe { v2.get() }; vec1.extend(vec2.iter()); @@ -1614,6 +1621,7 @@ mod tests { Python::attach(|py| { with_critical_section_mutex2(py, &m1, &m2, |v1, mut v2| { // v2.extend(v1) + // SAFETY: we never call back into the python interpreter inside this critical section let vec1 = unsafe { v1.get() }; let vec2 = unsafe { v2.get_mut() }; vec2.extend(vec1.iter()); From 88794fe4010e5b9a3842ed80e9870d351ed742f5 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 16:41:48 -0700 Subject: [PATCH 18/32] fix include guards --- src/sync.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sync.rs b/src/sync.rs index d7c52f39360..800bca3530d 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -491,9 +491,10 @@ impl Drop for CS2Guard { /// Used with the `with_critical_section_mutex` and /// `with_critical_section_mutex2` functions. See the documentation of those /// functions for more details. -#[cfg(all(Py_3_14, Py_GIL_DISABLED))] +#[cfg(any(Py_3_14, Py_GIL_DISABLED))] pub struct EnteredCriticalSection<'a, T>(&'a UnsafeCell); +#[cfg(any(Py_3_14, Py_GIL_DISABLED))] impl EnteredCriticalSection<'_, T> { /// Get a mutable reference to the data wrapped by a PyMutex /// From 4a22503bb421a7a84d0dde88e5713deb4e9427de Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 24 Nov 2025 16:44:59 -0700 Subject: [PATCH 19/32] remove unnecessary returns --- src/sync.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 800bca3530d..c0175427683 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -512,7 +512,7 @@ impl EnteredCriticalSection<'_, T> { /// objects may call back into the interpreter in a blocking manner because /// many C API calls can trigger the execution of arbitrary Python code. pub unsafe fn get_mut(&mut self) -> &mut T { - return unsafe { &mut *(self.0.get()) }; + unsafe { &mut *(self.0.get()) } } /// Get a immutable reference to the value wrapped by a PyMutex @@ -531,7 +531,7 @@ impl EnteredCriticalSection<'_, T> { /// objects may call back into the interpreter in a blocking manner because /// many C API calls can trigger the execution of arbitrary Python code. pub unsafe fn get(&self) -> &T { - return unsafe { &*(self.0.get()) }; + unsafe { &*(self.0.get()) } } } From 9b2a17f33d297c6665709d5607c8cd92521336a4 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Wed, 3 Dec 2025 09:59:38 -0700 Subject: [PATCH 20/32] fix whitespace formatting --- src/sync.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index c0175427683..50dfd6201c7 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -540,7 +540,7 @@ impl EnteredCriticalSection<'_, T> { /// Acquires the per-object lock for the object `op` that is held /// while the closure `f` is executing. The critical section may be temporarily /// released and re-acquired if the closure calls back into the interpreter in -/// a manner that would block. This is similar to how the GIL can be released +/// a manner that would block. This is similar to how the GIL can be released /// during blocking calls. See the safety notes below for caveats about /// releasing critical sections. /// @@ -591,7 +591,7 @@ where /// Acquires the per-object lock for the objects `a` and `b` that are held /// while the closure `f` is executing. The critical section may be temporarily /// released and re-acquired if the closure calls back into the interpreter in -/// a manner that would block. This is similar to how the GIL can be released +/// a manner that would block. This is similar to how the GIL can be released /// during blocking calls. See the safety notes below for caveats about /// releasing critical sections. /// @@ -641,7 +641,7 @@ where /// /// Acquires the mutex `mutex` until the closure `f` finishes. The mutex may be /// temporarily released and re-acquired if the closure calls back into the -/// interpreter in a manner that would block. This is similar to how the GIL +/// interpreter in a manner that would block. This is similar to how the GIL /// can be released during blocking calls. See the safety notes below for /// caveats about releasing critical sections. /// @@ -701,7 +701,7 @@ where /// Simultaneously acquires the mutexes `m1` and `m2` and holds them /// until the closure `f` is finished. The mutexes may be /// temporarily released and re-acquired if the closure calls back into the -/// interpreter in a manner that would block. This is similar to how the GIL +/// interpreter in a manner that would block. This is similar to how the GIL /// can be released during blocking calls. See the safety notes below for /// caveats about releasing critical sections. /// From d5e7403458bad82c0855c85de9a05fa2c179f760 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Wed, 3 Dec 2025 10:25:20 -0700 Subject: [PATCH 21/32] Move note about minimum Python version --- src/sync.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 50dfd6201c7..d2355659b34 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -656,6 +656,8 @@ where /// analogous manner to the GIL without introducing any deadlock risks or /// affecting runtime behavior on the GIL-enabled build. /// +/// Only available on Python 3.14 and newer. +/// /// # Safety /// /// Provides weaker locking guarantees than traditional locks, but can in some @@ -672,8 +674,6 @@ where /// operations on Python objects may call back into the interpreter in a /// blocking manner because many C API calls can trigger the execution of /// arbitrary Python code. -/// -/// Only available on Python 3.14 and newer. #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] pub fn with_critical_section_mutex<'py, 'a, F, R, T>( @@ -711,6 +711,8 @@ where /// A no-op on GIL-enabled builds, where the critical section API is exposed as /// a no-op by the Python C API. /// +/// Only available on Python 3.14 and newer. +/// /// # Safety /// /// Provides weaker locking guarantees than traditional locks, but can in some @@ -727,8 +729,6 @@ where /// operations on Python objects may call back into the interpreter in a /// blocking manner because many C API calls can trigger the execution of /// arbitrary Python code. -/// -/// Only available on Python 3.14 and newer. #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] pub fn with_critical_section_mutex2<'py, 'a, F, R, T1, T2>( From 715faf95f18bc009516f8b55a83504212ead5316 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Wed, 3 Dec 2025 10:25:49 -0700 Subject: [PATCH 22/32] Fix reference leak by adjusting lifetimes --- src/sync.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index d2355659b34..fd050fe01bb 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -676,13 +676,9 @@ where /// arbitrary Python code. #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section_mutex<'py, 'a, F, R, T>( - _py: Python<'py>, - mutex: &'a PyMutex, - f: F, -) -> R +pub fn with_critical_section_mutex(_py: Python<'_>, mutex: &PyMutex, f: F) -> R where - F: FnOnce(EnteredCriticalSection<'a, T>) -> R, + F: for<'s> FnOnce(EnteredCriticalSection<'s, T>) -> R, { #[cfg(Py_GIL_DISABLED)] { @@ -731,14 +727,14 @@ where /// arbitrary Python code. #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] #[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section_mutex2<'py, 'a, F, R, T1, T2>( - _py: Python<'py>, - m1: &'a PyMutex, - m2: &'a PyMutex, +pub fn with_critical_section_mutex2( + _py: Python<'_>, + m1: &PyMutex, + m2: &PyMutex, f: F, ) -> R where - F: FnOnce(EnteredCriticalSection<'a, T1>, EnteredCriticalSection<'a, T2>) -> R, + F: for<'s> FnOnce(EnteredCriticalSection<'s, T1>, EnteredCriticalSection<'s, T2>) -> R, { #[cfg(Py_GIL_DISABLED)] { From aeb363182098193b282110b5a28cb3ac751e3a50 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Wed, 3 Dec 2025 10:47:19 -0700 Subject: [PATCH 23/32] Apply suggestions for test tweaks from code review --- src/sync.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index fd050fe01bb..6774671ce7a 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -1602,6 +1602,8 @@ mod tests { fn test_critical_section_mutex2_two_containers() { let (m1, m2) = (PyMutex::new(vec![1, 2, 3]), PyMutex::new(vec![4, 5])); + let (m1_guard, m2_guard) = (m1.lock().unwrap(), m2.lock().unwrap()); + std::thread::scope(|s| { s.spawn(|| { Python::attach(|py| { @@ -1625,6 +1627,18 @@ mod tests { }) }); }); + // the other threads waiting for locks should not block this attach + Python::attach(|_| { + // On the free-threaded build, the critical sections should have blocked + // the other threads from modification. + #[cfg(Py_GIL_DISABLED)] + { + assert_eq!(&*m1_guard, &[1, 2, 3]); + assert_eq!(&*m2_guard, &[4, 5]); + } + }); + drop(m1_guard); + drop(m2_guard); }); // execution order is not guaranteed, so we need to check both @@ -1641,8 +1655,8 @@ mod tests { let v1 = m1.lock().unwrap(); let v2 = m2.lock().unwrap(); assert!( - ((*v1).eq(&expected1_vec1) && (*v2).eq(&expected1_vec2)) - || ((*v1).eq(&expected2_vec1) && (*v2).eq(&expected2_vec2)) + (&*v1, &*v2) == (&expected1_vec1, &expected1_vec2) + || (&*v1, &*v2) == (&expected2_vec1, &expected2_vec2) ); } From 47c3ad6ee993b122ba477485a984d1cca0d96466 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Wed, 3 Dec 2025 16:23:17 -0700 Subject: [PATCH 24/32] refactor into a new module and rewrite docs --- src/conversions/std/num.rs | 10 +- src/sync.rs | 674 +---------------------------------- src/sync/critical_section.rs | 625 ++++++++++++++++++++++++++++++++ src/types/bytearray.rs | 2 +- src/types/code.rs | 2 +- src/types/dict.rs | 6 +- src/types/list.rs | 6 +- 7 files changed, 659 insertions(+), 666 deletions(-) create mode 100644 src/sync/critical_section.rs diff --git a/src/conversions/std/num.rs b/src/conversions/std/num.rs index f20f6b18dcc..e62aa1c6685 100644 --- a/src/conversions/std/num.rs +++ b/src/conversions/std/num.rs @@ -342,10 +342,12 @@ impl BytesSequenceExtractor<'_, '_> { match self { BytesSequenceExtractor::Bytes(b) => copy_slice(b.as_bytes()), - BytesSequenceExtractor::ByteArray(b) => crate::sync::with_critical_section(b, || { - // Safety: b is protected by a critical section - copy_slice(unsafe { b.as_bytes() }) - }), + BytesSequenceExtractor::ByteArray(b) => { + crate::sync::critical_section::with_critical_section(b, || { + // Safety: b is protected by a critical section + copy_slice(unsafe { b.as_bytes() }) + }) + } } } } diff --git a/src/sync.rs b/src/sync.rs index 6774671ce7a..203390d50fa 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -9,12 +9,10 @@ //! interpreter. //! //! This module provides synchronization primitives which are able to synchronize under these conditions. -#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] -use crate::types::PyMutex; use crate::{ internal::state::SuspendAttach, sealed::Sealed, - types::{any::PyAnyMethods, PyAny, PyString}, + types::{any::PyAnyMethods, PyString}, Bound, Py, PyResult, PyTypeCheck, Python, }; use std::{ @@ -24,11 +22,14 @@ use std::{ sync::{Once, OnceState}, }; +pub mod critical_section; pub(crate) mod once_lock; #[cfg(not(Py_GIL_DISABLED))] use crate::PyVisit; +#[deprecated(since = "0.28.0", note = "use std::sync::critical_section instead")] +pub use self::critical_section::{with_critical_section, with_critical_section2}; pub use self::once_lock::PyOnceLock; /// Value with concurrent access protected by the GIL. @@ -462,318 +463,6 @@ impl Interned { } } -#[cfg(Py_GIL_DISABLED)] -struct CSGuard(crate::ffi::PyCriticalSection); - -#[cfg(Py_GIL_DISABLED)] -impl Drop for CSGuard { - fn drop(&mut self) { - unsafe { - crate::ffi::PyCriticalSection_End(&mut self.0); - } - } -} - -#[cfg(Py_GIL_DISABLED)] -struct CS2Guard(crate::ffi::PyCriticalSection2); - -#[cfg(Py_GIL_DISABLED)] -impl Drop for CS2Guard { - fn drop(&mut self) { - unsafe { - crate::ffi::PyCriticalSection2_End(&mut self.0); - } - } -} - -/// Allows access to data protected by a PyMutex in a critical section -/// -/// Used with the `with_critical_section_mutex` and -/// `with_critical_section_mutex2` functions. See the documentation of those -/// functions for more details. -#[cfg(any(Py_3_14, Py_GIL_DISABLED))] -pub struct EnteredCriticalSection<'a, T>(&'a UnsafeCell); - -#[cfg(any(Py_3_14, Py_GIL_DISABLED))] -impl EnteredCriticalSection<'_, T> { - /// Get a mutable reference to the data wrapped by a PyMutex - /// - /// # Safety - /// - /// The caller must ensure the critical section is not released while the - /// reference is alive. If a multithreaded program calls back into the - /// Python interpreter in a manner that would cause the critical section to - /// be released, the `PyMutex` will be released and the resource protected - /// by the `PyMutex` may be read from or modified by another thread while - /// the critical section is suspended and the thread that owns the reference - /// is blocked. Concurrent modifications are impossible, but races are - /// possible and the state of an object may change "underneath" a suspended - /// thread in possibly surprising ways. Note that many operations on Python - /// objects may call back into the interpreter in a blocking manner because - /// many C API calls can trigger the execution of arbitrary Python code. - pub unsafe fn get_mut(&mut self) -> &mut T { - unsafe { &mut *(self.0.get()) } - } - - /// Get a immutable reference to the value wrapped by a PyMutex - /// - /// # Safety - /// - /// The caller must ensure the critical section is not released while the - /// reference is alive. If a multithreaded program calls back into the - /// Python interpreter in a manner that would cause the critical section to - /// be released, the `PyMutex` will be released and the resource protected - /// by the `PyMutex` may be read from or modified by another thread while - /// the critical section is suspended and the thread that owns the reference - /// is blocked. Concurrent modifications are impossible, but races are - /// possible and the state of an object may change "underneath" a suspended - /// thread in possibly surprising ways. Note that many operations on Python - /// objects may call back into the interpreter in a blocking manner because - /// many C API calls can trigger the execution of arbitrary Python code. - pub unsafe fn get(&self) -> &T { - unsafe { &*(self.0.get()) } - } -} - -/// Executes a closure with a Python critical section held on an object. -/// -/// Acquires the per-object lock for the object `op` that is held -/// while the closure `f` is executing. The critical section may be temporarily -/// released and re-acquired if the closure calls back into the interpreter in -/// a manner that would block. This is similar to how the GIL can be released -/// during blocking calls. See the safety notes below for caveats about -/// releasing critical sections. -/// -/// This is structurally equivalent to the use of the paired -/// Py_BEGIN_CRITICAL_SECTION and Py_END_CRITICAL_SECTION C-API macros. -/// -/// A no-op on GIL-enabled builds, where the critical section API is exposed as -/// a no-op by the Python C API. -/// -/// Provides weaker locking guarantees than traditional locks, but can in some -/// cases be used to provide guarantees similar to the GIL without the risk of -/// deadlocks associated with traditional locks. -/// -/// The caller must ensure the closure cannot implicitly release the critical -/// section. If a multithreaded program calls back into the Python interpreter -/// in a manner that would cause the critical section to be released, the -/// per-object lock will be released and the state of the object may be read -/// from or modified by another thread. Concurrent modifications are impossible, -/// but races are possible and the state of an object may change "underneath" a -/// suspended thread in possibly surprising ways. Note that many operations on -/// Python objects may call back into the interpreter in a blocking manner -/// because many C API calls can trigger the execution of arbitrary Python code. -/// -/// Many CPython C API functions do not acquire the per-object lock on objects -/// passed to Python. You should not expect critical sections applied to -/// built-in types to prevent concurrent modification. This API is most useful -/// for user-defined types with full control over how the internal state for the -/// type is managed. -#[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section(object: &Bound<'_, PyAny>, f: F) -> R -where - F: FnOnce() -> R, -{ - #[cfg(Py_GIL_DISABLED)] - { - let mut guard = CSGuard(unsafe { std::mem::zeroed() }); - unsafe { crate::ffi::PyCriticalSection_Begin(&mut guard.0, object.as_ptr()) }; - f() - } - #[cfg(not(Py_GIL_DISABLED))] - { - f() - } -} - -/// Executes a closure with a Python critical section held on two objects. -/// -/// Acquires the per-object lock for the objects `a` and `b` that are held -/// while the closure `f` is executing. The critical section may be temporarily -/// released and re-acquired if the closure calls back into the interpreter in -/// a manner that would block. This is similar to how the GIL can be released -/// during blocking calls. See the safety notes below for caveats about -/// releasing critical sections. -/// -/// This is structurally equivalent to the use of the paired -/// Py_BEGIN_CRITICAL_SECTION2 and Py_END_CRITICAL_SECTION2 C-API macros. -/// -/// A no-op on GIL-enabled builds, where the critical section API is exposed as -/// a no-op by the Python C API. -/// -/// Provides weaker locking guarantees than traditional locks, but can in some -/// cases be used to provide guarantees similar to the GIL without the risk of -/// deadlocks associated with traditional locks. -/// -/// The caller must ensure the closure cannot implicitly release the critical -/// section. If a multithreaded program calls back into the Python interpreter -/// in a manner that would cause the critical section to be released, the -/// per-object lock will be released and the state of the object may be read -/// from or modified by another thread. Concurrent modifications are impossible, -/// but races are possible and the state of an object may change "underneath" a -/// suspended thread in possibly surprising ways. Note that many operations on -/// Python objects may call back into the interpreter in a blocking manner -/// because many C API calls can trigger the execution of arbitrary Python code. -/// -/// Many CPython C API functions do not acquire the per-object lock on objects -/// passed to Python. You should not expect critical sections applied to -/// built-in types to prevent concurrent modification. This API is most useful -/// for user-defined types with full control over how the internal state for the -/// type is managed. -#[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section2(a: &Bound<'_, PyAny>, b: &Bound<'_, PyAny>, f: F) -> R -where - F: FnOnce() -> R, -{ - #[cfg(Py_GIL_DISABLED)] - { - let mut guard = CS2Guard(unsafe { std::mem::zeroed() }); - unsafe { crate::ffi::PyCriticalSection2_Begin(&mut guard.0, a.as_ptr(), b.as_ptr()) }; - f() - } - #[cfg(not(Py_GIL_DISABLED))] - { - f() - } -} - -/// Executes a closure with a Python critical section held on a `PyMutex`. -/// -/// Acquires the mutex `mutex` until the closure `f` finishes. The mutex may be -/// temporarily released and re-acquired if the closure calls back into the -/// interpreter in a manner that would block. This is similar to how the GIL -/// can be released during blocking calls. See the safety notes below for -/// caveats about releasing critical sections. -/// -/// This is structurally equivalent to the use of the paired -/// Py_BEGIN_CRITICAL_SECTION_MUTEX and Py_END_CRITICAL_SECTION C-API macros. -/// -/// A no-op on GIL-enabled builds, where the critical section API is exposed as -/// a no-op by the Python C API. -/// -/// This variant is particularly useful when paired with a global `PyMutex` to -/// create a "local GIL" to protect global state in an extension in an -/// analogous manner to the GIL without introducing any deadlock risks or -/// affecting runtime behavior on the GIL-enabled build. -/// -/// Only available on Python 3.14 and newer. -/// -/// # Safety -/// -/// Provides weaker locking guarantees than traditional locks, but can in some -/// cases be used to provide guarantees similar to the GIL without the risk of -/// deadlocks associated with traditional locks. -/// -/// The caller must ensure the closure cannot implicitly release the critical -/// section. If a multithreaded program calls back into the Python interpreter -/// in a manner that would cause the critical section to be released, the -/// `PyMutex` will be released and the resource protected by the `PyMutex` may -/// be read from or modified by another thread. Concurrent modifications are -/// impossible, but races are possible and the state of an object may change -/// "underneath" a suspended thread in possibly surprising ways. Note that many -/// operations on Python objects may call back into the interpreter in a -/// blocking manner because many C API calls can trigger the execution of -/// arbitrary Python code. -#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] -#[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section_mutex(_py: Python<'_>, mutex: &PyMutex, f: F) -> R -where - F: for<'s> FnOnce(EnteredCriticalSection<'s, T>) -> R, -{ - #[cfg(Py_GIL_DISABLED)] - { - let mut guard = CSGuard(unsafe { std::mem::zeroed() }); - unsafe { crate::ffi::PyCriticalSection_BeginMutex(&mut guard.0, &mut *mutex.mutex.get()) }; - f(EnteredCriticalSection(&mutex.data)) - } - #[cfg(not(Py_GIL_DISABLED))] - { - f(EnteredCriticalSection(&mutex.data)) - } -} - -/// Executes a closure with a Python critical section held on two `PyMutex` instances. -/// -/// Simultaneously acquires the mutexes `m1` and `m2` and holds them -/// until the closure `f` is finished. The mutexes may be -/// temporarily released and re-acquired if the closure calls back into the -/// interpreter in a manner that would block. This is similar to how the GIL -/// can be released during blocking calls. See the safety notes below for -/// caveats about releasing critical sections. -/// -/// This is structurally equivalent to the use of the paired -/// Py_BEGIN_CRITICAL_SECTION2_MUTEX and Py_END_CRITICAL_SECTION2 C-API macros. -/// -/// A no-op on GIL-enabled builds, where the critical section API is exposed as -/// a no-op by the Python C API. -/// -/// Only available on Python 3.14 and newer. -/// -/// # Safety -/// -/// Provides weaker locking guarantees than traditional locks, but can in some -/// cases be used to provide guarantees similar to the GIL without the risk of -/// deadlocks associated with traditional locks. -/// -/// The caller must ensure the closure cannot implicitly release the critical -/// section. If a multithreaded program calls back into the Python interpreter -/// in a manner that would cause the critical section to be released, the -/// `PyMutex` will be released and the resource protected by the `PyMutex` may -/// be read from or modified by another thread. Concurrent modifications are -/// impossible, but races are possible and the state of an object may change -/// "underneath" a suspended thread in possibly surprising ways. Note that many -/// operations on Python objects may call back into the interpreter in a -/// blocking manner because many C API calls can trigger the execution of -/// arbitrary Python code. -#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] -#[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] -pub fn with_critical_section_mutex2( - _py: Python<'_>, - m1: &PyMutex, - m2: &PyMutex, - f: F, -) -> R -where - F: for<'s> FnOnce(EnteredCriticalSection<'s, T1>, EnteredCriticalSection<'s, T2>) -> R, -{ - #[cfg(Py_GIL_DISABLED)] - { - let mut guard = CS2Guard(unsafe { std::mem::zeroed() }); - unsafe { - crate::ffi::PyCriticalSection2_BeginMutex( - &mut guard.0, - &mut *m1.mutex.get(), - &mut *m2.mutex.get(), - ) - }; - f( - EnteredCriticalSection(&m1.data), - EnteredCriticalSection(&m2.data), - ) - } - #[cfg(not(Py_GIL_DISABLED))] - { - f( - EnteredCriticalSection(&m1.data), - EnteredCriticalSection(&m2.data), - ) - } -} - -mod once_lock_ext_sealed { - pub trait Sealed {} - impl Sealed for std::sync::OnceLock {} -} - -mod rwlock_ext_sealed { - pub trait Sealed {} - impl Sealed for std::sync::RwLock {} - #[cfg(feature = "lock_api")] - impl Sealed for lock_api::RwLock {} - #[cfg(feature = "arc_lock")] - impl Sealed for std::sync::Arc> {} -} - /// Extension trait for [`Once`] to help avoid deadlocking when using a [`Once`] when attached to a /// Python thread. pub trait OnceExt: Sealed { @@ -1229,6 +918,20 @@ where value } +mod once_lock_ext_sealed { + pub trait Sealed {} + impl Sealed for std::sync::OnceLock {} +} + +mod rwlock_ext_sealed { + pub trait Sealed {} + impl Sealed for std::sync::RwLock {} + #[cfg(feature = "lock_api")] + impl Sealed for lock_api::RwLock {} + #[cfg(feature = "arc_lock")] + impl Sealed for std::sync::Arc> {} +} + #[cfg(test)] mod tests { use super::*; @@ -1248,11 +951,6 @@ mod tests { #[crate::pyclass(crate = "crate")] struct BoolWrapper(AtomicBool); - #[cfg(not(target_arch = "wasm32"))] - #[cfg(feature = "macros")] - #[crate::pyclass(crate = "crate")] - struct VecWrapper(Vec); - #[test] fn test_intern() { Python::attach(|py| { @@ -1324,342 +1022,6 @@ mod tests { }); } - #[cfg(feature = "macros")] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled - #[test] - fn test_critical_section() { - let barrier = Barrier::new(2); - - let bool_wrapper = Python::attach(|py| -> Py { - Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap() - }); - - std::thread::scope(|s| { - s.spawn(|| { - Python::attach(|py| { - let b = bool_wrapper.bind(py); - with_critical_section(b, || { - barrier.wait(); - std::thread::sleep(std::time::Duration::from_millis(10)); - b.borrow().0.store(true, Ordering::Release); - }) - }); - }); - s.spawn(|| { - barrier.wait(); - Python::attach(|py| { - let b = bool_wrapper.bind(py); - // this blocks until the other thread's critical section finishes - with_critical_section(b, || { - assert!(b.borrow().0.load(Ordering::Acquire)); - }); - }); - }); - }); - } - - #[cfg(all(not(Py_LIMITED_API), Py_3_14))] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled - #[test] - fn test_critical_section_mutex() { - let barrier = Barrier::new(2); - - let mutex = PyMutex::new(false); - - std::thread::scope(|s| { - s.spawn(|| { - Python::attach(|py| { - with_critical_section_mutex(py, &mutex, |mut b| { - barrier.wait(); - std::thread::sleep(std::time::Duration::from_millis(10)); - // SAFETY: we never call back into the python interpreter inside this critical section - *(unsafe { b.get_mut() }) = true; - }); - }); - }); - s.spawn(|| { - barrier.wait(); - Python::attach(|py| { - // blocks until the other thread enters a critical section - with_critical_section_mutex(py, &mutex, |b| { - // SAFETY: we never call back into the python interpreter inside this critical section - assert!(unsafe { *b.get() }); - }); - }); - }); - }); - } - - #[cfg(feature = "macros")] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled - #[test] - fn test_critical_section2() { - let barrier = Barrier::new(3); - - let (bool_wrapper1, bool_wrapper2) = Python::attach(|py| { - ( - Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap(), - Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap(), - ) - }); - - std::thread::scope(|s| { - s.spawn(|| { - Python::attach(|py| { - let b1 = bool_wrapper1.bind(py); - let b2 = bool_wrapper2.bind(py); - with_critical_section2(b1, b2, || { - barrier.wait(); - std::thread::sleep(std::time::Duration::from_millis(10)); - b1.borrow().0.store(true, Ordering::Release); - b2.borrow().0.store(true, Ordering::Release); - }) - }); - }); - s.spawn(|| { - barrier.wait(); - Python::attach(|py| { - let b1 = bool_wrapper1.bind(py); - // this blocks until the other thread's critical section finishes - with_critical_section(b1, || { - assert!(b1.borrow().0.load(Ordering::Acquire)); - }); - }); - }); - s.spawn(|| { - barrier.wait(); - Python::attach(|py| { - let b2 = bool_wrapper2.bind(py); - // this blocks until the other thread's critical section finishes - with_critical_section(b2, || { - assert!(b2.borrow().0.load(Ordering::Acquire)); - }); - }); - }); - }); - } - - #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled - #[test] - fn test_critical_section_mutex2() { - let barrier = Barrier::new(2); - - let m1 = PyMutex::new(false); - let m2 = PyMutex::new(false); - - std::thread::scope(|s| { - s.spawn(|| { - Python::attach(|py| { - with_critical_section_mutex2(py, &m1, &m2, |mut b1, mut b2| { - barrier.wait(); - std::thread::sleep(std::time::Duration::from_millis(10)); - // SAFETY: we never call back into the python interpreter inside this critical section - unsafe { (*b1.get_mut()) = true }; - unsafe { (*b2.get_mut()) = true }; - }); - }); - }); - s.spawn(|| { - barrier.wait(); - Python::attach(|py| { - // blocks until the other thread enters a critical section - with_critical_section_mutex2(py, &m1, &m2, |b1, b2| { - // SAFETY: we never call back into the python interpreter inside this critical section - assert!(unsafe { *b1.get() }); - assert!(unsafe { *b2.get() }); - }); - }); - }); - }); - } - - #[cfg(feature = "macros")] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled - #[test] - fn test_critical_section2_same_object_no_deadlock() { - let barrier = Barrier::new(2); - - let bool_wrapper = Python::attach(|py| -> Py { - Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap() - }); - - std::thread::scope(|s| { - s.spawn(|| { - Python::attach(|py| { - let b = bool_wrapper.bind(py); - with_critical_section2(b, b, || { - barrier.wait(); - std::thread::sleep(std::time::Duration::from_millis(10)); - b.borrow().0.store(true, Ordering::Release); - }) - }); - }); - s.spawn(|| { - barrier.wait(); - Python::attach(|py| { - let b = bool_wrapper.bind(py); - // this blocks until the other thread's critical section finishes - with_critical_section(b, || { - assert!(b.borrow().0.load(Ordering::Acquire)); - }); - }); - }); - }); - } - - #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled - #[test] - fn test_critical_section_mutex2_same_object_no_deadlock() { - let barrier = Barrier::new(2); - - let m = PyMutex::new(false); - - std::thread::scope(|s| { - s.spawn(|| { - Python::attach(|py| { - with_critical_section_mutex2(py, &m, &m, |mut b1, b2| { - barrier.wait(); - std::thread::sleep(std::time::Duration::from_millis(10)); - // SAFETY: we never call back into the python interpreter inside this critical section - unsafe { (*b1.get_mut()) = true }; - assert!(unsafe { *b2.get() }); - }); - }); - }); - s.spawn(|| { - barrier.wait(); - Python::attach(|py| { - // this blocks until the other thread's critical section finishes - with_critical_section_mutex(py, &m, |b| { - // SAFETY: we never call back into the python interpreter inside this critical section - assert!(unsafe { *b.get() }); - }); - }); - }); - }); - } - - #[cfg(feature = "macros")] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled - #[test] - fn test_critical_section2_two_containers() { - let (vec1, vec2) = Python::attach(|py| { - ( - Py::new(py, VecWrapper(vec![1, 2, 3])).unwrap(), - Py::new(py, VecWrapper(vec![4, 5])).unwrap(), - ) - }); - - std::thread::scope(|s| { - s.spawn(|| { - Python::attach(|py| { - let v1 = vec1.bind(py); - let v2 = vec2.bind(py); - with_critical_section2(v1, v2, || { - // v2.extend(v1) - v2.borrow_mut().0.extend(v1.borrow().0.iter()); - }) - }); - }); - s.spawn(|| { - Python::attach(|py| { - let v1 = vec1.bind(py); - let v2 = vec2.bind(py); - with_critical_section2(v1, v2, || { - // v1.extend(v2) - v1.borrow_mut().0.extend(v2.borrow().0.iter()); - }) - }); - }); - }); - - Python::attach(|py| { - let v1 = vec1.bind(py); - let v2 = vec2.bind(py); - // execution order is not guaranteed, so we need to check both - // NB: extend should be atomic, items must not be interleaved - // v1.extend(v2) - // v2.extend(v1) - let expected1_vec1 = vec![1, 2, 3, 4, 5]; - let expected1_vec2 = vec![4, 5, 1, 2, 3, 4, 5]; - // v2.extend(v1) - // v1.extend(v2) - let expected2_vec1 = vec![1, 2, 3, 4, 5, 1, 2, 3]; - let expected2_vec2 = vec![4, 5, 1, 2, 3]; - - assert!( - (v1.borrow().0.eq(&expected1_vec1) && v2.borrow().0.eq(&expected1_vec2)) - || (v1.borrow().0.eq(&expected2_vec1) && v2.borrow().0.eq(&expected2_vec2)) - ); - }); - } - - #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled - #[test] - fn test_critical_section_mutex2_two_containers() { - let (m1, m2) = (PyMutex::new(vec![1, 2, 3]), PyMutex::new(vec![4, 5])); - - let (m1_guard, m2_guard) = (m1.lock().unwrap(), m2.lock().unwrap()); - - std::thread::scope(|s| { - s.spawn(|| { - Python::attach(|py| { - with_critical_section_mutex2(py, &m1, &m2, |mut v1, v2| { - // v1.extend(v1) - // SAFETY: we never call back into the python interpreter inside this critical section - let vec1 = unsafe { v1.get_mut() }; - let vec2 = unsafe { v2.get() }; - vec1.extend(vec2.iter()); - }) - }); - }); - s.spawn(|| { - Python::attach(|py| { - with_critical_section_mutex2(py, &m1, &m2, |v1, mut v2| { - // v2.extend(v1) - // SAFETY: we never call back into the python interpreter inside this critical section - let vec1 = unsafe { v1.get() }; - let vec2 = unsafe { v2.get_mut() }; - vec2.extend(vec1.iter()); - }) - }); - }); - // the other threads waiting for locks should not block this attach - Python::attach(|_| { - // On the free-threaded build, the critical sections should have blocked - // the other threads from modification. - #[cfg(Py_GIL_DISABLED)] - { - assert_eq!(&*m1_guard, &[1, 2, 3]); - assert_eq!(&*m2_guard, &[4, 5]); - } - }); - drop(m1_guard); - drop(m2_guard); - }); - - // execution order is not guaranteed, so we need to check both - // NB: extend should be atomic, items must not be interleaved - // v1.extend(v2) - // v2.extend(v1) - let expected1_vec1 = vec![1, 2, 3, 4, 5]; - let expected1_vec2 = vec![4, 5, 1, 2, 3, 4, 5]; - // v2.extend(v1) - // v1.extend(v2) - let expected2_vec1 = vec![1, 2, 3, 4, 5, 1, 2, 3]; - let expected2_vec2 = vec![4, 5, 1, 2, 3]; - - let v1 = m1.lock().unwrap(); - let v2 = m2.lock().unwrap(); - assert!( - (&*v1, &*v2) == (&expected1_vec1, &expected1_vec2) - || (&*v1, &*v2) == (&expected2_vec1, &expected2_vec2) - ); - } - #[test] #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled fn test_once_ext() { diff --git a/src/sync/critical_section.rs b/src/sync/critical_section.rs new file mode 100644 index 00000000000..1a3ea4ebbec --- /dev/null +++ b/src/sync/critical_section.rs @@ -0,0 +1,625 @@ +//! Wrappers for the Python critical section API +//! +//! [Critical Sections](https://docs.python.org/3/c-api/init.html#python-critical-section-api) allow +//! access to the [`PyMutex`](https://docs.python.org/3/c-api/init.html#c.PyMutex) lock attached to +//! each Python object in the free-threaded build. They are no-ops on the GIL-enabled build. +//! +//! Provides weaker locking guarantees than traditional locks, but can in some cases be used to +//! provide guarantees similar to the GIL without the risk of deadlocks associated with traditional +//! locks. +//! +//! # Usage Notes +//! +//! The calling thread acquires the per-object lock when it enters the critical section and holds it +//! until exiting the critical section unless the critical section is suspended. Any call into the +//! CPython C API may cause the critical section to be suspended. Creating an inner critical +//! section, for example by accessing an item in a Python list or dict, will cause the outer +//! critical section to be relased while the inner critical section is active. +//! +//! As a consequence, it is only possible to lock one or two objects at a time. If you need two lock +//! two objects, you should use the variants that accept two arguments. The outer critical section +//! is suspended if you create an outer an inner critical section on two objects using the +//! single-argument variants. +//! +//! It is not currently possible to lock more than two objects simultaneously using this mechanism. +//! Taking a critical section on a container object does not lock the objects stored in the +//! container. +//! +//! Many CPython C API functions do not acquire the per-object lock on objects passed to Python. You +//! should not expect critical sections applied to built-in types to prevent concurrent +//! modification. This API is most useful for user-defined types with full control over how the +//! internal state for the type is managed. +//! +//! The caller must ensure the closure cannot implicitly release the critical section. If a +//! multithreaded program calls back into the Python interpreter in a manner that would cause the +//! critical section to be released, the per-object lock will be released and the state of the +//! object may be read from or modified by another thread. Concurrent modifications are impossible, +//! but races are possible and the state of an object may change "underneath" a suspended thread in +//! possibly surprising ways. + +#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] +use crate::types::PyMutex; + +use crate::{types::PyAny, Bound, Python}; +use std::cell::UnsafeCell; + +#[cfg(Py_GIL_DISABLED)] +struct CSGuard(crate::ffi::PyCriticalSection); + +#[cfg(Py_GIL_DISABLED)] +impl Drop for CSGuard { + fn drop(&mut self) { + unsafe { + crate::ffi::PyCriticalSection_End(&mut self.0); + } + } +} + +#[cfg(Py_GIL_DISABLED)] +struct CS2Guard(crate::ffi::PyCriticalSection2); + +#[cfg(Py_GIL_DISABLED)] +impl Drop for CS2Guard { + fn drop(&mut self) { + unsafe { + crate::ffi::PyCriticalSection2_End(&mut self.0); + } + } +} + +/// Allows access to data protected by a PyMutex in a critical section +/// +/// Used with the `with_critical_section_mutex` and +/// `with_critical_section_mutex2` functions. See the documentation of those +/// functions for more details. +#[cfg(any(Py_3_14, Py_GIL_DISABLED))] +pub struct EnteredCriticalSection<'a, T>(&'a UnsafeCell); + +#[cfg(any(Py_3_14, Py_GIL_DISABLED))] +impl EnteredCriticalSection<'_, T> { + /// Get a mutable reference to the data wrapped by a PyMutex + /// + /// # Safety + /// + /// The caller must ensure the closure cannot implicitly release the critical section. + /// + /// If a multithreaded program calls back into the Python interpreter in a manner that would cause + /// the critical section to be released, the `PyMutex` will be released and the resource protected + /// by the `PyMutex` may be read from or modified by another thread while the critical section is + /// suspended. Concurrent modifications are impossible, but races are possible and the state of the + /// protected resource may change in possibly surprising ways after calls into the interpreter. + pub unsafe fn get_mut(&mut self) -> &mut T { + unsafe { &mut *(self.0.get()) } + } + + /// Get a immutable reference to the value wrapped by a PyMutex + /// + /// # Safety + /// + /// The caller must ensure the critical section is not released while the + /// reference is alive. If a multithreaded program calls back into the + /// Python interpreter in a manner that would cause the critical section to + /// be released, the `PyMutex` will be released and the resource protected + /// by the `PyMutex` may be read from or modified by another thread while + /// the critical section is suspended and the thread that owns the reference + /// is blocked. Concurrent modifications are impossible, but races are + /// possible and the state of an object may change "underneath" a suspended + /// thread in possibly surprising ways. Note that many operations on Python + /// objects may call back into the interpreter in a blocking manner because + /// many C API calls can trigger the execution of arbitrary Python code. + pub unsafe fn get(&self) -> &T { + unsafe { &*(self.0.get()) } + } +} + +/// Executes a closure with a Python critical section held on an object. +/// +/// Acquires the per-object lock for the object `op` that is held while the closure `f` is +/// executing. The critical section may be temporarily released and re-acquired if the closure calls +/// back into the interpreter. See the notes in the +/// [`pyo3::sync::critical_section`][crate::sync::critical_section] module documentation for more +/// details. +/// +/// This is structurally equivalent to the use of the paired Py_BEGIN_CRITICAL_SECTION and +/// Py_END_CRITICAL_SECTION C-API macros. +#[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] +pub fn with_critical_section(object: &Bound<'_, PyAny>, f: F) -> R +where + F: FnOnce() -> R, +{ + #[cfg(Py_GIL_DISABLED)] + { + let mut guard = CSGuard(unsafe { std::mem::zeroed() }); + unsafe { crate::ffi::PyCriticalSection_Begin(&mut guard.0, object.as_ptr()) }; + f() + } + #[cfg(not(Py_GIL_DISABLED))] + { + f() + } +} + +/// Executes a closure with a Python critical section held on two objects. +/// +/// Acquires the per-object lock for the objects `a` and `b` that are held while the closure `f` is +/// executing. The critical section may be temporarily released and re-acquired if the closure calls +/// back into the interpreter. See the notes in the +/// [`pyo3::sync::critical_section`][crate::sync::critical_section] module documentation for more +/// details. +/// +/// This is structurally equivalent to the use of the paired +/// Py_BEGIN_CRITICAL_SECTION2 and Py_END_CRITICAL_SECTION2 C-API macros. +#[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] +pub fn with_critical_section2(a: &Bound<'_, PyAny>, b: &Bound<'_, PyAny>, f: F) -> R +where + F: FnOnce() -> R, +{ + #[cfg(Py_GIL_DISABLED)] + { + let mut guard = CS2Guard(unsafe { std::mem::zeroed() }); + unsafe { crate::ffi::PyCriticalSection2_Begin(&mut guard.0, a.as_ptr(), b.as_ptr()) }; + f() + } + #[cfg(not(Py_GIL_DISABLED))] + { + f() + } +} + +/// Executes a closure with a Python critical section held on a `PyMutex`. +/// +/// Acquires the mutex `mutex` until the closure `f` finishes. The mutex may be temporarily released +/// and re-acquired if the closure calls back into the interpreter. See the notes in the +/// [`pyo3::sync::critical_section`][crate::sync::critical_section] module documentation for more +/// details. +/// +/// This variant is particularly useful when paired with a global `PyMutex` to create a "local GIL" +/// to protect global state in an extension in an analogous manner to the GIL without introducing +/// any deadlock risks or affecting runtime behavior on the GIL-enabled build. +/// +/// This is structurally equivalent to the use of the paired Py_BEGIN_CRITICAL_SECTION_MUTEX and +/// Py_END_CRITICAL_SECTION C-API macros. +/// +/// Only available on Python 3.14 and newer. +/// +/// # Safety +/// +/// The caller must ensure the closure cannot implicitly release the critical section. See the +/// safety notes in the documentation for +/// [`pyo3::sync::critical_section::EnteredCriticalSection`](crate::sync::critical_section::EnteredCriticalSection) +/// for more details. +#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] +#[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] +pub fn with_critical_section_mutex(_py: Python<'_>, mutex: &PyMutex, f: F) -> R +where + F: for<'s> FnOnce(EnteredCriticalSection<'s, T>) -> R, +{ + #[cfg(Py_GIL_DISABLED)] + { + let mut guard = CSGuard(unsafe { std::mem::zeroed() }); + unsafe { crate::ffi::PyCriticalSection_BeginMutex(&mut guard.0, &mut *mutex.mutex.get()) }; + f(EnteredCriticalSection(&mutex.data)) + } + #[cfg(not(Py_GIL_DISABLED))] + { + f(EnteredCriticalSection(&mutex.data)) + } +} + +/// Executes a closure with a Python critical section held on two `PyMutex` instances. +/// +/// Simultaneously acquires the mutexes `m1` and `m2` and holds them until the closure `f` is +/// finished. The mutexes may be temporarily released and re-acquired if the closure calls back into +/// the interpreter. See the notes in the +/// [`pyo3::sync::critical_section`][crate::sync::critical_section] module documentation for more +/// details. +/// +/// This is structurally equivalent to the use of the paired +/// Py_BEGIN_CRITICAL_SECTION2_MUTEX and Py_END_CRITICAL_SECTION2 C-API macros. +/// +/// A no-op on GIL-enabled builds, where the critical section API is exposed as +/// a no-op by the Python C API. +/// +/// Only available on Python 3.14 and newer. +/// +/// # Safety +/// +/// The caller must ensure the closure cannot implicitly release the critical section. See the +/// safety notes in the documentation for +/// [`pyo3::sync::critical_section::EnteredCriticalSection`](crate::sync::critical_section::EnteredCriticalSection) +/// for more details. +#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] +#[cfg_attr(not(Py_GIL_DISABLED), allow(unused_variables))] +pub fn with_critical_section_mutex2( + _py: Python<'_>, + m1: &PyMutex, + m2: &PyMutex, + f: F, +) -> R +where + F: for<'s> FnOnce(EnteredCriticalSection<'s, T1>, EnteredCriticalSection<'s, T2>) -> R, +{ + #[cfg(Py_GIL_DISABLED)] + { + let mut guard = CS2Guard(unsafe { std::mem::zeroed() }); + unsafe { + crate::ffi::PyCriticalSection2_BeginMutex( + &mut guard.0, + &mut *m1.mutex.get(), + &mut *m2.mutex.get(), + ) + }; + f( + EnteredCriticalSection(&m1.data), + EnteredCriticalSection(&m2.data), + ) + } + #[cfg(not(Py_GIL_DISABLED))] + { + f( + EnteredCriticalSection(&m1.data), + EnteredCriticalSection(&m2.data), + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + #[cfg(not(target_arch = "wasm32"))] + #[cfg(feature = "macros")] + use std::sync::atomic::{AtomicBool, Ordering}; + #[cfg(not(target_arch = "wasm32"))] + #[cfg(any(feature = "macros", all(not(Py_LIMITED_API), Py_3_14)))] + use std::sync::Barrier; + + #[cfg(not(target_arch = "wasm32"))] + #[cfg(feature = "macros")] + use crate::Py; + + #[cfg(not(target_arch = "wasm32"))] + #[cfg(feature = "macros")] + #[crate::pyclass(crate = "crate")] + struct VecWrapper(Vec); + + #[cfg(not(target_arch = "wasm32"))] + #[cfg(feature = "macros")] + #[crate::pyclass(crate = "crate")] + struct BoolWrapper(AtomicBool); + + #[cfg(feature = "macros")] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section() { + let barrier = Barrier::new(2); + + let bool_wrapper = Python::attach(|py| -> Py { + Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap() + }); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + let b = bool_wrapper.bind(py); + with_critical_section(b, || { + barrier.wait(); + std::thread::sleep(std::time::Duration::from_millis(10)); + b.borrow().0.store(true, Ordering::Release); + }) + }); + }); + s.spawn(|| { + barrier.wait(); + Python::attach(|py| { + let b = bool_wrapper.bind(py); + // this blocks until the other thread's critical section finishes + with_critical_section(b, || { + assert!(b.borrow().0.load(Ordering::Acquire)); + }); + }); + }); + }); + } + + #[cfg(all(not(Py_LIMITED_API), Py_3_14))] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section_mutex() { + let barrier = Barrier::new(2); + + let mutex = PyMutex::new(false); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + with_critical_section_mutex(py, &mutex, |mut b| { + barrier.wait(); + std::thread::sleep(std::time::Duration::from_millis(10)); + // SAFETY: we never call back into the python interpreter inside this critical section + *(unsafe { b.get_mut() }) = true; + }); + }); + }); + s.spawn(|| { + barrier.wait(); + Python::attach(|py| { + // blocks until the other thread enters a critical section + with_critical_section_mutex(py, &mutex, |b| { + // SAFETY: we never call back into the python interpreter inside this critical section + assert!(unsafe { *b.get() }); + }); + }); + }); + }); + } + + #[cfg(feature = "macros")] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section2() { + let barrier = Barrier::new(3); + + let (bool_wrapper1, bool_wrapper2) = Python::attach(|py| { + ( + Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap(), + Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap(), + ) + }); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + let b1 = bool_wrapper1.bind(py); + let b2 = bool_wrapper2.bind(py); + with_critical_section2(b1, b2, || { + barrier.wait(); + std::thread::sleep(std::time::Duration::from_millis(10)); + b1.borrow().0.store(true, Ordering::Release); + b2.borrow().0.store(true, Ordering::Release); + }) + }); + }); + s.spawn(|| { + barrier.wait(); + Python::attach(|py| { + let b1 = bool_wrapper1.bind(py); + // this blocks until the other thread's critical section finishes + with_critical_section(b1, || { + assert!(b1.borrow().0.load(Ordering::Acquire)); + }); + }); + }); + s.spawn(|| { + barrier.wait(); + Python::attach(|py| { + let b2 = bool_wrapper2.bind(py); + // this blocks until the other thread's critical section finishes + with_critical_section(b2, || { + assert!(b2.borrow().0.load(Ordering::Acquire)); + }); + }); + }); + }); + } + + #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section_mutex2() { + let barrier = Barrier::new(2); + + let m1 = PyMutex::new(false); + let m2 = PyMutex::new(false); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + with_critical_section_mutex2(py, &m1, &m2, |mut b1, mut b2| { + barrier.wait(); + std::thread::sleep(std::time::Duration::from_millis(10)); + // SAFETY: we never call back into the python interpreter inside this critical section + unsafe { (*b1.get_mut()) = true }; + unsafe { (*b2.get_mut()) = true }; + }); + }); + }); + s.spawn(|| { + barrier.wait(); + Python::attach(|py| { + // blocks until the other thread enters a critical section + with_critical_section_mutex2(py, &m1, &m2, |b1, b2| { + // SAFETY: we never call back into the python interpreter inside this critical section + assert!(unsafe { *b1.get() }); + assert!(unsafe { *b2.get() }); + }); + }); + }); + }); + } + + #[cfg(feature = "macros")] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section2_same_object_no_deadlock() { + let barrier = Barrier::new(2); + + let bool_wrapper = Python::attach(|py| -> Py { + Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap() + }); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + let b = bool_wrapper.bind(py); + with_critical_section2(b, b, || { + barrier.wait(); + std::thread::sleep(std::time::Duration::from_millis(10)); + b.borrow().0.store(true, Ordering::Release); + }) + }); + }); + s.spawn(|| { + barrier.wait(); + Python::attach(|py| { + let b = bool_wrapper.bind(py); + // this blocks until the other thread's critical section finishes + with_critical_section(b, || { + assert!(b.borrow().0.load(Ordering::Acquire)); + }); + }); + }); + }); + } + + #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section_mutex2_same_object_no_deadlock() { + let barrier = Barrier::new(2); + + let m = PyMutex::new(false); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + with_critical_section_mutex2(py, &m, &m, |mut b1, b2| { + barrier.wait(); + std::thread::sleep(std::time::Duration::from_millis(10)); + // SAFETY: we never call back into the python interpreter inside this critical section + unsafe { (*b1.get_mut()) = true }; + assert!(unsafe { *b2.get() }); + }); + }); + }); + s.spawn(|| { + barrier.wait(); + Python::attach(|py| { + // this blocks until the other thread's critical section finishes + with_critical_section_mutex(py, &m, |b| { + // SAFETY: we never call back into the python interpreter inside this critical section + assert!(unsafe { *b.get() }); + }); + }); + }); + }); + } + + #[cfg(feature = "macros")] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section2_two_containers() { + let (vec1, vec2) = Python::attach(|py| { + ( + Py::new(py, VecWrapper(vec![1, 2, 3])).unwrap(), + Py::new(py, VecWrapper(vec![4, 5])).unwrap(), + ) + }); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + let v1 = vec1.bind(py); + let v2 = vec2.bind(py); + with_critical_section2(v1, v2, || { + // v2.extend(v1) + v2.borrow_mut().0.extend(v1.borrow().0.iter()); + }) + }); + }); + s.spawn(|| { + Python::attach(|py| { + let v1 = vec1.bind(py); + let v2 = vec2.bind(py); + with_critical_section2(v1, v2, || { + // v1.extend(v2) + v1.borrow_mut().0.extend(v2.borrow().0.iter()); + }) + }); + }); + }); + + Python::attach(|py| { + let v1 = vec1.bind(py); + let v2 = vec2.bind(py); + // execution order is not guaranteed, so we need to check both + // NB: extend should be atomic, items must not be interleaved + // v1.extend(v2) + // v2.extend(v1) + let expected1_vec1 = vec![1, 2, 3, 4, 5]; + let expected1_vec2 = vec![4, 5, 1, 2, 3, 4, 5]; + // v2.extend(v1) + // v1.extend(v2) + let expected2_vec1 = vec![1, 2, 3, 4, 5, 1, 2, 3]; + let expected2_vec2 = vec![4, 5, 1, 2, 3]; + + assert!( + (v1.borrow().0.eq(&expected1_vec1) && v2.borrow().0.eq(&expected1_vec2)) + || (v1.borrow().0.eq(&expected2_vec1) && v2.borrow().0.eq(&expected2_vec2)) + ); + }); + } + + #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_critical_section_mutex2_two_containers() { + let (m1, m2) = (PyMutex::new(vec![1, 2, 3]), PyMutex::new(vec![4, 5])); + + let (m1_guard, m2_guard) = (m1.lock().unwrap(), m2.lock().unwrap()); + + std::thread::scope(|s| { + s.spawn(|| { + Python::attach(|py| { + with_critical_section_mutex2(py, &m1, &m2, |mut v1, v2| { + // v1.extend(v1) + // SAFETY: we never call back into the python interpreter inside this critical section + let vec1 = unsafe { v1.get_mut() }; + let vec2 = unsafe { v2.get() }; + vec1.extend(vec2.iter()); + }) + }); + }); + s.spawn(|| { + Python::attach(|py| { + with_critical_section_mutex2(py, &m1, &m2, |v1, mut v2| { + // v2.extend(v1) + // SAFETY: we never call back into the python interpreter inside this critical section + let vec1 = unsafe { v1.get() }; + let vec2 = unsafe { v2.get_mut() }; + vec2.extend(vec1.iter()); + }) + }); + }); + // the other threads waiting for locks should not block this attach + Python::attach(|_| { + // On the free-threaded build, the critical sections should have blocked + // the other threads from modification. + #[cfg(Py_GIL_DISABLED)] + { + assert_eq!(&*m1_guard, &[1, 2, 3]); + assert_eq!(&*m2_guard, &[4, 5]); + } + }); + drop(m1_guard); + drop(m2_guard); + }); + + // execution order is not guaranteed, so we need to check both + // NB: extend should be atomic, items must not be interleaved + // v1.extend(v2) + // v2.extend(v1) + let expected1_vec1 = vec![1, 2, 3, 4, 5]; + let expected1_vec2 = vec![4, 5, 1, 2, 3, 4, 5]; + // v2.extend(v1) + // v1.extend(v2) + let expected2_vec1 = vec![1, 2, 3, 4, 5, 1, 2, 3]; + let expected2_vec2 = vec![4, 5, 1, 2, 3]; + + let v1 = m1.lock().unwrap(); + let v2 = m2.lock().unwrap(); + assert!( + (&*v1, &*v2) == (&expected1_vec1, &expected1_vec2) + || (&*v1, &*v2) == (&expected2_vec1, &expected2_vec2) + ); + } +} diff --git a/src/types/bytearray.rs b/src/types/bytearray.rs index 473d1c60484..e7374535adf 100644 --- a/src/types/bytearray.rs +++ b/src/types/bytearray.rs @@ -2,7 +2,7 @@ use crate::err::{PyErr, PyResult}; use crate::ffi_ptr_ext::FfiPtrExt; use crate::instance::{Borrowed, Bound}; use crate::py_result_ext::PyResultExt; -use crate::sync::with_critical_section; +use crate::sync::critical_section::with_critical_section; use crate::{ffi, PyAny, Python}; use std::slice; diff --git a/src/types/code.rs b/src/types/code.rs index ffe6c964095..5e1bfa2eda2 100644 --- a/src/types/code.rs +++ b/src/types/code.rs @@ -114,7 +114,7 @@ impl<'py> PyCodeMethods<'py> for Bound<'py, PyCode> { let builtins_s = crate::intern!(self.py(), "__builtins__"); let has_builtins = globals.contains(builtins_s)?; if !has_builtins { - crate::sync::with_critical_section(globals, || { + crate::sync::critical_section::with_critical_section(globals, || { // check if another thread set __builtins__ while this thread was blocked on the critical section let has_builtins = globals.contains(builtins_s)?; if !has_builtins { diff --git a/src/types/dict.rs b/src/types/dict.rs index 9e842a2c659..57eb2133acb 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -360,7 +360,7 @@ impl<'py> PyDictMethods<'py> for Bound<'py, PyDict> { #[cfg(not(feature = "nightly"))] { - crate::sync::with_critical_section(self, || { + crate::sync::critical_section::with_critical_section(self, || { self.iter().try_for_each(|(key, value)| f(key, value)) }) } @@ -499,7 +499,9 @@ impl DictIterImpl { F: FnOnce(&mut Self) -> R, { match self { - Self::DictIter { .. } => crate::sync::with_critical_section(dict, || f(self)), + Self::DictIter { .. } => { + crate::sync::critical_section::with_critical_section(dict, || f(self)) + } } } } diff --git a/src/types/list.rs b/src/types/list.rs index 3143a51518b..d9db1aed17a 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -427,7 +427,9 @@ impl<'py> PyListMethods<'py> for Bound<'py, PyList> { where F: Fn(Bound<'py, PyAny>) -> PyResult<()>, { - crate::sync::with_critical_section(self, || self.iter().try_for_each(closure)) + crate::sync::critical_section::with_critical_section(self, || { + self.iter().try_for_each(closure) + }) } /// Sorts the list in-place. Equivalent to the Python expression `l.sort()`. @@ -626,7 +628,7 @@ impl<'py> BoundListIterator<'py> { length, list, } = self; - crate::sync::with_critical_section(list, || f(index, length, list)) + crate::sync::critical_section::with_critical_section(list, || f(index, length, list)) } } From 82fd33abc4466443661a59a26cbd7129d37902b2 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Wed, 3 Dec 2025 17:02:31 -0700 Subject: [PATCH 25/32] adjust imports to satisfy clippy --- src/sync.rs | 2 +- src/sync/critical_section.rs | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 203390d50fa..0b5fa6734fa 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -941,7 +941,7 @@ mod tests { #[cfg(feature = "macros")] use std::sync::atomic::{AtomicBool, Ordering}; #[cfg(not(target_arch = "wasm32"))] - #[cfg(any(feature = "macros", all(not(Py_LIMITED_API), Py_3_14)))] + #[cfg(feature = "macros")] use std::sync::Barrier; #[cfg(not(target_arch = "wasm32"))] use std::sync::Mutex; diff --git a/src/sync/critical_section.rs b/src/sync/critical_section.rs index 1a3ea4ebbec..ac3ea453ca8 100644 --- a/src/sync/critical_section.rs +++ b/src/sync/critical_section.rs @@ -40,7 +40,10 @@ #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] use crate::types::PyMutex; -use crate::{types::PyAny, Bound, Python}; +#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] +use crate::Python; +use crate::{types::PyAny, Bound}; +#[cfg(any(Py_3_14, not(Py_LIMITED_API)))] use std::cell::UnsafeCell; #[cfg(Py_GIL_DISABLED)] @@ -265,9 +268,16 @@ where #[cfg(test)] mod tests { - use super::*; #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "macros")] + use super::{with_critical_section, with_critical_section2}; + #[cfg(not(target_arch = "wasm32"))] + #[cfg(all(not(Py_LIMITED_API), Py_3_14))] + use super::{with_critical_section_mutex, with_critical_section_mutex2}; + #[cfg(not(target_arch = "wasm32"))] + #[cfg(all(not(Py_LIMITED_API), Py_3_14))] + use crate::types::PyMutex; + #[cfg(feature = "macros")] use std::sync::atomic::{AtomicBool, Ordering}; #[cfg(not(target_arch = "wasm32"))] #[cfg(any(feature = "macros", all(not(Py_LIMITED_API), Py_3_14)))] @@ -276,6 +286,9 @@ mod tests { #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "macros")] use crate::Py; + #[cfg(not(target_arch = "wasm32"))] + #[cfg(any(feature = "macros", all(not(Py_LIMITED_API), Py_3_14)))] + use crate::Python; #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "macros")] From 4dcdc769bbf867e55ad064004702a84f47192769 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 4 Dec 2025 11:15:21 -0700 Subject: [PATCH 26/32] Fix EnteredCriticalSection conditinal compilation --- src/sync/critical_section.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sync/critical_section.rs b/src/sync/critical_section.rs index ac3ea453ca8..0ce44e60036 100644 --- a/src/sync/critical_section.rs +++ b/src/sync/critical_section.rs @@ -43,7 +43,7 @@ use crate::types::PyMutex; #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] use crate::Python; use crate::{types::PyAny, Bound}; -#[cfg(any(Py_3_14, not(Py_LIMITED_API)))] +#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] use std::cell::UnsafeCell; #[cfg(Py_GIL_DISABLED)] @@ -75,10 +75,10 @@ impl Drop for CS2Guard { /// Used with the `with_critical_section_mutex` and /// `with_critical_section_mutex2` functions. See the documentation of those /// functions for more details. -#[cfg(any(Py_3_14, Py_GIL_DISABLED))] +#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] pub struct EnteredCriticalSection<'a, T>(&'a UnsafeCell); -#[cfg(any(Py_3_14, Py_GIL_DISABLED))] +#[cfg(all(Py_3_14, not(Py_LIMITED_API)))] impl EnteredCriticalSection<'_, T> { /// Get a mutable reference to the data wrapped by a PyMutex /// From ab5f66545478c3afab827f5708e397aeb437182b Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 4 Dec 2025 11:33:05 -0700 Subject: [PATCH 27/32] fix wasm clippy --- src/sync/critical_section.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sync/critical_section.rs b/src/sync/critical_section.rs index 0ce44e60036..729bb5d85cf 100644 --- a/src/sync/critical_section.rs +++ b/src/sync/critical_section.rs @@ -278,6 +278,7 @@ mod tests { #[cfg(all(not(Py_LIMITED_API), Py_3_14))] use crate::types::PyMutex; #[cfg(feature = "macros")] + #[cfg(not(target_arch = "wasm32"))] use std::sync::atomic::{AtomicBool, Ordering}; #[cfg(not(target_arch = "wasm32"))] #[cfg(any(feature = "macros", all(not(Py_LIMITED_API), Py_3_14)))] From dd2d1c9944af3f2d151d0b2640aa95bc44d76d37 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 4 Dec 2025 11:51:09 -0700 Subject: [PATCH 28/32] add changelog for the moved functions --- newsfragments/5642.changed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/5642.changed.md diff --git a/newsfragments/5642.changed.md b/newsfragments/5642.changed.md new file mode 100644 index 00000000000..c74a2a6b436 --- /dev/null +++ b/newsfragments/5642.changed.md @@ -0,0 +1 @@ +The `with_critical_section` and `with_critical_section2` functions are now available in `pyo3::sync::critical_section`. Aliases that generate deprecation warnings are available in `pyo3::sync`. \ No newline at end of file From 5f7f48cb26510078b20267f312e5f9ace8c65bfa Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 12 Dec 2025 12:42:53 -0700 Subject: [PATCH 29/32] actually deprecate the old export --- src/sync.rs | 27 ++++++++++++++++++++++++--- src/types/bytearray.rs | 4 ++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 0b5fa6734fa..8b3b5216361 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -12,7 +12,7 @@ use crate::{ internal::state::SuspendAttach, sealed::Sealed, - types::{any::PyAnyMethods, PyString}, + types::{any::PyAnyMethods, PyAny, PyString}, Bound, Py, PyResult, PyTypeCheck, Python, }; use std::{ @@ -28,8 +28,29 @@ pub(crate) mod once_lock; #[cfg(not(Py_GIL_DISABLED))] use crate::PyVisit; -#[deprecated(since = "0.28.0", note = "use std::sync::critical_section instead")] -pub use self::critical_section::{with_critical_section, with_critical_section2}; +/// Deprecated alias for [`pyo3::sync::critical_section::with_critical_section`][crate::sync::critical_section::with_critical_section] +#[deprecated( + since = "0.28.0", + note = "use pyo3::sync::critical_section::with_critical_section instead" +)] +pub fn with_critical_section(object: &Bound<'_, PyAny>, f: F) -> R +where + F: FnOnce() -> R, +{ + crate::sync::critical_section::with_critical_section(object, f) +} + +/// Deprecated alias for [`pyo3::sync::critical_section::with_critical_section2`][crate::sync::critical_section::with_critical_section2] +#[deprecated( + since = "0.28.0", + note = "use pyo3::sync::critical_section::with_critical_section2 instead" +)] +pub fn with_critical_section2(a: &Bound<'_, PyAny>, b: &Bound<'_, PyAny>, f: F) -> R +where + F: FnOnce() -> R, +{ + crate::sync::critical_section::with_critical_section2(a, b, f) +} pub use self::once_lock::PyOnceLock; /// Value with concurrent access protected by the GIL. diff --git a/src/types/bytearray.rs b/src/types/bytearray.rs index e7374535adf..98cf76c427e 100644 --- a/src/types/bytearray.rs +++ b/src/types/bytearray.rs @@ -132,7 +132,7 @@ pub trait PyByteArrayMethods<'py>: crate::sealed::Sealed { /// ```rust /// use pyo3::prelude::*; /// use pyo3::exceptions::PyRuntimeError; - /// use pyo3::sync::with_critical_section; + /// use pyo3::sync::critical_section::with_critical_section; /// use pyo3::types::PyByteArray; /// /// #[pyfunction] @@ -461,7 +461,7 @@ mod tests { #[test] fn test_data_integrity_in_critical_section() { use crate::instance::Py; - use crate::sync::{with_critical_section, MutexExt}; + use crate::sync::{critical_section::with_critical_section, MutexExt}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Mutex; From 3a9c51d6a36c9dd24cd2dc2284d401ee90b2ad5b Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 12 Dec 2025 12:43:42 -0700 Subject: [PATCH 30/32] refer to locking mutexes and acquiring critical sections consistently --- src/sync/critical_section.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/sync/critical_section.rs b/src/sync/critical_section.rs index 729bb5d85cf..9c985906858 100644 --- a/src/sync/critical_section.rs +++ b/src/sync/critical_section.rs @@ -10,7 +10,7 @@ //! //! # Usage Notes //! -//! The calling thread acquires the per-object lock when it enters the critical section and holds it +//! The calling thread locks the per-object mutex when it enters the critical section and holds it //! until exiting the critical section unless the critical section is suspended. Any call into the //! CPython C API may cause the critical section to be suspended. Creating an inner critical //! section, for example by accessing an item in a Python list or dict, will cause the outer @@ -25,14 +25,14 @@ //! Taking a critical section on a container object does not lock the objects stored in the //! container. //! -//! Many CPython C API functions do not acquire the per-object lock on objects passed to Python. You +//! Many CPython C API functions do not lock the per-object mutex on objects passed to Python. You //! should not expect critical sections applied to built-in types to prevent concurrent //! modification. This API is most useful for user-defined types with full control over how the //! internal state for the type is managed. //! //! The caller must ensure the closure cannot implicitly release the critical section. If a //! multithreaded program calls back into the Python interpreter in a manner that would cause the -//! critical section to be released, the per-object lock will be released and the state of the +//! critical section to be released, the per-object mutex will be unlocked and the state of the //! object may be read from or modified by another thread. Concurrent modifications are impossible, //! but races are possible and the state of an object may change "underneath" a suspended thread in //! possibly surprising ways. @@ -87,7 +87,7 @@ impl EnteredCriticalSection<'_, T> { /// The caller must ensure the closure cannot implicitly release the critical section. /// /// If a multithreaded program calls back into the Python interpreter in a manner that would cause - /// the critical section to be released, the `PyMutex` will be released and the resource protected + /// the critical section to be released, the `PyMutex` will be unlocked and the resource protected /// by the `PyMutex` may be read from or modified by another thread while the critical section is /// suspended. Concurrent modifications are impossible, but races are possible and the state of the /// protected resource may change in possibly surprising ways after calls into the interpreter. @@ -102,7 +102,7 @@ impl EnteredCriticalSection<'_, T> { /// The caller must ensure the critical section is not released while the /// reference is alive. If a multithreaded program calls back into the /// Python interpreter in a manner that would cause the critical section to - /// be released, the `PyMutex` will be released and the resource protected + /// be released, the `PyMutex` will be unlocked and the resource protected /// by the `PyMutex` may be read from or modified by another thread while /// the critical section is suspended and the thread that owns the reference /// is blocked. Concurrent modifications are impossible, but races are @@ -117,7 +117,7 @@ impl EnteredCriticalSection<'_, T> { /// Executes a closure with a Python critical section held on an object. /// -/// Acquires the per-object lock for the object `op` that is held while the closure `f` is +/// Locks the per-object mutex for the object `op` that is held while the closure `f` is /// executing. The critical section may be temporarily released and re-acquired if the closure calls /// back into the interpreter. See the notes in the /// [`pyo3::sync::critical_section`][crate::sync::critical_section] module documentation for more @@ -144,7 +144,7 @@ where /// Executes a closure with a Python critical section held on two objects. /// -/// Acquires the per-object lock for the objects `a` and `b` that are held while the closure `f` is +/// Locks the per-object mutex for the objects `a` and `b` that are held while the closure `f` is /// executing. The critical section may be temporarily released and re-acquired if the closure calls /// back into the interpreter. See the notes in the /// [`pyo3::sync::critical_section`][crate::sync::critical_section] module documentation for more @@ -171,7 +171,7 @@ where /// Executes a closure with a Python critical section held on a `PyMutex`. /// -/// Acquires the mutex `mutex` until the closure `f` finishes. The mutex may be temporarily released +/// Locks the mutex `mutex` until the closure `f` finishes. The mutex may be temporarily unlocked /// and re-acquired if the closure calls back into the interpreter. See the notes in the /// [`pyo3::sync::critical_section`][crate::sync::critical_section] module documentation for more /// details. @@ -211,8 +211,8 @@ where /// Executes a closure with a Python critical section held on two `PyMutex` instances. /// -/// Simultaneously acquires the mutexes `m1` and `m2` and holds them until the closure `f` is -/// finished. The mutexes may be temporarily released and re-acquired if the closure calls back into +/// Simultaneously locks the mutexes `m1` and `m2` and holds them until the closure `f` is +/// finished. The mutexes may be temporarily unlock and re-acquired if the closure calls back into /// the interpreter. See the notes in the /// [`pyo3::sync::critical_section`][crate::sync::critical_section] module documentation for more /// details. From ba976aba576f316b6a1e749ed5911a4d5a7e97dc Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 12 Dec 2025 12:47:19 -0700 Subject: [PATCH 31/32] remove unnecessary availability note --- src/sync/critical_section.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/sync/critical_section.rs b/src/sync/critical_section.rs index 9c985906858..690646b3fa4 100644 --- a/src/sync/critical_section.rs +++ b/src/sync/critical_section.rs @@ -183,8 +183,6 @@ where /// This is structurally equivalent to the use of the paired Py_BEGIN_CRITICAL_SECTION_MUTEX and /// Py_END_CRITICAL_SECTION C-API macros. /// -/// Only available on Python 3.14 and newer. -/// /// # Safety /// /// The caller must ensure the closure cannot implicitly release the critical section. See the @@ -223,8 +221,6 @@ where /// A no-op on GIL-enabled builds, where the critical section API is exposed as /// a no-op by the Python C API. /// -/// Only available on Python 3.14 and newer. -/// /// # Safety /// /// The caller must ensure the closure cannot implicitly release the critical section. See the From 2b6946a58854780994dc2396098dd4df78f2b6bb Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 12 Dec 2025 12:50:38 -0700 Subject: [PATCH 32/32] simplify wasm conditional compilation --- src/sync/critical_section.rs | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/sync/critical_section.rs b/src/sync/critical_section.rs index 690646b3fa4..278be0da12a 100644 --- a/src/sync/critical_section.rs +++ b/src/sync/critical_section.rs @@ -262,43 +262,36 @@ where } } +// We are building wasm Python with pthreads disabled and all these +// tests use threads +#[cfg(not(target_arch = "wasm32"))] #[cfg(test)] mod tests { - #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "macros")] use super::{with_critical_section, with_critical_section2}; - #[cfg(not(target_arch = "wasm32"))] #[cfg(all(not(Py_LIMITED_API), Py_3_14))] use super::{with_critical_section_mutex, with_critical_section_mutex2}; - #[cfg(not(target_arch = "wasm32"))] #[cfg(all(not(Py_LIMITED_API), Py_3_14))] use crate::types::PyMutex; #[cfg(feature = "macros")] - #[cfg(not(target_arch = "wasm32"))] use std::sync::atomic::{AtomicBool, Ordering}; - #[cfg(not(target_arch = "wasm32"))] #[cfg(any(feature = "macros", all(not(Py_LIMITED_API), Py_3_14)))] use std::sync::Barrier; - #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "macros")] use crate::Py; - #[cfg(not(target_arch = "wasm32"))] #[cfg(any(feature = "macros", all(not(Py_LIMITED_API), Py_3_14)))] use crate::Python; - #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "macros")] #[crate::pyclass(crate = "crate")] struct VecWrapper(Vec); - #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "macros")] #[crate::pyclass(crate = "crate")] struct BoolWrapper(AtomicBool); #[cfg(feature = "macros")] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section() { let barrier = Barrier::new(2); @@ -332,7 +325,6 @@ mod tests { } #[cfg(all(not(Py_LIMITED_API), Py_3_14))] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section_mutex() { let barrier = Barrier::new(2); @@ -364,7 +356,6 @@ mod tests { } #[cfg(feature = "macros")] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section2() { let barrier = Barrier::new(3); @@ -413,7 +404,6 @@ mod tests { } #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section_mutex2() { let barrier = Barrier::new(2); @@ -448,7 +438,6 @@ mod tests { } #[cfg(feature = "macros")] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section2_same_object_no_deadlock() { let barrier = Barrier::new(2); @@ -482,7 +471,6 @@ mod tests { } #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section_mutex2_same_object_no_deadlock() { let barrier = Barrier::new(2); @@ -515,7 +503,6 @@ mod tests { } #[cfg(feature = "macros")] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section2_two_containers() { let (vec1, vec2) = Python::attach(|py| { @@ -570,7 +557,6 @@ mod tests { } #[cfg(all(Py_3_14, not(Py_LIMITED_API)))] - #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section_mutex2_two_containers() { let (m1, m2) = (PyMutex::new(vec![1, 2, 3]), PyMutex::new(vec![4, 5]));