-
Notifications
You must be signed in to change notification settings - Fork 763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove GILProtected on free-threaded build #4504
Changes from 5 commits
056857a
0767621
ac7b2f1
5351baa
e33212c
c1db75d
af80134
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -198,6 +198,83 @@ impl<'a, 'py> IntoPyObject<'py> for &'a MyPyObjectWrapper { | |
``` | ||
</details> | ||
|
||
### Free-threaded Python Support | ||
<details open> | ||
<summary><small>Click to expand</small></summary> | ||
|
||
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 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. 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 | ||
ngoldbaum marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# use pyo3::prelude::*; | ||
use pyo3::sync::GILProtected; | ||
use pyo3::types::{PyDict, PyNone}; | ||
use std::cell::RefCell; | ||
|
||
static OBJECTS: GILProtected<RefCell<Vec<Py<PyDict>>>> = | ||
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<RefCell<Vec<Py<PyDict>>>> = | ||
GILProtected::new(RefCell::new(Vec::new())); | ||
#[cfg(Py_GIL_DISABLED)] | ||
static OBJECTS: Mutex<Vec<Py<PyDict>>> = 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()); | ||
}); | ||
} | ||
``` | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll expand this to include a discussion of how to handle executing arbitrary python code while you hold the global lock by leveraging |
||
</details> | ||
|
||
## from 0.21.* to 0.22 | ||
|
||
### Deprecation of `gil-refs` feature continues | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
* The `GILProtected` struct is not available on the free-threaded build of | ||
Python 3.13. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,16 +6,21 @@ | |
//! [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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should document that this type is unavailable on the freethreaded build and recommend use of atomics or There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’ll take a look at adding and documenting safe wrappers for |
||
/// | ||
/// This is a synchronization primitive based on Python's global interpreter lock (GIL). | ||
/// 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: | ||
|
@@ -31,10 +36,12 @@ use std::cell::UnsafeCell; | |
/// NUMBERS.get(py).borrow_mut().push(42); | ||
/// }); | ||
/// ``` | ||
#[cfg(not(Py_GIL_DISABLED))] | ||
ngoldbaum marked this conversation as resolved.
Show resolved
Hide resolved
|
||
pub struct GILProtected<T> { | ||
value: T, | ||
} | ||
|
||
#[cfg(not(Py_GIL_DISABLED))] | ||
impl<T> GILProtected<T> { | ||
/// Place the given value under the protection of the GIL. | ||
pub const fn new(value: T) -> Self { | ||
|
@@ -52,6 +59,7 @@ impl<T> GILProtected<T> { | |
} | ||
} | ||
|
||
#[cfg(not(Py_GIL_DISABLED))] | ||
unsafe impl<T> Sync for GILProtected<T> where T: Send {} | ||
|
||
/// A write-once cell similar to [`once_cell::OnceCell`](https://docs.rs/once_cell/latest/once_cell/). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the end is
GILProtected
the only feature we're hiding? It looks like #4512 leavesGILOnceCell
exposed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm considering #4512 and #4513 as possible ideas, would welcome your thoughts on those two options. It seems like migrating off
GILOnceCell
is desired in the long term, but I'd prefer not force users to do that until we can support module state...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it's still up in the air I'll leave this language as-is. We can massage it before the 0.23 release (which I think we're getting close to!).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. I also think at some point before the release we should move some of this content into a more permanent, dedicated guide page and then make this migration entry link there. This content will be relevant going forward, not just for those upgrading.