Skip to content

Commit d74fadc

Browse files
Implement OnceExt & MutexExt for parking_lot & lock_api (#5044)
* Implement `OnceExt` & `MutexExt` for `parking_lot` & `lock_api` * Implement `MutexExt` for `Arc<lock_api::Mutex>` * Move `MutexExt` lock result into type parameter * Remove lifetime from associated type of `OnceExt` * Add docs * add changelog * Add new features to `full` * mention `parking_lot` in guide * Add tests * Update docs to mention all cargo features Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * Also test `parking_lot::Mutex::lock_py_attached` on `lock_api` feature * Update newsfragments/5044.added.md Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * Add additional case to `once_ext` test * enable `arc_lock` feature on `parking_lot` during testing --------- Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
1 parent f87b286 commit d74fadc

File tree

6 files changed

+199
-35
lines changed

6 files changed

+199
-35
lines changed

Cargo.toml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ time = { version = "0.3.38", default-features = false, optional = true }
4949
serde = { version = "1.0", optional = true }
5050
smallvec = { version = "1.0", optional = true }
5151
uuid = { version = "1.11.0", optional = true }
52+
lock_api = { version = "0.4", optional = true }
53+
parking_lot = { version = "0.12", optional = true}
5254

5355
[target.'cfg(not(target_has_atomic = "64"))'.dependencies]
5456
portable-atomic = "1.0"
@@ -68,6 +70,7 @@ futures = "0.3.28"
6870
tempfile = "3.12.0"
6971
static_assertions = "1.1.0"
7072
uuid = { version = "1.10.0", features = ["v4"] }
73+
parking_lot = { version = "0.12.3", features = ["arc_lock"]}
7174

7275
[build-dependencies]
7376
pyo3-build-config = { path = "pyo3-build-config", version = "=0.25.0-dev", features = ["resolve-config"] }
@@ -115,6 +118,9 @@ auto-initialize = []
115118
# Enables `Clone`ing references to Python objects `Py<T>` which panics if the GIL is not held.
116119
py-clone = []
117120

121+
parking_lot = ["dep:parking_lot", "lock_api"]
122+
arc_lock = ["lock_api", "lock_api/arc_lock", "parking_lot?/arc_lock"]
123+
118124
# Optimizes PyObject to Vec conversion and so on.
119125
nightly = []
120126

@@ -124,6 +130,7 @@ full = [
124130
"macros",
125131
# "multiple-pymethods", # Not supported by wasm
126132
"anyhow",
133+
"arc_lock",
127134
"bigdecimal",
128135
"chrono",
129136
"chrono-tz",
@@ -133,10 +140,12 @@ full = [
133140
"eyre",
134141
"hashbrown",
135142
"indexmap",
143+
"lock_api",
136144
"num-bigint",
137145
"num-complex",
138146
"num-rational",
139147
"ordered-float",
148+
"parking_lot",
140149
"py-clone",
141150
"rust_decimal",
142151
"serde",

guide/src/class/thread-safety.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ impl MyClass {
101101
}
102102
```
103103

104-
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.
104+
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. Additionally, support for the `parking_lot` and `lock_api` synchronization libraries is gated behind the `parking_lot` and `lock_api` features. You can also enable the `arc_lock` feature if you need the `arc_lock` features of either library.
105105

106106
### Wrapping unsynchronized data
107107

guide/src/features.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ These features enable conversions between Python types and types from other Rust
121121

122122
Adds a dependency on [anyhow](https://docs.rs/anyhow). Enables a conversion from [anyhow](https://docs.rs/anyhow)’s [`Error`](https://docs.rs/anyhow/latest/anyhow/struct.Error.html) type to [`PyErr`]({{#PYO3_DOCS_URL}}/pyo3/struct.PyErr.html), for easy error handling.
123123

124+
### `arc_lock`
125+
126+
Enables Pyo3's `MutexExt` trait for all Mutexes that extend on [`lock_api::Mutex`](https://docs.rs/lock_api/latest/lock_api/struct.Mutex.html) and are wrapped in an [`Arc`](https://doc.rust-lang.org/std/sync/struct.Arc.html) type. Like [`Arc<parking_lot::Mutex>`](https://docs.rs/parking_lot/latest/parking_lot/type.Mutex.html#method.lock_arc)
127+
124128
### `bigdecimal`
125129
Adds a dependency on [bigdecimal](https://docs.rs/bigdecimal) and enables conversions into its [`BigDecimal`](https://docs.rs/bigdecimal/latest/bigdecimal/struct.BigDecimal.html) type.
126130

@@ -168,6 +172,10 @@ Adds a dependency on [jiff@0.2](https://docs.rs/jiff/0.2) and requires MSRV 1.70
168172
- [Zoned](https://docs.rs/jiff/0.2/jiff/struct.Zoned.html) -> [`PyDateTime`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyDateTime.html)
169173
- [Timestamp](https://docs.rs/jiff/0.2/jiff/struct.Timestamp.html) -> [`PyDateTime`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyDateTime.html)
170174

175+
### `lock_api`
176+
177+
Adds a dependency on [lock_api](https://docs.rs/lock_api) and enables Pyo3's `MutexExt` trait for all mutexes that extend on [`lock_api::Mutex`](https://docs.rs/lock_api/latest/lock_api/struct.Mutex.html) (like `parking_lot` or `spin`).
178+
171179
### `num-bigint`
172180

173181
Adds a dependency on [num-bigint](https://docs.rs/num-bigint) and enables conversions into its [`BigInt`](https://docs.rs/num-bigint/latest/num_bigint/struct.BigInt.html) and [`BigUint`](https://docs.rs/num-bigint/latest/num_bigint/struct.BigUint.html) types.
@@ -186,6 +194,10 @@ Adds a dependency on [ordered-float](https://docs.rs/ordered-float) and enables
186194
- [NotNan](https://docs.rs/ordered-float/latest/ordered_float/struct.NotNan.html) -> [`PyFloat`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyFloat.html)
187195
- [OrderedFloat](https://docs.rs/ordered-float/latest/ordered_float/struct.OrderedFloat.html) -> [`PyFloat`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyFloat.html)
188196

197+
### `parking-lot`
198+
199+
Adds a dependency on [parking_lot](https://docs.rs/parking_lot) and enables Pyo3's `OnceExt` & `MutexExt` traits for [`parking_lot::Once`](https://docs.rs/parking_lot/latest/parking_lot/struct.Once.html) and [`parking_lot::Mutex`](https://docs.rs/parking_lot/latest/parking_lot/type.Mutex.html) types.
200+
189201
### `rust_decimal`
190202

191203
Adds a dependency on [rust_decimal](https://docs.rs/rust_decimal) and enables conversions into its [`Decimal`](https://docs.rs/rust_decimal/latest/rust_decimal/struct.Decimal.html) type.

newsfragments/5044.added.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Implement `OnceExt` & `MutexExt` for `parking_lot` & `lock_api`. Use the new extension traits by enabling the `arc_lock`, `lock_api`, or `parking_lot` cargo features.

src/sealed.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,9 @@ impl<T: crate::pyclass::PyClass> Sealed for PyClassInitializer<T> {}
5757

5858
impl Sealed for std::sync::Once {}
5959
impl<T> Sealed for std::sync::Mutex<T> {}
60+
#[cfg(feature = "lock_api")]
61+
impl<R, T> Sealed for lock_api::Mutex<R, T> {}
62+
#[cfg(feature = "parking_lot")]
63+
impl Sealed for parking_lot::Once {}
64+
#[cfg(feature = "arc_lock")]
65+
impl<R: lock_api::RawMutex, T> Sealed for std::sync::Arc<lock_api::Mutex<R, T>> {}

src/sync.rs

Lines changed: 170 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -536,13 +536,16 @@ mod once_lock_ext_sealed {
536536
/// Helper trait for `Once` to help avoid deadlocking when using a `Once` when attached to a
537537
/// Python thread.
538538
pub trait OnceExt: Sealed {
539+
///The state of `Once`
540+
type OnceState;
541+
539542
/// Similar to [`call_once`][Once::call_once], but releases the Python GIL temporarily
540543
/// if blocking on another thread currently calling this `Once`.
541544
fn call_once_py_attached(&self, py: Python<'_>, f: impl FnOnce());
542545

543546
/// Similar to [`call_once_force`][Once::call_once_force], but releases the Python GIL
544547
/// temporarily if blocking on another thread currently calling this `Once`.
545-
fn call_once_force_py_attached(&self, py: Python<'_>, f: impl FnOnce(&OnceState));
548+
fn call_once_force_py_attached(&self, py: Python<'_>, f: impl FnOnce(&Self::OnceState));
546549
}
547550

548551
/// Extension trait for [`std::sync::OnceLock`] which helps avoid deadlocks between the Python
@@ -565,21 +568,23 @@ pub trait OnceLockExt<T>: once_lock_ext_sealed::Sealed {
565568

566569
/// Extension trait for [`std::sync::Mutex`] which helps avoid deadlocks between
567570
/// the Python interpreter and acquiring the `Mutex`.
568-
pub trait MutexExt<T>: Sealed {
571+
pub trait MutexExt<'a, T, R>: Sealed
572+
where
573+
Self: 'a,
574+
{
569575
/// Lock this `Mutex` in a manner that cannot deadlock with the Python interpreter.
570576
///
571577
/// Before attempting to lock the mutex, this function detaches from the
572578
/// Python runtime. When the lock is acquired, it re-attaches to the Python
573579
/// runtime before returning the `LockResult`. This avoids deadlocks between
574580
/// the GIL and other global synchronization events triggered by the Python
575581
/// interpreter.
576-
fn lock_py_attached(
577-
&self,
578-
py: Python<'_>,
579-
) -> std::sync::LockResult<std::sync::MutexGuard<'_, T>>;
582+
fn lock_py_attached(&'a self, py: Python<'_>) -> R;
580583
}
581584

582585
impl OnceExt for Once {
586+
type OnceState = OnceState;
587+
583588
fn call_once_py_attached(&self, py: Python<'_>, f: impl FnOnce()) {
584589
if self.is_completed() {
585590
return;
@@ -597,6 +602,41 @@ impl OnceExt for Once {
597602
}
598603
}
599604

605+
#[cfg(feature = "parking_lot")]
606+
impl OnceExt for parking_lot::Once {
607+
type OnceState = parking_lot::OnceState;
608+
609+
fn call_once_py_attached(&self, _py: Python<'_>, f: impl FnOnce()) {
610+
if self.state().done() {
611+
return;
612+
}
613+
614+
let ts_guard = unsafe { SuspendGIL::new() };
615+
616+
self.call_once(move || {
617+
drop(ts_guard);
618+
f();
619+
});
620+
}
621+
622+
fn call_once_force_py_attached(
623+
&self,
624+
_py: Python<'_>,
625+
f: impl FnOnce(&parking_lot::OnceState),
626+
) {
627+
if self.state().done() {
628+
return;
629+
}
630+
631+
let ts_guard = unsafe { SuspendGIL::new() };
632+
633+
self.call_once_force(move |state| {
634+
drop(ts_guard);
635+
f(&state);
636+
});
637+
}
638+
}
639+
600640
#[cfg(rustc_has_once_lock)]
601641
impl<T> OnceLockExt<T> for std::sync::OnceLock<T> {
602642
fn get_or_init_py_attached<F>(&self, py: Python<'_>, f: F) -> &T
@@ -612,11 +652,13 @@ impl<T> OnceLockExt<T> for std::sync::OnceLock<T> {
612652
}
613653
}
614654

615-
impl<T> MutexExt<T> for std::sync::Mutex<T> {
655+
impl<'a, T> MutexExt<'a, T, std::sync::LockResult<std::sync::MutexGuard<'a, T>>>
656+
for std::sync::Mutex<T>
657+
{
616658
fn lock_py_attached(
617-
&self,
659+
&'a self,
618660
_py: Python<'_>,
619-
) -> std::sync::LockResult<std::sync::MutexGuard<'_, T>> {
661+
) -> std::sync::LockResult<std::sync::MutexGuard<'a, T>> {
620662
// If try_lock is successful or returns a poisoned mutex, return them so
621663
// the caller can deal with them. Otherwise we need to use blocking
622664
// lock, which requires detaching from the Python runtime to avoid
@@ -638,6 +680,41 @@ impl<T> MutexExt<T> for std::sync::Mutex<T> {
638680
}
639681
}
640682

683+
#[cfg(feature = "lock_api")]
684+
impl<'a, R: lock_api::RawMutex, T> MutexExt<'a, T, lock_api::MutexGuard<'a, R, T>>
685+
for lock_api::Mutex<R, T>
686+
{
687+
fn lock_py_attached(&'a self, _py: Python<'_>) -> lock_api::MutexGuard<'a, R, T> {
688+
if let Some(guard) = self.try_lock() {
689+
return guard;
690+
}
691+
692+
let ts_guard = unsafe { SuspendGIL::new() };
693+
let res = self.lock();
694+
drop(ts_guard);
695+
res
696+
}
697+
}
698+
699+
#[cfg(feature = "arc_lock")]
700+
impl<'a, R, T> MutexExt<'a, T, lock_api::ArcMutexGuard<R, T>>
701+
for std::sync::Arc<lock_api::Mutex<R, T>>
702+
where
703+
R: lock_api::RawMutex,
704+
Self: 'a,
705+
{
706+
fn lock_py_attached(&self, _py: Python<'_>) -> lock_api::ArcMutexGuard<R, T> {
707+
if let Some(guard) = self.try_lock_arc() {
708+
return guard;
709+
}
710+
711+
let ts_guard = unsafe { SuspendGIL::new() };
712+
let res = self.lock_arc();
713+
drop(ts_guard);
714+
res
715+
}
716+
}
717+
641718
#[cold]
642719
fn init_once_py_attached<F, T>(once: &Once, _py: Python<'_>, f: F)
643720
where
@@ -966,36 +1043,47 @@ mod tests {
9661043
#[test]
9671044
#[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled
9681045
fn test_once_ext() {
969-
// adapted from the example in the docs for Once::try_once_force
970-
let init = Once::new();
971-
std::thread::scope(|s| {
972-
// poison the once
973-
let handle = s.spawn(|| {
974-
Python::with_gil(|py| {
975-
init.call_once_py_attached(py, || panic!());
976-
})
977-
});
978-
assert!(handle.join().is_err());
1046+
macro_rules! test_once {
1047+
($once:expr, $is_poisoned:expr) => {{
1048+
// adapted from the example in the docs for Once::try_once_force
1049+
let init = $once;
1050+
std::thread::scope(|s| {
1051+
// poison the once
1052+
let handle = s.spawn(|| {
1053+
Python::with_gil(|py| {
1054+
init.call_once_py_attached(py, || panic!());
1055+
})
1056+
});
1057+
assert!(handle.join().is_err());
9791058

980-
// poisoning propagates
981-
let handle = s.spawn(|| {
982-
Python::with_gil(|py| {
983-
init.call_once_py_attached(py, || {});
984-
});
985-
});
1059+
// poisoning propagates
1060+
let handle = s.spawn(|| {
1061+
Python::with_gil(|py| {
1062+
init.call_once_py_attached(py, || {});
1063+
});
1064+
});
1065+
1066+
assert!(handle.join().is_err());
9861067

987-
assert!(handle.join().is_err());
1068+
// call_once_force will still run and reset the poisoned state
1069+
Python::with_gil(|py| {
1070+
init.call_once_force_py_attached(py, |state| {
1071+
assert!($is_poisoned(state.clone()));
1072+
});
9881073

989-
// call_once_force will still run and reset the poisoned state
990-
Python::with_gil(|py| {
991-
init.call_once_force_py_attached(py, |state| {
992-
assert!(state.is_poisoned());
1074+
// once any success happens, we stop propagating the poison
1075+
init.call_once_py_attached(py, || {});
1076+
});
1077+
1078+
// calling call_once_force should return immediately without calling the closure
1079+
Python::with_gil(|py| init.call_once_force_py_attached(py, |_| panic!()));
9931080
});
1081+
}};
1082+
}
9941083

995-
// once any success happens, we stop propagating the poison
996-
init.call_once_py_attached(py, || {});
997-
});
998-
});
1084+
test_once!(Once::new(), OnceState::is_poisoned);
1085+
#[cfg(feature = "parking_lot")]
1086+
test_once!(parking_lot::Once::new(), parking_lot::OnceState::poisoned);
9991087
}
10001088

10011089
#[cfg(rustc_has_once_lock)]
@@ -1047,6 +1135,54 @@ mod tests {
10471135
});
10481136
}
10491137

1138+
#[cfg(feature = "macros")]
1139+
#[cfg(all(
1140+
any(feature = "parking_lot", feature = "lock_api"),
1141+
not(target_arch = "wasm32") // We are building wasm Python with pthreads disabled
1142+
))]
1143+
#[test]
1144+
fn test_parking_lot_mutex_ext() {
1145+
macro_rules! test_mutex {
1146+
($guard:ty ,$mutex:stmt) => {{
1147+
let barrier = Barrier::new(2);
1148+
1149+
let mutex = Python::with_gil({ $mutex });
1150+
1151+
std::thread::scope(|s| {
1152+
s.spawn(|| {
1153+
Python::with_gil(|py| {
1154+
let b: $guard = mutex.lock_py_attached(py);
1155+
barrier.wait();
1156+
// sleep to ensure the other thread actually blocks
1157+
std::thread::sleep(std::time::Duration::from_millis(10));
1158+
(*b).bind(py).borrow().0.store(true, Ordering::Release);
1159+
drop(b);
1160+
});
1161+
});
1162+
s.spawn(|| {
1163+
barrier.wait();
1164+
Python::with_gil(|py| {
1165+
// blocks until the other thread releases the lock
1166+
let b: $guard = mutex.lock_py_attached(py);
1167+
assert!((*b).bind(py).borrow().0.load(Ordering::Acquire));
1168+
});
1169+
});
1170+
});
1171+
}};
1172+
}
1173+
1174+
test_mutex!(parking_lot::MutexGuard<'_, _>, |py| {
1175+
parking_lot::Mutex::new(Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap())
1176+
});
1177+
1178+
#[cfg(feature = "arc_lock")]
1179+
test_mutex!(parking_lot::ArcMutexGuard<_, _>, |py| {
1180+
let mutex =
1181+
parking_lot::Mutex::new(Py::new(py, BoolWrapper(AtomicBool::new(false))).unwrap());
1182+
std::sync::Arc::new(mutex)
1183+
});
1184+
}
1185+
10501186
#[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled
10511187
#[test]
10521188
fn test_mutex_ext_poison() {

0 commit comments

Comments
 (0)