lock: add an opt-in rwlock that detaches before it blocks · RustPython/RustPython@72d9243 · GitHub

RustPython / RustPython Public

• Notifications You must be signed in to change notification settings
• Fork 1.5k
• Star 22.3k

• Code
• Issues 295
• Pull requests 103
• Discussions
• Actions
• Projects
• Wiki
• Security and quality
• Insights

Additional navigation options

Commit 72d9243

Browse filesBrowse the repository at this point in the historyBrowse files

youknowone

committed

lock: add an opt-in rwlock that detaches before it blocks

A thread blocked acquiring a lock reaches no safepoint, so stop-the-world cannot stop it, and the lock it waits for is routinely one a thread the requester already suspended is holding. `RawDetachingRwLock` wraps the raw rwlock and hands the wait for a contended acquire to a hook that leaves the interpreter first; an acquire that takes the lock on its first try does not reach the hook. The vm installs the hook during interpreter init and implements it with `allow_threads`. The wait ends with the lock acquired while detached, so re-attaching can park the thread holding it. That is only safe where nothing reachable from a stop-the-world section takes the same lock, so it is opt-in per lock: `PyDetachingRwLock` is a separate type from `PyRwLock`, and `Traverse` is not implemented for it, so a payload holding one cannot derive `Traverse`. `PyByteArray::inner` takes it. `BorrowedValue`/`BorrowedValueMut` gain the matching mapped-guard variants. `a_thread_blocked_on_a_lock_does_not_stall_stop_the_world` blocks an interpreter thread on a `PyDetachingRwLock` and asserts stop-the-world still completes, running the stop on its own thread with a timeout so a stop that never completes fails rather than hangs. Without the hook installed it fails on the 10 s timeout; with it, it passes in 0.07 s. Assisted-by: Claude

1 parent ebc0459 commit 72d9243
Copy full SHA for 72d9243

6 files changed

+402-15Lines changed: 402 additions & 15 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• crates
  • common/src
    • borrow.rs
    • lock.rs
    • lock
      • detaching.rs
  • vm/src
    • builtins
      • bytearray.rs
    • vm
      • interpreter.rs
      • thread.rs

Expand file treeCollapse file tree

Open diff view settings

Collapse file

‎crates/common/src/borrow.rs‎

Copy file name to clipboardExpand all lines: crates/common/src/borrow.rs

+15-1Lines changed: 15 additions & 1 deletion

Original file line number	Diff line number	Diff line change
@@ -1,5 +1,6 @@
1	1	use crate::lock::{
2		- MapImmutable, PyImmutableMappedMutexGuard, PyMappedMutexGuard, PyMappedRwLockReadGuard,
	2	+ MapImmutable, PyImmutableMappedMutexGuard, PyMappedDetachingRwLockReadGuard,
	3	+ PyMappedDetachingRwLockWriteGuard, PyMappedMutexGuard, PyMappedRwLockReadGuard,
3	4	PyMappedRwLockWriteGuard, PyMutexGuard, PyRwLockReadGuard, PyRwLockWriteGuard,
4	5	};
5	6	use alloc::fmt;
@@ -24,13 +25,15 @@ pub enum BorrowedValue<'a, T: ?Sized> {
24	25	MappedMuLock(PyImmutableMappedMutexGuard<'a, T>),
25	26	ReadLock(PyRwLockReadGuard<'a, T>),
26	27	MappedReadLock(PyMappedRwLockReadGuard<'a, T>),
	28	+ MappedDetachingReadLock(PyMappedDetachingRwLockReadGuard<'a, T>),
27	29	}
28	30	impl_from!('a, T, BorrowedValue<'a, T>,
29	31	Ref(&'a T),
30	32	MuLock(PyMutexGuard<'a, T>),
31	33	MappedMuLock(PyImmutableMappedMutexGuard<'a, T>),
32	34	ReadLock(PyRwLockReadGuard<'a, T>),
33	35	MappedReadLock(PyMappedRwLockReadGuard<'a, T>),
	36	+ MappedDetachingReadLock(PyMappedDetachingRwLockReadGuard<'a, T>),
34	37	);
35	38
36	39	impl<'a, T: ?Sized> BorrowedValue<'a, T> {
@@ -59,6 +62,9 @@ impl<'a, T: ?Sized> BorrowedValue<'a, T> {
59	62	Self::MappedReadLock(m) => {
60	63	BorrowedValue::MappedReadLock(PyMappedRwLockReadGuard::map(m, f))
61	64	}
	65	+ Self::MappedDetachingReadLock(m) => {
	66	+ BorrowedValue::MappedDetachingReadLock(PyMappedDetachingRwLockReadGuard::map(m, f))
	67	+ }
62	68	}
63	69	}
64	70	}
@@ -73,6 +79,7 @@ impl<T: ?Sized> Deref for BorrowedValue<'_, T> {
73	79	Self::MappedMuLock(m) => m,
74	80	Self::ReadLock(r) => r,
75	81	Self::MappedReadLock(m) => m,
	82	+ Self::MappedDetachingReadLock(m) => m,
76	83	}
77	84	}
78	85	}
@@ -90,6 +97,7 @@ pub enum BorrowedValueMut<'a, T: ?Sized> {
90	97	MappedMuLock(PyMappedMutexGuard<'a, T>),
91	98	WriteLock(PyRwLockWriteGuard<'a, T>),
92	99	MappedWriteLock(PyMappedRwLockWriteGuard<'a, T>),
	100	+ MappedDetachingWriteLock(PyMappedDetachingRwLockWriteGuard<'a, T>),
93	101	}
94	102
95	103	impl_from!('a, T, BorrowedValueMut<'a, T>,
@@ -98,6 +106,7 @@ impl_from!('a, T, BorrowedValueMut<'a, T>,
98	106	MappedMuLock(PyMappedMutexGuard<'a, T>),
99	107	WriteLock(PyRwLockWriteGuard<'a, T>),
100	108	MappedWriteLock(PyMappedRwLockWriteGuard<'a, T>),
	109	+ MappedDetachingWriteLock(PyMappedDetachingRwLockWriteGuard<'a, T>),
101	110	);
102	111
103	112	impl<'a, T: ?Sized> BorrowedValueMut<'a, T> {
@@ -113,6 +122,9 @@ impl<'a, T: ?Sized> BorrowedValueMut<'a, T> {
113	122	Self::MappedWriteLock(m) => {
114	123	BorrowedValueMut::MappedWriteLock(PyMappedRwLockWriteGuard::map(m, f))
115	124	}
	125	+ Self::MappedDetachingWriteLock(m) => BorrowedValueMut::MappedDetachingWriteLock(
	126	+ PyMappedDetachingRwLockWriteGuard::map(m, f),
	127	+ ),
116	128	}
117	129	}
118	130	}
@@ -127,6 +139,7 @@ impl<T: ?Sized> Deref for BorrowedValueMut<'_, T> {
127	139	Self::MappedMuLock(m) => m,
128	140	Self::WriteLock(w) => w,
129	141	Self::MappedWriteLock(w) => w,
	142	+ Self::MappedDetachingWriteLock(w) => w,
130	143	}
131	144	}
132	145	}
@@ -139,6 +152,7 @@ impl<T: ?Sized> DerefMut for BorrowedValueMut<'_, T> {
139	152	Self::MappedMuLock(m) => &mut *m,
140	153	Self::WriteLock(w) => &mut *w,
141	154	Self::MappedWriteLock(w) => &mut *w,
	155	+ Self::MappedDetachingWriteLock(w) => &mut *w,
142	156	}
143	157	}
144	158	}

Collapse file

‎crates/common/src/lock.rs‎

Copy file name to clipboardExpand all lines: crates/common/src/lock.rs

+16Lines changed: 16 additions & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -8,6 +8,7 @@ use lock_api::{
8	8
9	9	cfg_select! {
10	10	feature = "threading" => {
	11	+ pub use detaching::{BlockingWaitHook, set_blocking_wait_hook};
11	12	pub use parking_lot::{RawMutex, RawRwLock, RawThreadId};
12	13	pub use std::sync::OnceLock as OnceCell;
13	14	pub use core::cell::LazyCell;
@@ -47,6 +48,8 @@ cfg_select! {
47	48	}
48	49	}
49	50
	51	+ mod detaching;
	52	+ pub use detaching::RawDetachingRwLock;
50	53	mod immutable_mutex;
51	54	pub use immutable_mutex::*;
52	55	mod thread_mutex;
@@ -60,6 +63,19 @@ pub type PyThreadMutex<T> = ThreadMutex<RawMutex, RawThreadId, T>;
60	63	pub type PyThreadMutexGuard<'a, T> = ThreadMutexGuard<'a, RawMutex, RawThreadId, T>;
61	64	pub type PyMappedThreadMutexGuard<'a, T> = MappedThreadMutexGuard<'a, RawMutex, RawThreadId, T>;
62	65
	66	+ /// A `PyRwLock` for data a thread may hold locked across a blocking call.
	67	+ ///
	68	+ /// Waiting for one of these leaves the interpreter first, so a thread blocked
	69	+ /// on it is a thread stop-the-world can park. That is only safe where a
	70	+ /// collection never takes the same lock — see [`RawDetachingRwLock`] — so this
	71	+ /// is opt-in per lock rather than what every `PyRwLock` does.
	72	+ pub type PyDetachingRwLock<T> = RwLock<RawDetachingRwLock, T>;
	73	+ pub type PyDetachingRwLockReadGuard<'a, T> = RwLockReadGuard<'a, RawDetachingRwLock, T>;
	74	+ pub type PyDetachingRwLockWriteGuard<'a, T> = RwLockWriteGuard<'a, RawDetachingRwLock, T>;
	75	+ pub type PyMappedDetachingRwLockReadGuard<'a, T> = MappedRwLockReadGuard<'a, RawDetachingRwLock, T>;
	76	+ pub type PyMappedDetachingRwLockWriteGuard<'a, T> =
	77	+ MappedRwLockWriteGuard<'a, RawDetachingRwLock, T>;
	78	+
63	79	pub type PyRwLock<T> = RwLock<RawRwLock, T>;
64	80	pub type PyRwLockUpgradableReadGuard<'a, T> = RwLockUpgradableReadGuard<'a, RawRwLock, T>;
65	81	pub type PyRwLockReadGuard<'a, T> = RwLockReadGuard<'a, RawRwLock, T>;

Collapse file

‎crates/common/src/lock/detaching.rs‎

Copy file name to clipboard

+250Lines changed: 250 additions & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -0,0 +1,250 @@
	1	+ //! A reader-writer lock that lets a thread leave its interpreter before it
	2	+ //! blocks.
	3	+ //!
	4	+ //! Stopping the world means waiting for every running thread to reach a
	5	+ //! safepoint. A thread blocked on a lock reaches none, so if the thread holding
	6	+ //! that lock has already been stopped, the two wait on each other forever. The
	7	+ //! holder is not the one who can avoid this — a lock is held across a blocking
	8	+ //! call precisely because that is what the call needs — so the waiter gives up
	9	+ //! its interpreter for the duration of the wait instead, which is what a
	10	+ //! blocking call does anyway.
	11	+ //!
	12	+ //! Doing so is safe only for locks nothing reachable from a stop-the-world
	13	+ //! section takes, so it is opt-in per lock — see [`RawDetachingRwLock`] for the
	14	+ //! rule and why it is needed.
	15	+ //!
	16	+ //! Only the contended path pays for any of this: an acquire that takes the lock
	17	+ //! on the first try is the same atomic exchange it was, and never reaches the
	18	+ //! hook. The hook is installed by whoever knows how to detach a thread
	19	+ //! ([`set_blocking_wait_hook`]); until then, and on any thread that is not
	20	+ //! running an interpreter, a blocked acquire just blocks.
	21	+
	22	+ use super::RawRwLock;
	23	+ #[cfg(feature = "threading")]
	24	+ use core::cell::Cell;
	25	+ use lock_api::{
	26	+ RawRwLock as RawRwLockTrait, RawRwLockDowngrade, RawRwLockRecursive as RawRwLockRecursiveTrait,
	27	+ RawRwLockUpgrade as RawRwLockUpgradeTrait, RawRwLockUpgradeDowngrade,
	28	+ };
	29	+ #[cfg(feature = "threading")]
	30	+ use std::sync::OnceLock;
	31	+
	32	+ /// Runs `wait` with the calling thread detached from its interpreter.
	33	+ #[cfg(feature = "threading")]
	34	+ pub type BlockingWaitHook = fn(wait: &dyn Fn());
	35	+
	36	+ #[cfg(feature = "threading")]
	37	+ static BLOCKING_WAIT: OnceLock<BlockingWaitHook> = OnceLock::new();
	38	+
	39	+ /// Install the hook that detaches a thread around a blocked lock acquire.
	40	+ ///
	41	+ /// Later calls are ignored, so every interpreter in a process can call this
	42	+ /// during its own initialization.
	43	+ #[cfg(feature = "threading")]
	44	+ pub fn set_blocking_wait_hook(hook: BlockingWaitHook) {
	45	+ let _ = BLOCKING_WAIT.set(hook);
	46	+ }
	47	+
	48	+ #[cfg(feature = "threading")]
	49	+ std::thread_local! {
	50	+ /// Set while this thread is inside the hook, so that a lock taken by the
	51	+ /// hook itself — or by anything detaching and re-attaching runs — waits
	52	+ /// plainly instead of recursing back into it.
	53	+ static IN_HOOK: Cell<bool> = const { Cell::new(false) };
	54	+ }
	55	+
	56	+ /// Clears [`IN_HOOK`] even if the hook unwinds.
	57	+ #[cfg(feature = "threading")]
	58	+ struct HookGuard;
	59	+
	60	+ #[cfg(feature = "threading")]
	61	+ impl Drop for HookGuard {
	62	+ fn drop(&mut self) {
	63	+ let _ = IN_HOOK.try_with(|in_hook| in_hook.set(false));
	64	+ }
	65	+ }
	66	+
	67	+ /// Block on `wait`, detached from this thread's interpreter if there is one.
	68	+ #[cfg(feature = "threading")]
	69	+ #[cold]
	70	+ #[inline(never)]
	71	+ fn wait_detached(wait: impl Fn()) {
	72	+ let Some(hook) = BLOCKING_WAIT.get() else {
	73	+ wait();
	74	+ return;
	75	+ };
	76	+ // `try_with` fails once the thread's locals are being destroyed, which is
	77	+ // also a point at which there is no interpreter left to detach from.
	78	+ let entered = IN_HOOK
	79	+ .try_with(|in_hook| !in_hook.replace(true))
	80	+ .unwrap_or(false);
	81	+ if !entered {
	82	+ wait();
	83	+ return;
	84	+ }
	85	+ let _guard = HookGuard;
	86	+ hook(&wait);
	87	+ }
	88	+
	89	+ /// Without threads there is no interpreter to leave and nothing to stop.
	90	+ #[cfg(not(feature = "threading"))]
	91	+ #[inline]
	92	+ fn wait_detached(wait: impl Fn()) {
	93	+ wait();
	94	+ }
	95	+
	96	+ /// A reader-writer lock whose blocking acquires detach first, and which is the
	97	+ /// raw lock it wraps in every other respect.
	98	+ ///
	99	+ /// Use through [`PyDetachingRwLock`](super::PyDetachingRwLock).
	100	+ ///
	101	+ /// # Only for locks a collection never takes
	102	+ ///
	103	+ /// The wait acquires the lock while detached, so the thread comes back holding
	104	+ /// it, and re-attaching is a point at which a stop-the-world in flight will
	105	+ /// park the thread. It is therefore parked *holding the lock*. Everything that
	106	+ /// stops the world must be able to finish without that lock: if a collection
	107	+ /// were to take it, the collection would block on a thread only the collection
	108	+ /// can release, and neither would move again.
	109	+ ///
	110	+ /// So this is opt-in per lock, and the rule for opting in is that nothing
	111	+ /// reachable from a stop-the-world section takes the same lock. An object whose
	112	+ /// payload holds no references — nothing for the collector to traverse into —
	113	+ /// satisfies that; most do not.
	114	+ ///
	115	+ /// Not implementing the vm's `Traverse` for this lock is what keeps that from
	116	+ /// being only a convention: a payload holding one cannot derive `Traverse`, so
	117	+ /// it cannot become something a collection walks into.
	118	+ #[repr(transparent)]
	119	+ pub struct RawDetachingRwLock(RawRwLock);
	120	+
	121	+ // SAFETY: every method forwards to the wrapped raw lock, which upholds the
	122	+ // contract; the blocking acquires only add a wait that ends with the same lock
	123	+ // acquired.
	124	+ unsafe impl RawRwLockTrait for RawDetachingRwLock {
	125	+ #[allow(
	126	+ clippy::declare_interior_mutable_const,
	127	+ reason = "raw lock initializer, as in the type it wraps"
	128	+ )]
	129	+ const INIT: Self = Self(<RawRwLock as RawRwLockTrait>::INIT);
	130	+
	131	+ type GuardMarker = <RawRwLock as RawRwLockTrait>::GuardMarker;
	132	+
	133	+ #[inline]
	134	+ fn lock_shared(&self) {
	135	+ if !self.0.try_lock_shared() {
	136	+ wait_detached(|| self.0.lock_shared());
	137	+ }
	138	+ }
	139	+
	140	+ #[inline]
	141	+ fn try_lock_shared(&self) -> bool {
	142	+ self.0.try_lock_shared()
	143	+ }
	144	+
	145	+ #[inline]
	146	+ unsafe fn unlock_shared(&self) {
	147	+ unsafe { self.0.unlock_shared() }
	148	+ }
	149	+
	150	+ #[inline]
	151	+ fn lock_exclusive(&self) {
	152	+ if !self.0.try_lock_exclusive() {
	153	+ wait_detached(|| self.0.lock_exclusive());
	154	+ }
	155	+ }
	156	+
	157	+ #[inline]
	158	+ fn try_lock_exclusive(&self) -> bool {
	159	+ self.0.try_lock_exclusive()
	160	+ }
	161	+
	162	+ #[inline]
	163	+ unsafe fn unlock_exclusive(&self) {
	164	+ unsafe { self.0.unlock_exclusive() }
	165	+ }
	166	+
	167	+ #[inline]
	168	+ fn is_locked(&self) -> bool {
	169	+ self.0.is_locked()
	170	+ }
	171	+
	172	+ #[inline]
	173	+ fn is_locked_exclusive(&self) -> bool {
	174	+ self.0.is_locked_exclusive()
	175	+ }
	176	+ }
	177	+
	178	+ // SAFETY: forwards to the wrapped raw lock.
	179	+ unsafe impl RawRwLockDowngrade for RawDetachingRwLock {
	180	+ #[inline]
	181	+ unsafe fn downgrade(&self) {
	182	+ unsafe { self.0.downgrade() }
	183	+ }
	184	+ }
	185	+
	186	+ // SAFETY: forwards to the wrapped raw lock; the blocking acquires only add
	187	+ // a wait that ends with the same lock acquired.
	188	+ unsafe impl RawRwLockUpgradeTrait for RawDetachingRwLock {
	189	+ #[inline]
	190	+ fn lock_upgradable(&self) {
	191	+ if !self.0.try_lock_upgradable() {
	192	+ wait_detached(|| self.0.lock_upgradable());
	193	+ }
	194	+ }
	195	+
	196	+ #[inline]
	197	+ fn try_lock_upgradable(&self) -> bool {
	198	+ self.0.try_lock_upgradable()
	199	+ }
	200	+
	201	+ #[inline]
	202	+ unsafe fn unlock_upgradable(&self) {
	203	+ unsafe { self.0.unlock_upgradable() }
	204	+ }
	205	+
	206	+ #[inline]
	207	+ unsafe fn upgrade(&self) {
	208	+ // SAFETY: the caller holds the upgradable lock, as `upgrade` requires,
	209	+ // and it stays held for both the failed attempt and the wait.
	210	+ unsafe {
	211	+ if !self.0.try_upgrade() {
	212	+ wait_detached(|| self.0.upgrade());
	213	+ }
	214	+ }
	215	+ }
	216	+
	217	+ #[inline]
	218	+ unsafe fn try_upgrade(&self) -> bool {
	219	+ unsafe { self.0.try_upgrade() }
	220	+ }
	221	+ }
	222	+
	223	+ // SAFETY: forwards to the wrapped raw lock.
	224	+ unsafe impl RawRwLockUpgradeDowngrade for RawDetachingRwLock {
	225	+ #[inline]
	226	+ unsafe fn downgrade_upgradable(&self) {
	227	+ unsafe { self.0.downgrade_upgradable() }
	228	+ }
	229	+
	230	+ #[inline]
	231	+ unsafe fn downgrade_to_upgradable(&self) {
	232	+ unsafe { self.0.downgrade_to_upgradable() }
	233	+ }
	234	+ }
	235	+
	236	+ // SAFETY: forwards to the wrapped raw lock; the blocking acquire only adds
	237	+ // a wait that ends with the same lock acquired.
	238	+ unsafe impl RawRwLockRecursiveTrait for RawDetachingRwLock {
	239	+ #[inline]
	240	+ fn lock_shared_recursive(&self) {
	241	+ if !self.0.try_lock_shared_recursive() {
	242	+ wait_detached(|| self.0.lock_shared_recursive());
	243	+ }
	244	+ }
	245	+
	246	+ #[inline]
	247	+ fn try_lock_shared_recursive(&self) -> bool {
	248	+ self.0.try_lock_shared_recursive()
	249	+ }
	250	+ }

0 commit comments

Comments

0 (0)