From e77c6fcf7e1edfc9a217ba19f719d85d6c864a00 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Tue, 12 Aug 2025 08:14:11 -0400 Subject: [PATCH] modify `LazyLock` poison panic message Fixes an issue where if the underlying `Once` panics because it is poisoned, the panic displays the wrong message. Also consolidates all of the `Once` panic messages in one place so that we don't have to edit the message in 6 different files if we want to change it. Signed-off-by: Connor Tsui --- library/std/src/sync/lazy_lock.rs | 40 +++++++++++++++------ library/std/src/sys/sync/mod.rs | 2 +- library/std/src/sys/sync/once/futex.rs | 4 +-- library/std/src/sys/sync/once/mod.rs | 5 +++ library/std/src/sys/sync/once/no_threads.rs | 2 +- library/std/src/sys/sync/once/queue.rs | 4 +-- library/std/tests/sync/lazy_lock.rs | 29 +++++++++++++++ 7 files changed, 69 insertions(+), 17 deletions(-) diff --git a/library/std/src/sync/lazy_lock.rs b/library/std/src/sync/lazy_lock.rs index a40e29a772a9c..e6f10d2d876fb 100644 --- a/library/std/src/sync/lazy_lock.rs +++ b/library/std/src/sync/lazy_lock.rs @@ -2,8 +2,9 @@ use super::poison::once::ExclusiveState; use crate::cell::UnsafeCell; use crate::mem::ManuallyDrop; use crate::ops::{Deref, DerefMut}; -use crate::panic::{RefUnwindSafe, UnwindSafe}; +use crate::panic::{self, AssertUnwindSafe, RefUnwindSafe, UnwindSafe}; use crate::sync::Once; +use crate::sys::sync::once::ONCE_POISON_PANIC_MSG; use crate::{fmt, ptr}; // We use the state of a Once as discriminant value. Upon creation, the state is @@ -244,21 +245,38 @@ impl T> LazyLock { #[inline] #[stable(feature = "lazy_cell", since = "1.80.0")] pub fn force(this: &LazyLock) -> &T { - this.once.call_once(|| { - // SAFETY: `call_once` only runs this closure once, ever. - let data = unsafe { &mut *this.data.get() }; - let f = unsafe { ManuallyDrop::take(&mut data.f) }; - let value = f(); - data.value = ManuallyDrop::new(value); - }); + let call_once_closure = || { + this.once.call_once(|| { + // SAFETY: `call_once` only runs this closure once, ever. + let data = unsafe { &mut *this.data.get() }; + let f = unsafe { ManuallyDrop::take(&mut data.f) }; + let value = f(); + data.value = ManuallyDrop::new(value); + }); + }; + + // If the internal `Once` is poisoned, it will panic with "Once instance has previously been + // poisoned", but we want to panic with "LazyLock instance ..." instead. + let result = panic::catch_unwind(AssertUnwindSafe(call_once_closure)); + + if let Err(payload) = result { + // Check if this is the `Once` poison panic. + if let Some(s) = payload.downcast_ref::() + && s == ONCE_POISON_PANIC_MSG + { + panic_poisoned(); + } else { + // This panic came from the closure `f`, so we don't need to change the message. + panic::resume_unwind(payload); + } + } // SAFETY: // There are four possible scenarios: // * the closure was called and initialized `value`. - // * the closure was called and panicked, so this point is never reached. + // * the closure was called and panicked, which we handled above. // * the closure was not called, but a previous call initialized `value`. - // * the closure was not called because the Once is poisoned, so this point - // is never reached. + // * the closure was not called because the Once is poisoned, which we handled above. // So `value` has definitely been initialized and will not be modified again. unsafe { &*(*this.data.get()).value } } diff --git a/library/std/src/sys/sync/mod.rs b/library/std/src/sys/sync/mod.rs index 0691e96785198..b2bbf083cc76f 100644 --- a/library/std/src/sys/sync/mod.rs +++ b/library/std/src/sys/sync/mod.rs @@ -1,6 +1,6 @@ mod condvar; mod mutex; -mod once; +pub(crate) mod once; // For `ONCE_POISON_PANIC_MSG`. mod once_box; mod rwlock; mod thread_parking; diff --git a/library/std/src/sys/sync/once/futex.rs b/library/std/src/sys/sync/once/futex.rs index 407fdcebcf5cc..7f2f302632f0f 100644 --- a/library/std/src/sys/sync/once/futex.rs +++ b/library/std/src/sys/sync/once/futex.rs @@ -112,7 +112,7 @@ impl Once { COMPLETE => return, POISONED if !ignore_poisoning => { // Panic to propagate the poison. - panic!("Once instance has previously been poisoned"); + panic!("{}", super::ONCE_POISON_PANIC_MSG); } _ => { // Set the QUEUED bit if it has not already been set. @@ -147,7 +147,7 @@ impl Once { COMPLETE => return, POISONED if !ignore_poisoning => { // Panic to propagate the poison. - panic!("Once instance has previously been poisoned"); + panic!("{}", super::ONCE_POISON_PANIC_MSG); } INCOMPLETE | POISONED => { // Try to register the current thread as the one running. diff --git a/library/std/src/sys/sync/once/mod.rs b/library/std/src/sys/sync/once/mod.rs index 0e38937b1219a..e99a9ff5b586e 100644 --- a/library/std/src/sys/sync/once/mod.rs +++ b/library/std/src/sys/sync/once/mod.rs @@ -7,6 +7,11 @@ // This also gives us the opportunity to optimize the implementation a bit which // should help the fast path on call sites. +/// The panic message used when a `Once` is accessed after being poisoned. +/// +/// This message is checked by `LazyLock` to detect when a `Once` has been poisoned. +pub(crate) const ONCE_POISON_PANIC_MSG: &str = "Once instance has previously been poisoned"; + cfg_if::cfg_if! { if #[cfg(any( all(target_os = "windows", not(target_vendor="win7")), diff --git a/library/std/src/sys/sync/once/no_threads.rs b/library/std/src/sys/sync/once/no_threads.rs index 2568059cfe3a8..31d9bad345a52 100644 --- a/library/std/src/sys/sync/once/no_threads.rs +++ b/library/std/src/sys/sync/once/no_threads.rs @@ -76,7 +76,7 @@ impl Once { match state { State::Poisoned if !ignore_poisoning => { // Panic to propagate the poison. - panic!("Once instance has previously been poisoned"); + panic!("{}", super::ONCE_POISON_PANIC_MSG); } State::Incomplete | State::Poisoned => { self.state.set(State::Running); diff --git a/library/std/src/sys/sync/once/queue.rs b/library/std/src/sys/sync/once/queue.rs index 49e15d65f25a2..1a5d527ee5a61 100644 --- a/library/std/src/sys/sync/once/queue.rs +++ b/library/std/src/sys/sync/once/queue.rs @@ -159,7 +159,7 @@ impl Once { COMPLETE => return, POISONED if !ignore_poisoning => { // Panic to propagate the poison. - panic!("Once instance has previously been poisoned"); + panic!("{}", super::ONCE_POISON_PANIC_MSG); } _ => { current = wait(&self.state_and_queue, current, !ignore_poisoning); @@ -189,7 +189,7 @@ impl Once { COMPLETE => break, POISONED if !ignore_poisoning => { // Panic to propagate the poison. - panic!("Once instance has previously been poisoned"); + panic!("{}", super::ONCE_POISON_PANIC_MSG); } POISONED | INCOMPLETE => { // Try to register this thread as the one RUNNING. diff --git a/library/std/tests/sync/lazy_lock.rs b/library/std/tests/sync/lazy_lock.rs index 6c14b79f2ce7c..9f6a65154f268 100644 --- a/library/std/tests/sync/lazy_lock.rs +++ b/library/std/tests/sync/lazy_lock.rs @@ -165,3 +165,32 @@ fn lazy_force_mut() { p.clear(); LazyLock::force_mut(&mut lazy); } + +/// Verifies that when a `LazyLock` is poisoned, it panics with the correct error message ("LazyLock +/// instance has previously been poisoned") instead of the underlying `Once` error message. +#[test] +#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] +#[should_panic(expected = "LazyLock instance has previously been poisoned")] +fn lazy_lock_poison_message() { + let lazy: LazyLock = LazyLock::new(|| panic!("initialization failed")); + + // First access will panic during initialization. + let _ = panic::catch_unwind(|| { + let _ = &*lazy; + }); + + // Second access should panic with the poisoned message. + let _ = &*lazy; +} + +// Verifies that when the initialization closure panics with a custom message, that message is +// preserved and not overridden by `LazyLock`. +#[test] +#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] +#[should_panic(expected = "custom panic message from closure")] +fn lazy_lock_preserves_closure_panic_message() { + let lazy: LazyLock = LazyLock::new(|| panic!("custom panic message from closure")); + + // This should panic with the original message from the closure. + let _ = &*lazy; +}