diff --git a/guide/src/class/thread-safety.md b/guide/src/class/thread-safety.md index 841ee6ae2db..55c2a3caca8 100644 --- a/guide/src/class/thread-safety.md +++ b/guide/src/class/thread-safety.md @@ -101,8 +101,10 @@ impl MyClass { } ``` +If you need to lock around state stored in the Python interpreter or otherwise call into the Python C API while a lock is held, you might find the `MutexExt` trait useful. It provides a `lock_py_attached` method for `std::sync::Mutex` that avoids deadlocks with the GIL or other global synchronization events in the interpreter. + ### Wrapping unsynchronized data In some cases, the data structures stored within a `#[pyclass]` may themselves not be thread-safe. Rust will therefore not implement `Send` and `Sync` on the `#[pyclass]` type. -To achieve thread-safety, a manual `Send` and `Sync` implementation is required which is `unsafe` and should only be done following careful review of the soundness of the implementation. Doing this for PyO3 types is no different than for any other Rust code, [the Rustonomicon](https://doc.rust-lang.org/nomicon/send-and-sync.html) has a great discussion on this. +To achieve thread-safety, a manual `Send` and `Sync` implementation is required which is `unsafe` and should only be done following careful review of the soundness of the implementation. Doing this for PyO3 types is no different than for any other Rust code, [the Rustonomicon](https://doc.rust-lang.org/nomicon/send-and-sync.html) has a great discussion on this. \ No newline at end of file diff --git a/guide/src/free-threading.md b/guide/src/free-threading.md index 8ee9a2e100e..ffb95d240a1 100644 --- a/guide/src/free-threading.md +++ b/guide/src/free-threading.md @@ -387,18 +387,15 @@ Python::with_gil(|py| { # } ``` -If you are executing arbitrary Python code while holding the lock, then you will -need to use conditional compilation to use [`GILProtected`] on GIL-enabled Python -builds and mutexes otherwise. If your use of [`GILProtected`] does not guard the -execution of arbitrary Python code or use of the CPython C API, then conditional -compilation is likely unnecessary since [`GILProtected`] was not needed in the -first place and instead Rust mutexes or atomics should be preferred. Python 3.13 -introduces [`PyMutex`](https://docs.python.org/3/c-api/init.html#c.PyMutex), -which releases the GIL while the waiting for the lock, so that is another option -if you only need to support newer Python versions. +If you are executing arbitrary Python code while holding the lock, then you +should import the [`MutexExt`] trait and use the `lock_py_attached` method +instead of `lock`. This ensures that global synchronization events started by +the Python runtime can proceed, avoiding possible deadlocks with the +interpreter. [`GILOnceCell`]: {{#PYO3_DOCS_URL}}/pyo3/sync/struct.GILOnceCell.html [`GILProtected`]: https://docs.rs/pyo3/0.22/pyo3/sync/struct.GILProtected.html +[`MutexExt`]: {{#PYO3_DOCS_URL}}/pyo3/sync/trait.MutexExt.html [`Once`]: https://doc.rust-lang.org/stable/std/sync/struct.Once.html [`Once::call_once`]: https://doc.rust-lang.org/stable/std/sync/struct.Once.html#tymethod.call_once [`Once::call_once_force`]: https://doc.rust-lang.org/stable/std/sync/struct.Once.html#tymethod.call_once_force diff --git a/src/sealed.rs b/src/sealed.rs index 0a2846b134a..2c715468047 100644 --- a/src/sealed.rs +++ b/src/sealed.rs @@ -55,3 +55,4 @@ impl Sealed for PyNativeTypeInitializer {} impl Sealed for PyClassInitializer {} impl Sealed for std::sync::Once {} +impl Sealed for std::sync::Mutex {} diff --git a/src/sync.rs b/src/sync.rs index 0845eaf8cec..1d4a6c7a69a 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -498,7 +498,7 @@ pub trait OnceExt: Sealed { fn call_once_force_py_attached(&self, py: Python<'_>, f: impl FnOnce(&OnceState)); } -// Extension trait for [`std::sync::OnceLock`] which helps avoid deadlocks between the Python +/// Extension trait for [`std::sync::OnceLock`] which helps avoid deadlocks between the Python /// interpreter and initialization with the `OnceLock`. #[cfg(rustc_has_once_lock)] pub trait OnceLockExt: once_lock_ext_sealed::Sealed { @@ -516,14 +516,36 @@ pub trait OnceLockExt: once_lock_ext_sealed::Sealed { F: FnOnce() -> T; } -struct Guard(*mut crate::ffi::PyThreadState); +/// Extension trat for [`std::sync::Mutex`] which helps avoid deadlocks between +/// the Python interpreter and acquiring the `Mutex`. +pub trait MutexExt: Sealed { + /// Lock this `Mutex` in a manner that cannot deadlock with the Python interpreter. + /// + /// Before attempting to lock the mutex, this function detaches from the + /// Python runtime. When the lock is acquired, it re-attaches to the Python + /// runtime before returning the `LockResult`. This avoids deadlocks between + /// the GIL and other global synchronization events triggered by the Python + /// interpreter. + fn lock_py_attached( + &self, + py: Python<'_>, + ) -> std::sync::LockResult>; +} -impl Drop for Guard { +struct ThreadStateGuard(*mut crate::ffi::PyThreadState); + +impl Drop for ThreadStateGuard { fn drop(&mut self) { unsafe { ffi::PyEval_RestoreThread(self.0) }; } } +impl ThreadStateGuard { + fn new() -> ThreadStateGuard { + ThreadStateGuard(unsafe { crate::ffi::PyEval_SaveThread() }) + } +} + impl OnceExt for Once { fn call_once_py_attached(&self, py: Python<'_>, f: impl FnOnce()) { if self.is_completed() { @@ -557,6 +579,18 @@ impl OnceLockExt for std::sync::OnceLock { } } +impl MutexExt for std::sync::Mutex { + fn lock_py_attached( + &self, + _py: Python<'_>, + ) -> std::sync::LockResult> { + let ts_guard = ThreadStateGuard::new(); + let res = self.lock(); + drop(ts_guard); + res + } +} + #[cold] fn init_once_py_attached(once: &Once, _py: Python<'_>, f: F) where @@ -564,7 +598,7 @@ where { // Safety: we are currently attached to the GIL, and we expect to block. We will save // the current thread state and restore it as soon as we are done blocking. - let ts_guard = Guard(unsafe { ffi::PyEval_SaveThread() }); + let ts_guard = ThreadStateGuard::new(); once.call_once(move || { drop(ts_guard); @@ -579,7 +613,7 @@ where { // Safety: we are currently attached to the GIL, and we expect to block. We will save // the current thread state and restore it as soon as we are done blocking. - let ts_guard = Guard(unsafe { ffi::PyEval_SaveThread() }); + let ts_guard = ThreadStateGuard::new(); once.call_once_force(move |state| { drop(ts_guard); @@ -598,7 +632,7 @@ where F: FnOnce() -> T, { // SAFETY: we are currently attached to a Python thread - let ts_guard = Guard(unsafe { ffi::PyEval_SaveThread() }); + let ts_guard = ThreadStateGuard::new(); // this trait is guarded by a rustc version config // so clippy's MSRV check is wrong @@ -618,6 +652,13 @@ mod tests { use super::*; use crate::types::{PyDict, PyDictMethods}; + use std::sync::{ + atomic::{AtomicBool, Ordering}, + Barrier, Mutex, + }; + + #[crate::pyclass(crate = "crate")] + struct BoolWrapper(AtomicBool); #[test] fn test_intern() { @@ -692,16 +733,8 @@ mod tests { #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled #[test] fn test_critical_section() { - use std::sync::{ - atomic::{AtomicBool, Ordering}, - Barrier, - }; - let barrier = Barrier::new(2); - #[crate::pyclass(crate = "crate")] - struct BoolWrapper(AtomicBool); - let bool_wrapper = Python::with_gil(|py| -> Py { Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap() }); @@ -781,4 +814,61 @@ mod tests { }); assert_eq!(cell.get(), Some(&12345)); } + + #[cfg(feature = "macros")] + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_mutex_ext() { + let barrier = Barrier::new(2); + + let mutex = Python::with_gil(|py| -> Mutex> { + Mutex::new(Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap()) + }); + + std::thread::scope(|s| { + s.spawn(|| { + Python::with_gil(|py| { + let b = mutex.lock_py_attached(py).unwrap(); + barrier.wait(); + // sleep to ensure the other thread actually blocks + std::thread::sleep(std::time::Duration::from_millis(10)); + (*b).bind(py).borrow().0.store(true, Ordering::Release); + drop(b); + }); + }); + s.spawn(|| { + barrier.wait(); + Python::with_gil(|py| { + // blocks until the other thread releases the lock + let b = mutex.lock_py_attached(py).unwrap(); + assert!((*b).bind(py).borrow().0.load(Ordering::Acquire)); + }); + }); + }); + } + + #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled + #[test] + fn test_mutex_ext_poison() { + let mutex = Mutex::new(42); + + std::thread::scope(|s| { + let lock_result = s.spawn(|| { + Python::with_gil(|py| { + let _unused = mutex.lock_py_attached(py); + panic!(); + }); + }); + assert!(lock_result.join().is_err()); + assert!(mutex.is_poisoned()); + }); + let guard = Python::with_gil(|py| { + // recover from the poisoning + match mutex.lock_py_attached(py) { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + } + }); + assert!(*guard == 42); + } }