From 056857a4e5ec0ddc2bc87e5e125702a21588451d Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 30 Aug 2024 12:51:48 -0600 Subject: [PATCH 1/7] do not define GILProtected if Py_GIL_DISABLED is set --- src/impl_/pyclass/lazy_type_object.rs | 44 ++++++++++++++++++++++++--- src/sync.rs | 8 ++++- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/impl_/pyclass/lazy_type_object.rs b/src/impl_/pyclass/lazy_type_object.rs index be383a272f3..373e8134919 100644 --- a/src/impl_/pyclass/lazy_type_object.rs +++ b/src/impl_/pyclass/lazy_type_object.rs @@ -11,11 +11,17 @@ use crate::{ impl_::pyclass::MaybeRuntimePyMethodDef, impl_::pymethods::PyMethodDefType, pyclass::{create_type_object, PyClassTypeObject}, - sync::{GILOnceCell, GILProtected}, + sync::GILOnceCell, types::PyType, Bound, PyClass, PyErr, PyObject, PyResult, Python, }; +#[cfg(not(Py_GIL_DISABLED))] +use crate::sync::GILProtected; + +#[cfg(Py_GIL_DISABLED)] +use std::sync::Mutex; + use super::PyClassItemsIter; /// Lazy type object for PyClass. @@ -27,7 +33,10 @@ struct LazyTypeObjectInner { value: GILOnceCell, // Threads which have begun initialization of the `tp_dict`. Used for // reentrant initialization detection. + #[cfg(not(Py_GIL_DISABLED))] initializing_threads: GILProtected>>, + #[cfg(Py_GIL_DISABLED)] + initializing_threads: Mutex>>, tp_dict_filled: GILOnceCell<()>, } @@ -38,7 +47,10 @@ impl LazyTypeObject { LazyTypeObject( LazyTypeObjectInner { value: GILOnceCell::new(), + #[cfg(not(Py_GIL_DISABLED))] initializing_threads: GILProtected::new(RefCell::new(Vec::new())), + #[cfg(Py_GIL_DISABLED)] + initializing_threads: Mutex::new(RefCell::new(Vec::new())), tp_dict_filled: GILOnceCell::new(), }, PhantomData, @@ -117,7 +129,11 @@ impl LazyTypeObjectInner { let thread_id = thread::current().id(); { - let mut threads = self.initializing_threads.get(py).borrow_mut(); + #[cfg(not(Py_GIL_DISABLED))] + let cell = self.initializing_threads.get(py); + #[cfg(Py_GIL_DISABLED)] + let cell = self.initializing_threads.lock().unwrap(); + let mut threads = cell.borrow_mut(); if threads.contains(&thread_id) { // Reentrant call: just return the type object, even if the // `tp_dict` is not filled yet. @@ -127,19 +143,28 @@ impl LazyTypeObjectInner { } struct InitializationGuard<'a> { + #[cfg(not(Py_GIL_DISABLED))] initializing_threads: &'a GILProtected>>, + #[cfg(Py_GIL_DISABLED)] + initializing_threads: &'a Mutex>>, + #[cfg(not(Py_GIL_DISABLED))] py: Python<'a>, thread_id: ThreadId, } impl Drop for InitializationGuard<'_> { fn drop(&mut self) { - let mut threads = self.initializing_threads.get(self.py).borrow_mut(); + #[cfg(not(Py_GIL_DISABLED))] + let cell = self.initializing_threads.get(self.py); + #[cfg(Py_GIL_DISABLED)] + let cell = self.initializing_threads.lock().unwrap(); + let mut threads = cell.borrow_mut(); threads.retain(|id| *id != self.thread_id); } } let guard = InitializationGuard { initializing_threads: &self.initializing_threads, + #[cfg(not(Py_GIL_DISABLED))] py, thread_id, }; @@ -185,8 +210,17 @@ impl LazyTypeObjectInner { // Initialization successfully complete, can clear the thread list. // (No further calls to get_or_init() will try to init, on any thread.) - std::mem::forget(guard); - self.initializing_threads.get(py).replace(Vec::new()); + #[cfg(not(Py_GIL_DISABLED))] + let cell = { + std::mem::forget(guard); + self.initializing_threads.get(py) + }; + #[cfg(Py_GIL_DISABLED)] + let cell = { + drop(guard); + self.initializing_threads.lock().unwrap() + }; + cell.replace(Vec::new()); result }); diff --git a/src/sync.rs b/src/sync.rs index c781755c067..7a84f35b051 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -6,10 +6,13 @@ //! [PEP 703]: https://peps.python.org/pep-703/ use crate::{ types::{any::PyAnyMethods, PyString, PyType}, - Bound, Py, PyResult, PyVisit, Python, + Bound, Py, PyResult, Python, }; use std::cell::UnsafeCell; +#[cfg(not(Py_GIL_DISABLED))] +use crate::PyVisit; + /// Value with concurrent access protected by the GIL. /// /// This is a synchronization primitive based on Python's global interpreter lock (GIL). @@ -31,10 +34,12 @@ use std::cell::UnsafeCell; /// NUMBERS.get(py).borrow_mut().push(42); /// }); /// ``` +#[cfg(not(Py_GIL_DISABLED))] pub struct GILProtected { value: T, } +#[cfg(not(Py_GIL_DISABLED))] impl GILProtected { /// Place the given value under the protection of the GIL. pub const fn new(value: T) -> Self { @@ -52,6 +57,7 @@ impl GILProtected { } } +#[cfg(not(Py_GIL_DISABLED))] unsafe impl Sync for GILProtected where T: Send {} /// A write-once cell similar to [`once_cell::OnceCell`](https://docs.rs/once_cell/latest/once_cell/). From 076762111fbd18b0b99ebd57c52cc1df00eebb1c Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 30 Aug 2024 12:52:06 -0600 Subject: [PATCH 2/7] add stub for migration guide on free-threaded support --- guide/src/migration.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/guide/src/migration.md b/guide/src/migration.md index b1dfc9e30c4..8292e0054a1 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -198,6 +198,27 @@ impl<'a, 'py> IntoPyObject<'py> for &'a MyPyObjectWrapper { ``` +### Free-threaded Python Support +
+Click to expand + +PyO3 0.23 introduces preliminary support for the new free-threaded build of +CPython 3.13. PyO3 features that implicitly assumed the existence of the GIL +are not exposed in the free-threaded build, since they are no longer safe. + +If you make use of these features then you will need to use conditional +compilation to replace them for extensions built for free-threaded +Python. Extensions built for the free-threaded build will have the +`Py_GIL_DISABLED` attribute defined. + +### `GILProtected` + +`GILProtected` allows mutable access to static data by leveraging the GIL to +lock concurrent access from other threads. In free-threaded python there is no +GIL, so you will need to replace this type with some other form of locking. + +
+ ## from 0.21.* to 0.22 ### Deprecation of `gil-refs` feature continues From ac7b2f1c0e26ae94ee0a6cb7bdc6fcd3691d41a0 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 2 Sep 2024 11:09:08 -0600 Subject: [PATCH 3/7] remove internal uses of GILProtected on gil-enabled builds as well --- src/impl_/pyclass/lazy_type_object.rs | 46 ++++----------------------- 1 file changed, 7 insertions(+), 39 deletions(-) diff --git a/src/impl_/pyclass/lazy_type_object.rs b/src/impl_/pyclass/lazy_type_object.rs index 373e8134919..d3bede7b2f3 100644 --- a/src/impl_/pyclass/lazy_type_object.rs +++ b/src/impl_/pyclass/lazy_type_object.rs @@ -1,5 +1,4 @@ use std::{ - cell::RefCell, ffi::CStr, marker::PhantomData, thread::{self, ThreadId}, @@ -16,10 +15,6 @@ use crate::{ Bound, PyClass, PyErr, PyObject, PyResult, Python, }; -#[cfg(not(Py_GIL_DISABLED))] -use crate::sync::GILProtected; - -#[cfg(Py_GIL_DISABLED)] use std::sync::Mutex; use super::PyClassItemsIter; @@ -33,10 +28,7 @@ struct LazyTypeObjectInner { value: GILOnceCell, // Threads which have begun initialization of the `tp_dict`. Used for // reentrant initialization detection. - #[cfg(not(Py_GIL_DISABLED))] - initializing_threads: GILProtected>>, - #[cfg(Py_GIL_DISABLED)] - initializing_threads: Mutex>>, + initializing_threads: Mutex>, tp_dict_filled: GILOnceCell<()>, } @@ -47,10 +39,7 @@ impl LazyTypeObject { LazyTypeObject( LazyTypeObjectInner { value: GILOnceCell::new(), - #[cfg(not(Py_GIL_DISABLED))] - initializing_threads: GILProtected::new(RefCell::new(Vec::new())), - #[cfg(Py_GIL_DISABLED)] - initializing_threads: Mutex::new(RefCell::new(Vec::new())), + initializing_threads: Mutex::new(Vec::new()), tp_dict_filled: GILOnceCell::new(), }, PhantomData, @@ -129,11 +118,7 @@ impl LazyTypeObjectInner { let thread_id = thread::current().id(); { - #[cfg(not(Py_GIL_DISABLED))] - let cell = self.initializing_threads.get(py); - #[cfg(Py_GIL_DISABLED)] - let cell = self.initializing_threads.lock().unwrap(); - let mut threads = cell.borrow_mut(); + let mut threads = self.initializing_threads.lock().unwrap(); if threads.contains(&thread_id) { // Reentrant call: just return the type object, even if the // `tp_dict` is not filled yet. @@ -143,29 +128,18 @@ impl LazyTypeObjectInner { } struct InitializationGuard<'a> { - #[cfg(not(Py_GIL_DISABLED))] - initializing_threads: &'a GILProtected>>, - #[cfg(Py_GIL_DISABLED)] - initializing_threads: &'a Mutex>>, - #[cfg(not(Py_GIL_DISABLED))] - py: Python<'a>, + initializing_threads: &'a Mutex>, thread_id: ThreadId, } impl Drop for InitializationGuard<'_> { fn drop(&mut self) { - #[cfg(not(Py_GIL_DISABLED))] - let cell = self.initializing_threads.get(self.py); - #[cfg(Py_GIL_DISABLED)] - let cell = self.initializing_threads.lock().unwrap(); - let mut threads = cell.borrow_mut(); + let mut threads = self.initializing_threads.lock().unwrap(); threads.retain(|id| *id != self.thread_id); } } let guard = InitializationGuard { initializing_threads: &self.initializing_threads, - #[cfg(not(Py_GIL_DISABLED))] - py, thread_id, }; @@ -210,17 +184,11 @@ impl LazyTypeObjectInner { // Initialization successfully complete, can clear the thread list. // (No further calls to get_or_init() will try to init, on any thread.) - #[cfg(not(Py_GIL_DISABLED))] - let cell = { - std::mem::forget(guard); - self.initializing_threads.get(py) - }; - #[cfg(Py_GIL_DISABLED)] - let cell = { + let mut threads = { drop(guard); self.initializing_threads.lock().unwrap() }; - cell.replace(Vec::new()); + threads.clear(); result }); From 5351baac91eba71fba447640deec55639be2d61e Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 2 Sep 2024 11:12:37 -0600 Subject: [PATCH 4/7] add newsfragment --- newsfragments/4504.changed.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 newsfragments/4504.changed.md diff --git a/newsfragments/4504.changed.md b/newsfragments/4504.changed.md new file mode 100644 index 00000000000..94d056dcef9 --- /dev/null +++ b/newsfragments/4504.changed.md @@ -0,0 +1,2 @@ +* The `GILProtected` struct is not available on the free-threaded build of + Python 3.13. From e33212c8557b0087b347be48ea4eb9926141db7d Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 2 Sep 2024 11:46:21 -0600 Subject: [PATCH 5/7] flesh out documentation --- guide/src/migration.md | 66 ++++++++++++++++++++++++++++++++++++++---- src/sync.rs | 2 ++ 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/guide/src/migration.md b/guide/src/migration.md index 8292e0054a1..dfc9bd4e406 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -206,16 +206,72 @@ PyO3 0.23 introduces preliminary support for the new free-threaded build of CPython 3.13. PyO3 features that implicitly assumed the existence of the GIL are not exposed in the free-threaded build, since they are no longer safe. -If you make use of these features then you will need to use conditional -compilation to replace them for extensions built for free-threaded -Python. Extensions built for the free-threaded build will have the -`Py_GIL_DISABLED` attribute defined. +If you make use of these features then you will need to account for the +unavailability of this API in the free-threaded build. One way to handle it is +via conditional compilation -- extensions built for the free-threaded build will +have the `Py_GIL_DISABLED` attribute defined. ### `GILProtected` `GILProtected` allows mutable access to static data by leveraging the GIL to lock concurrent access from other threads. In free-threaded python there is no -GIL, so you will need to replace this type with some other form of locking. +GIL, so you will need to replace this type with some other form of locking. In +many cases, `std::sync::Atomic` or `std::sync::Mutex` will be sufficient. If the +locks do 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. + +Before: + +```rust,ignore +# use pyo3::prelude::*; +use pyo3::sync::GILProtected; +use pyo3::types::{PyDict, PyNone}; +use std::cell::RefCell; + +static OBJECTS: GILProtected>>> = + GILProtected::new(RefCell::new(Vec::new())); + +fn main() { + Python::with_gil(|py: Python| { + let d = PyDict::new(py); + // stand-in for something that executes arbitrary python code + d.set_item(PyNone::get(py), PyNone::get(py)).unwrap(); + OBJECTS.get(py).borrow_mut().push(d.unbind()); + }); +} +``` + +After: + +```rust +use pyo3::prelude::*; +#[cfg(not(Py_GIL_DISABLED))] +use pyo3::sync::GILProtected; +use pyo3::types::{PyDict, PyNone}; +#[cfg(not(Py_GIL_DISABLED))] +use std::cell::RefCell; +#[cfg(Py_GIL_DISABLED)] +use std::sync::Mutex; + +#[cfg(not(Py_GIL_DISABLED))] +static OBJECTS: GILProtected>>> = + GILProtected::new(RefCell::new(Vec::new())); +#[cfg(Py_GIL_DISABLED)] +static OBJECTS: Mutex>> = Mutex::new(Vec::new()); + +fn main() { + Python::with_gil(|py| { + let d = PyDict::new(py); + // stand-in for something that executes arbitrary python code + d.set_item(PyNone::get(py), PyNone::get(py)).unwrap(); + #[cfg(not(Py_GIL_DISABLED))] + OBJECTS.get(py).borrow_mut().push(d.unbind()); + #[cfg(Py_GIL_DISABLED)] + OBJECTS.lock().unwrap().push(d.unbind()); + }); +} +``` diff --git a/src/sync.rs b/src/sync.rs index 7a84f35b051..c7122a45976 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -19,6 +19,8 @@ use crate::PyVisit; /// It ensures that only one thread at a time can access the inner value via shared references. /// It can be combined with interior mutability to obtain mutable references. /// +/// This type is not defined for extensions built against the free-threaded CPython ABI. +/// /// # Example /// /// Combining `GILProtected` with `RefCell` enables mutable access to static data: From c1db75d8e24b8f94630313184ecf6f87c92a42ed Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 3 Sep 2024 09:50:42 -0600 Subject: [PATCH 6/7] fixup migration guide examples --- guide/src/migration.md | 43 +++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/guide/src/migration.md b/guide/src/migration.md index dfc9bd4e406..b9de0da78a3 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -223,7 +223,9 @@ was not needed in the first place. Before: -```rust,ignore +```rust +# fn main() { +# #[cfg(not(Py_GIL_DISABLED))] { # use pyo3::prelude::*; use pyo3::sync::GILProtected; use pyo3::types::{PyDict, PyNone}; @@ -232,20 +234,20 @@ use std::cell::RefCell; static OBJECTS: GILProtected>>> = GILProtected::new(RefCell::new(Vec::new())); -fn main() { - Python::with_gil(|py: Python| { - let d = PyDict::new(py); - // stand-in for something that executes arbitrary python code - d.set_item(PyNone::get(py), PyNone::get(py)).unwrap(); - OBJECTS.get(py).borrow_mut().push(d.unbind()); - }); -} +Python::with_gil(|py| { + // stand-in for something that executes arbitrary python code + let d = PyDict::new(py); + d.set_item(PyNone::get(py), PyNone::get(py)).unwrap(); + OBJECTS.get(py).borrow_mut().push(d.unbind()); +}); +# }} ``` After: ```rust -use pyo3::prelude::*; +# use pyo3::prelude::*; +# fn main() { #[cfg(not(Py_GIL_DISABLED))] use pyo3::sync::GILProtected; use pyo3::types::{PyDict, PyNone}; @@ -260,17 +262,16 @@ static OBJECTS: GILProtected>>> = #[cfg(Py_GIL_DISABLED)] static OBJECTS: Mutex>> = Mutex::new(Vec::new()); -fn main() { - Python::with_gil(|py| { - let d = PyDict::new(py); - // stand-in for something that executes arbitrary python code - d.set_item(PyNone::get(py), PyNone::get(py)).unwrap(); - #[cfg(not(Py_GIL_DISABLED))] - OBJECTS.get(py).borrow_mut().push(d.unbind()); - #[cfg(Py_GIL_DISABLED)] - OBJECTS.lock().unwrap().push(d.unbind()); - }); -} +Python::with_gil(|py| { + // stand-in for something that executes arbitrary python code + let d = PyDict::new(py); + d.set_item(PyNone::get(py), PyNone::get(py)).unwrap(); + #[cfg(not(Py_GIL_DISABLED))] + OBJECTS.get(py).borrow_mut().push(d.unbind()); + #[cfg(Py_GIL_DISABLED)] + OBJECTS.lock().unwrap().push(d.unbind()); +}); +# } ``` From af8013425fe461a7ff2bac9ec96aef4c08ad5425 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 3 Sep 2024 10:15:47 -0600 Subject: [PATCH 7/7] simplify migration guide example --- guide/src/migration.md | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/guide/src/migration.md b/guide/src/migration.md index b9de0da78a3..4697e2d1d65 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -248,32 +248,28 @@ After: ```rust # use pyo3::prelude::*; # fn main() { -#[cfg(not(Py_GIL_DISABLED))] -use pyo3::sync::GILProtected; use pyo3::types::{PyDict, PyNone}; -#[cfg(not(Py_GIL_DISABLED))] -use std::cell::RefCell; -#[cfg(Py_GIL_DISABLED)] use std::sync::Mutex; -#[cfg(not(Py_GIL_DISABLED))] -static OBJECTS: GILProtected>>> = - GILProtected::new(RefCell::new(Vec::new())); -#[cfg(Py_GIL_DISABLED)] static OBJECTS: Mutex>> = Mutex::new(Vec::new()); Python::with_gil(|py| { // stand-in for something that executes arbitrary python code let d = PyDict::new(py); d.set_item(PyNone::get(py), PyNone::get(py)).unwrap(); - #[cfg(not(Py_GIL_DISABLED))] - OBJECTS.get(py).borrow_mut().push(d.unbind()); - #[cfg(Py_GIL_DISABLED)] + // we're not executing python code while holding the lock, so GILProtected + // was never needed OBJECTS.lock().unwrap().push(d.unbind()); }); # } ``` +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. Python 3.13 introduces `PyMutex`, which releases +the GIL while the lock is held, so that is another option if you only need to +support newer Python versions. + ## from 0.21.* to 0.22