From 5b2c61edbe508163ad11cde0f35278417d7c26b3 Mon Sep 17 00:00:00 2001 From: Alisa Sireneva Date: Sat, 19 Jul 2025 18:18:01 +0300 Subject: [PATCH 1/4] Document guarantees of poisoning This mostly documents the current behavior of `Mutex` and `RwLock` 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. --- library/std/src/sync/once_lock.rs | 2 ++ library/std/src/sync/poison.rs | 24 +++++++------ library/std/src/sync/poison/mutex.rs | 52 ++++++++++++++++++++++----- library/std/src/sync/poison/once.rs | 6 +++- library/std/src/sync/poison/rwlock.rs | 8 ++--- 5 files changed, 67 insertions(+), 25 deletions(-) diff --git a/library/std/src/sync/once_lock.rs b/library/std/src/sync/once_lock.rs index a5c3a6c46a433..b224044cbe09c 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`]: 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 0c05f152ef84c..e928b6772cc81 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::nonpoisoning`. @@ -34,14 +35,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. @@ -51,7 +53,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 30325be685c32..15dfe116133ba 100644 --- a/library/std/src/sync/poison/mutex.rs +++ b/library/std/src/sync/poison/mutex.rs @@ -18,20 +18,54 @@ 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. +/// Therefore, `unsafe` code cannot rely on poisoning for soundness. Here's an +/// example of **incorrect** use of poisoning: +/// +/// ```rust +/// use std::sync::Mutex; +/// +/// struct MutexBox { +/// data: Mutex<*mut T>, +/// } +/// +/// impl MutexBox { +/// 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, this can lead to +/// // use-after-free. +/// unsafe { +/// (*ptr).write(f((*ptr).read())); +/// } +/// } +/// } +/// ``` /// /// [`new`]: Self::new /// [`lock`]: Self::lock diff --git a/library/std/src/sync/poison/once.rs b/library/std/src/sync/poison/once.rs index 103e519540795..faf2913c54730 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 934a173425a81..0a50b6c2d8f10 100644 --- a/library/std/src/sync/poison/rwlock.rs +++ b/library/std/src/sync/poison/rwlock.rs @@ -46,10 +46,10 @@ 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. /// /// # Examples /// From da4687bafe4ff78a9816797c3b74b6bcc0c5a3bc Mon Sep 17 00:00:00 2001 From: Alisa Sireneva Date: Fri, 25 Jul 2025 16:09:29 +0300 Subject: [PATCH 2/4] Link to Mutex poisoning docs from RwLock docs --- library/std/src/sync/poison/rwlock.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/std/src/sync/poison/rwlock.rs b/library/std/src/sync/poison/rwlock.rs index 0a50b6c2d8f10..2c92602bc878f 100644 --- a/library/std/src/sync/poison/rwlock.rs +++ b/library/std/src/sync/poison/rwlock.rs @@ -46,11 +46,13 @@ use crate::sys::sync as sys; /// /// # Poisoning /// -/// An `RwLock`, like [`Mutex`], will usually become poisoned on a panic. Note, +/// 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 /// /// ``` From 5b8c61494f85a58759dad04194c8353de07d75e1 Mon Sep 17 00:00:00 2001 From: Alisa Sireneva Date: Fri, 25 Jul 2025 21:03:09 +0300 Subject: [PATCH 3/4] Add a list of failure conditions for poisoning --- library/std/src/sync/poison/mutex.rs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/library/std/src/sync/poison/mutex.rs b/library/std/src/sync/poison/mutex.rs index 15dfe116133ba..317e5fc3bfb9f 100644 --- a/library/std/src/sync/poison/mutex.rs +++ b/library/std/src/sync/poison/mutex.rs @@ -35,8 +35,20 @@ use crate::sys::sync as sys; /// /// 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. -/// Therefore, `unsafe` code cannot rely on poisoning for soundness. Here's an -/// example of **incorrect** use of poisoning: +/// 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. +/// +/// 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; @@ -58,8 +70,8 @@ use crate::sys::sync as sys; /// // 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, this can lead to -/// // use-after-free. +/// // 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())); /// } @@ -73,6 +85,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 /// From c1d06cccaed537c799838dc5338dda28bbace52b Mon Sep 17 00:00:00 2001 From: Alisa Sireneva Date: Fri, 25 Jul 2025 22:14:02 +0300 Subject: [PATCH 4/4] Add a note on foreign exceptions --- library/std/src/sync/poison/mutex.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/std/src/sync/poison/mutex.rs b/library/std/src/sync/poison/mutex.rs index 317e5fc3bfb9f..363220baf7b98 100644 --- a/library/std/src/sync/poison/mutex.rs +++ b/library/std/src/sync/poison/mutex.rs @@ -46,6 +46,9 @@ use crate::sys::sync as sys; /// 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: