Rollup merge of #144185 - purplesyringa:poisoning-wording, r=Amanieu

Document guarantees of poisoning

This mostly documents the current behavior of `Mutex` and `RwLock` (rust-lang/rust#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 rust-lang/rust#143471 by improving documentation.

r? ``@Amanieu``

5 files changed, 87 insertions, 25 deletions

diff --git a/library/std/src/sync/once_lock.rs b/library/std/src/sync/once_lock.rs
index a5c3a6c46a4..b224044cbe0 100644
--- a/library/std/src/sync/once_lock.rs
+++ b/library/std/src/sync/once_lock.rs
@@ -16,6 +16,8 @@ use crate::sync::Once;
 /// A `OnceLock` can be thought of as a safe abstraction over uninitialized data that becomes
 /// initialized once written.
 ///
+/// Unlike [`Mutex`](crate::sync::Mutex), `OnceLock` is never poisoned on panic.
+///
 /// [`OnceCell`]: crate::cell::OnceCell
 /// [`LazyLock<T, F>`]: crate::sync::LazyLock
 /// [`LazyLock::new(|| ...)`]: crate::sync::LazyLock::new
diff --git a/library/std/src/sync/poison.rs b/library/std/src/sync/poison.rs
index b901a5701a4..d5adc9e29b5 100644
--- a/library/std/src/sync/poison.rs
+++ b/library/std/src/sync/poison.rs
@@ -2,15 +2,16 @@
 //!
 //! # Poisoning
 //!
-//! All synchronization objects in this module implement a strategy called "poisoning"
-//! where if a thread panics while holding the exclusive access granted by the primitive,
-//! the state of the primitive is set to "poisoned".
-//! This information is then propagated to all other threads
+//! All synchronization objects in this module implement a strategy called
+//! "poisoning" where a primitive becomes poisoned if it recognizes that some
+//! thread has panicked while holding the exclusive access granted by the
+//! primitive. This information is then propagated to all other threads
 //! to signify that the data protected by this primitive is likely tainted
 //! (some invariant is not being upheld).
 //!
-//! The specifics of how this "poisoned" state affects other threads
-//! depend on the primitive. See [#Overview] below.
+//! The specifics of how this "poisoned" state affects other threads and whether
+//! the panics are recognized reliably or on a best-effort basis depend on the
+//! primitive. See [#Overview] below.
 //!
 //! For the alternative implementations that do not employ poisoning,
 //! see [`std::sync::nonpoison`].
@@ -36,14 +37,15 @@
 //! - [`Mutex`]: Mutual Exclusion mechanism, which ensures that at
 //!   most one thread at a time is able to access some data.
 //!
-//!   [`Mutex::lock()`] returns a [`LockResult`],
-//!   providing a way to deal with the poisoned state.
-//!   See [`Mutex`'s documentation](Mutex#poisoning) for more.
+//!   Panicking while holding the lock typically poisons the mutex, but it is
+//!   not guaranteed to detect this condition in all circumstances.
+//!   [`Mutex::lock()`] returns a [`LockResult`], providing a way to deal with
+//!   the poisoned state. See [`Mutex`'s documentation](Mutex#poisoning) for more.
 //!
 //! - [`Once`]: A thread-safe way to run a piece of code only once.
 //!   Mostly useful for implementing one-time global initialization.
 //!
-//!   [`Once`] is poisoned if the piece of code passed to
+//!   [`Once`] is reliably poisoned if the piece of code passed to
 //!   [`Once::call_once()`] or [`Once::call_once_force()`] panics.
 //!   When in poisoned state, subsequent calls to [`Once::call_once()`] will panic too.
 //!   [`Once::call_once_force()`] can be used to clear the poisoned state.
@@ -53,7 +55,7 @@
 //!   writer at a time. In some cases, this can be more efficient than
 //!   a mutex.
 //!
-//!   This implementation, like [`Mutex`], will become poisoned on a panic.
+//!   This implementation, like [`Mutex`], usually becomes poisoned on a panic.
 //!   Note, however, that an `RwLock` may only be poisoned if a panic occurs
 //!   while it is locked exclusively (write mode). If a panic occurs in any reader,
 //!   then the lock will not be poisoned.
diff --git a/library/std/src/sync/poison/mutex.rs b/library/std/src/sync/poison/mutex.rs
index 64744f18c74..6205c4fa4ca 100644
--- a/library/std/src/sync/poison/mutex.rs
+++ b/library/std/src/sync/poison/mutex.rs
@@ -18,20 +18,69 @@ use crate::sys::sync as sys;
 /// # Poisoning
 ///
 /// The mutexes in this module implement a strategy called "poisoning" where a
-/// mutex is considered poisoned whenever a thread panics while holding the
-/// mutex. Once a mutex is poisoned, all other threads are unable to access the
-/// data by default as it is likely tainted (some invariant is not being
-/// upheld).
+/// mutex becomes poisoned if it recognizes that the thread holding it has
+/// panicked.
 ///
-/// For a mutex, this means that the [`lock`] and [`try_lock`] methods return a
+/// Once a mutex is poisoned, all other threads are unable to access the data by
+/// default as it is likely tainted (some invariant is not being upheld). For a
+/// mutex, this means that the [`lock`] and [`try_lock`] methods return a
 /// [`Result`] which indicates whether a mutex has been poisoned or not. Most
 /// usage of a mutex will simply [`unwrap()`] these results, propagating panics
 /// among threads to ensure that a possibly invalid invariant is not witnessed.
 ///
-/// A poisoned mutex, however, does not prevent all access to the underlying
-/// data. The [`PoisonError`] type has an [`into_inner`] method which will return
-/// the guard that would have otherwise been returned on a successful lock. This
-/// allows access to the data, despite the lock being poisoned.
+/// Poisoning is only advisory: the [`PoisonError`] type has an [`into_inner`]
+/// method which will return the guard that would have otherwise been returned
+/// on a successful lock. This allows access to the data, despite the lock being
+/// poisoned.
+///
+/// In addition, the panic detection is not ideal, so even unpoisoned mutexes
+/// need to be handled with care, since certain panics may have been skipped.
+/// Here is a non-exhaustive list of situations where this might occur:
+///
+/// - If a mutex is locked while a panic is underway, e.g. within a [`Drop`]
+///   implementation or a [panic hook], panicking for the second time while the
+///   lock is held will leave the mutex unpoisoned. Note that while double panic
+///   usually aborts the program, [`catch_unwind`] can prevent this.
+///
+/// - Locking and unlocking the mutex across different panic contexts, e.g. by
+///   storing the guard to a [`Cell`] within [`Drop::drop`] and accessing it
+///   outside, or vice versa, can affect poisoning status in an unexpected way.
+///
+/// - Foreign exceptions do not currently trigger poisoning even in absence of
+///   other panics.
+///
+/// While this rarely happens in realistic code, `unsafe` code cannot rely on
+/// poisoning for soundness, since the behavior of poisoning can depend on
+/// outside context. Here's an example of **incorrect** use of poisoning:
+///
+/// ```rust
+/// use std::sync::Mutex;
+///
+/// struct MutexBox<T> {
+///     data: Mutex<*mut T>,
+/// }
+///
+/// impl<T> MutexBox<T> {
+///     pub fn new(value: T) -> Self {
+///         Self {
+///             data: Mutex::new(Box::into_raw(Box::new(value))),
+///         }
+///     }
+///
+///     pub fn replace_with(&self, f: impl FnOnce(T) -> T) {
+///         let ptr = self.data.lock().expect("poisoned");
+///         // While `f` is running, the data is moved out of `*ptr`. If `f`
+///         // panics, `*ptr` keeps pointing at a dropped value. The intention
+///         // is that this will poison the mutex, so the following calls to
+///         // `replace_with` will panic without reading `*ptr`. But since
+///         // poisoning is not guaranteed to occur if this is run from a panic
+///         // hook, this can lead to use-after-free.
+///         unsafe {
+///             (*ptr).write(f((*ptr).read()));
+///         }
+///     }
+/// }
+/// ```
 ///
 /// [`new`]: Self::new
 /// [`lock`]: Self::lock
@@ -39,6 +88,9 @@ use crate::sys::sync as sys;
 /// [`unwrap()`]: Result::unwrap
 /// [`PoisonError`]: super::PoisonError
 /// [`into_inner`]: super::PoisonError::into_inner
+/// [panic hook]: crate::panic::set_hook
+/// [`catch_unwind`]: crate::panic::catch_unwind
+/// [`Cell`]: crate::cell::Cell
 ///
 /// # Examples
 ///
diff --git a/library/std/src/sync/poison/once.rs b/library/std/src/sync/poison/once.rs
index 103e5195407..faf2913c547 100644
--- a/library/std/src/sync/poison/once.rs
+++ b/library/std/src/sync/poison/once.rs
@@ -136,7 +136,8 @@ impl Once {
     /// it will *poison* this [`Once`] instance, causing all future invocations of
     /// `call_once` to also panic.
     ///
-    /// This is similar to [poisoning with mutexes][poison].
+    /// This is similar to [poisoning with mutexes][poison], but this mechanism
+    /// is guaranteed to never skip panics within `f`.
     ///
     /// [poison]: struct.Mutex.html#poisoning
     #[inline]
@@ -293,6 +294,9 @@ impl Once {
 
     /// Blocks the current thread until initialization has completed, ignoring
     /// poisoning.
+    ///
+    /// If this [`Once`] has been poisoned, this function blocks until it
+    /// becomes completed, unlike [`Once::wait()`], which panics in this case.
     #[stable(feature = "once_wait", since = "1.86.0")]
     pub fn wait_force(&self) {
         if !self.inner.is_completed() {
diff --git a/library/std/src/sync/poison/rwlock.rs b/library/std/src/sync/poison/rwlock.rs
index 934a173425a..2c92602bc87 100644
--- a/library/std/src/sync/poison/rwlock.rs
+++ b/library/std/src/sync/poison/rwlock.rs
@@ -46,10 +46,12 @@ use crate::sys::sync as sys;
 ///
 /// # Poisoning
 ///
-/// An `RwLock`, like [`Mutex`], will become poisoned on a panic. Note, however,
-/// that an `RwLock` may only be poisoned if a panic occurs while it is locked
-/// exclusively (write mode). If a panic occurs in any reader, then the lock
-/// will not be poisoned.
+/// An `RwLock`, like [`Mutex`], will [usually] become poisoned on a panic. Note,
+/// however, that an `RwLock` may only be poisoned if a panic occurs while it is
+/// locked exclusively (write mode). If a panic occurs in any reader, then the
+/// lock will not be poisoned.
+///
+/// [usually]: super::Mutex#poisoning
 ///
 /// # Examples
 ///