Skip to content

Commit ce1961b

Browse files
Rollup merge of #144185 - purplesyringa:poisoning-wording, r=Amanieu
Document guarantees of poisoning This mostly documents the current behavior of `Mutex` and `RwLock` (#143471) as imperfect. It's unlikely that the situation improves significantly in the future, and even if it does, the rules will probably be more complicated than "poisoning is completely reliable", so this is a conservative guarantee. We also explicitly specify that `OnceLock` never poisons, even though it has an API similar to mutexes. Fixes #143471 by improving documentation. r? ``@Amanieu``
2 parents 96b3b83 + c1d06cc commit ce1961b

File tree

5 files changed

+87
-25
lines changed

5 files changed

+87
-25
lines changed

library/std/src/sync/once_lock.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ use crate::sync::Once;
1616
/// A `OnceLock` can be thought of as a safe abstraction over uninitialized data that becomes
1717
/// initialized once written.
1818
///
19+
/// Unlike [`Mutex`](crate::sync::Mutex), `OnceLock` is never poisoned on panic.
20+
///
1921
/// [`OnceCell`]: crate::cell::OnceCell
2022
/// [`LazyLock<T, F>`]: crate::sync::LazyLock
2123
/// [`LazyLock::new(|| ...)`]: crate::sync::LazyLock::new

library/std/src/sync/poison.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,16 @@
22
//!
33
//! # Poisoning
44
//!
5-
//! All synchronization objects in this module implement a strategy called "poisoning"
6-
//! where if a thread panics while holding the exclusive access granted by the primitive,
7-
//! the state of the primitive is set to "poisoned".
8-
//! This information is then propagated to all other threads
5+
//! All synchronization objects in this module implement a strategy called
6+
//! "poisoning" where a primitive becomes poisoned if it recognizes that some
7+
//! thread has panicked while holding the exclusive access granted by the
8+
//! primitive. This information is then propagated to all other threads
99
//! to signify that the data protected by this primitive is likely tainted
1010
//! (some invariant is not being upheld).
1111
//!
12-
//! The specifics of how this "poisoned" state affects other threads
13-
//! depend on the primitive. See [#Overview] below.
12+
//! The specifics of how this "poisoned" state affects other threads and whether
13+
//! the panics are recognized reliably or on a best-effort basis depend on the
14+
//! primitive. See [#Overview] below.
1415
//!
1516
//! For the alternative implementations that do not employ poisoning,
1617
//! see [`std::sync::nonpoison`].
@@ -36,14 +37,15 @@
3637
//! - [`Mutex`]: Mutual Exclusion mechanism, which ensures that at
3738
//! most one thread at a time is able to access some data.
3839
//!
39-
//! [`Mutex::lock()`] returns a [`LockResult`],
40-
//! providing a way to deal with the poisoned state.
41-
//! See [`Mutex`'s documentation](Mutex#poisoning) for more.
40+
//! Panicking while holding the lock typically poisons the mutex, but it is
41+
//! not guaranteed to detect this condition in all circumstances.
42+
//! [`Mutex::lock()`] returns a [`LockResult`], providing a way to deal with
43+
//! the poisoned state. See [`Mutex`'s documentation](Mutex#poisoning) for more.
4244
//!
4345
//! - [`Once`]: A thread-safe way to run a piece of code only once.
4446
//! Mostly useful for implementing one-time global initialization.
4547
//!
46-
//! [`Once`] is poisoned if the piece of code passed to
48+
//! [`Once`] is reliably poisoned if the piece of code passed to
4749
//! [`Once::call_once()`] or [`Once::call_once_force()`] panics.
4850
//! When in poisoned state, subsequent calls to [`Once::call_once()`] will panic too.
4951
//! [`Once::call_once_force()`] can be used to clear the poisoned state.
@@ -53,7 +55,7 @@
5355
//! writer at a time. In some cases, this can be more efficient than
5456
//! a mutex.
5557
//!
56-
//! This implementation, like [`Mutex`], will become poisoned on a panic.
58+
//! This implementation, like [`Mutex`], usually becomes poisoned on a panic.
5759
//! Note, however, that an `RwLock` may only be poisoned if a panic occurs
5860
//! while it is locked exclusively (write mode). If a panic occurs in any reader,
5961
//! then the lock will not be poisoned.

library/std/src/sync/poison/mutex.rs

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,79 @@ use crate::sys::sync as sys;
1818
/// # Poisoning
1919
///
2020
/// The mutexes in this module implement a strategy called "poisoning" where a
21-
/// mutex is considered poisoned whenever a thread panics while holding the
22-
/// mutex. Once a mutex is poisoned, all other threads are unable to access the
23-
/// data by default as it is likely tainted (some invariant is not being
24-
/// upheld).
21+
/// mutex becomes poisoned if it recognizes that the thread holding it has
22+
/// panicked.
2523
///
26-
/// For a mutex, this means that the [`lock`] and [`try_lock`] methods return a
24+
/// Once a mutex is poisoned, all other threads are unable to access the data by
25+
/// default as it is likely tainted (some invariant is not being upheld). For a
26+
/// mutex, this means that the [`lock`] and [`try_lock`] methods return a
2727
/// [`Result`] which indicates whether a mutex has been poisoned or not. Most
2828
/// usage of a mutex will simply [`unwrap()`] these results, propagating panics
2929
/// among threads to ensure that a possibly invalid invariant is not witnessed.
3030
///
31-
/// A poisoned mutex, however, does not prevent all access to the underlying
32-
/// data. The [`PoisonError`] type has an [`into_inner`] method which will return
33-
/// the guard that would have otherwise been returned on a successful lock. This
34-
/// allows access to the data, despite the lock being poisoned.
31+
/// Poisoning is only advisory: the [`PoisonError`] type has an [`into_inner`]
32+
/// method which will return the guard that would have otherwise been returned
33+
/// on a successful lock. This allows access to the data, despite the lock being
34+
/// poisoned.
35+
///
36+
/// In addition, the panic detection is not ideal, so even unpoisoned mutexes
37+
/// need to be handled with care, since certain panics may have been skipped.
38+
/// Here is a non-exhaustive list of situations where this might occur:
39+
///
40+
/// - If a mutex is locked while a panic is underway, e.g. within a [`Drop`]
41+
/// implementation or a [panic hook], panicking for the second time while the
42+
/// lock is held will leave the mutex unpoisoned. Note that while double panic
43+
/// usually aborts the program, [`catch_unwind`] can prevent this.
44+
///
45+
/// - Locking and unlocking the mutex across different panic contexts, e.g. by
46+
/// storing the guard to a [`Cell`] within [`Drop::drop`] and accessing it
47+
/// outside, or vice versa, can affect poisoning status in an unexpected way.
48+
///
49+
/// - Foreign exceptions do not currently trigger poisoning even in absence of
50+
/// other panics.
51+
///
52+
/// While this rarely happens in realistic code, `unsafe` code cannot rely on
53+
/// poisoning for soundness, since the behavior of poisoning can depend on
54+
/// outside context. Here's an example of **incorrect** use of poisoning:
55+
///
56+
/// ```rust
57+
/// use std::sync::Mutex;
58+
///
59+
/// struct MutexBox<T> {
60+
/// data: Mutex<*mut T>,
61+
/// }
62+
///
63+
/// impl<T> MutexBox<T> {
64+
/// pub fn new(value: T) -> Self {
65+
/// Self {
66+
/// data: Mutex::new(Box::into_raw(Box::new(value))),
67+
/// }
68+
/// }
69+
///
70+
/// pub fn replace_with(&self, f: impl FnOnce(T) -> T) {
71+
/// let ptr = self.data.lock().expect("poisoned");
72+
/// // While `f` is running, the data is moved out of `*ptr`. If `f`
73+
/// // panics, `*ptr` keeps pointing at a dropped value. The intention
74+
/// // is that this will poison the mutex, so the following calls to
75+
/// // `replace_with` will panic without reading `*ptr`. But since
76+
/// // poisoning is not guaranteed to occur if this is run from a panic
77+
/// // hook, this can lead to use-after-free.
78+
/// unsafe {
79+
/// (*ptr).write(f((*ptr).read()));
80+
/// }
81+
/// }
82+
/// }
83+
/// ```
3584
///
3685
/// [`new`]: Self::new
3786
/// [`lock`]: Self::lock
3887
/// [`try_lock`]: Self::try_lock
3988
/// [`unwrap()`]: Result::unwrap
4089
/// [`PoisonError`]: super::PoisonError
4190
/// [`into_inner`]: super::PoisonError::into_inner
91+
/// [panic hook]: crate::panic::set_hook
92+
/// [`catch_unwind`]: crate::panic::catch_unwind
93+
/// [`Cell`]: crate::cell::Cell
4294
///
4395
/// # Examples
4496
///

library/std/src/sync/poison/once.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ impl Once {
136136
/// it will *poison* this [`Once`] instance, causing all future invocations of
137137
/// `call_once` to also panic.
138138
///
139-
/// This is similar to [poisoning with mutexes][poison].
139+
/// This is similar to [poisoning with mutexes][poison], but this mechanism
140+
/// is guaranteed to never skip panics within `f`.
140141
///
141142
/// [poison]: struct.Mutex.html#poisoning
142143
#[inline]
@@ -293,6 +294,9 @@ impl Once {
293294

294295
/// Blocks the current thread until initialization has completed, ignoring
295296
/// poisoning.
297+
///
298+
/// If this [`Once`] has been poisoned, this function blocks until it
299+
/// becomes completed, unlike [`Once::wait()`], which panics in this case.
296300
#[stable(feature = "once_wait", since = "1.86.0")]
297301
pub fn wait_force(&self) {
298302
if !self.inner.is_completed() {

library/std/src/sync/poison/rwlock.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,12 @@ use crate::sys::sync as sys;
4646
///
4747
/// # Poisoning
4848
///
49-
/// An `RwLock`, like [`Mutex`], will become poisoned on a panic. Note, however,
50-
/// that an `RwLock` may only be poisoned if a panic occurs while it is locked
51-
/// exclusively (write mode). If a panic occurs in any reader, then the lock
52-
/// will not be poisoned.
49+
/// An `RwLock`, like [`Mutex`], will [usually] become poisoned on a panic. Note,
50+
/// however, that an `RwLock` may only be poisoned if a panic occurs while it is
51+
/// locked exclusively (write mode). If a panic occurs in any reader, then the
52+
/// lock will not be poisoned.
53+
///
54+
/// [usually]: super::Mutex#poisoning
5355
///
5456
/// # Examples
5557
///

0 commit comments

Comments
 (0)