Skip to content

Commit 3ead751

Browse files
committed
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 <[email protected]>
1 parent 9eb4a26 commit 3ead751

File tree

7 files changed

+69
-17
lines changed

7 files changed

+69
-17
lines changed

library/std/src/sync/lazy_lock.rs

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ use super::poison::once::ExclusiveState;
22
use crate::cell::UnsafeCell;
33
use crate::mem::ManuallyDrop;
44
use crate::ops::{Deref, DerefMut};
5-
use crate::panic::{RefUnwindSafe, UnwindSafe};
5+
use crate::panic::{self, AssertUnwindSafe, RefUnwindSafe, UnwindSafe};
66
use crate::sync::Once;
7+
use crate::sys::sync::once::ONCE_POISON_PANIC_MSG;
78
use crate::{fmt, ptr};
89

910
// We use the state of a Once as discriminant value. Upon creation, the state is
@@ -244,21 +245,38 @@ impl<T, F: FnOnce() -> T> LazyLock<T, F> {
244245
#[inline]
245246
#[stable(feature = "lazy_cell", since = "1.80.0")]
246247
pub fn force(this: &LazyLock<T, F>) -> &T {
247-
this.once.call_once(|| {
248-
// SAFETY: `call_once` only runs this closure once, ever.
249-
let data = unsafe { &mut *this.data.get() };
250-
let f = unsafe { ManuallyDrop::take(&mut data.f) };
251-
let value = f();
252-
data.value = ManuallyDrop::new(value);
253-
});
248+
let call_once_closure = || {
249+
this.once.call_once(|| {
250+
// SAFETY: `call_once` only runs this closure once, ever.
251+
let data = unsafe { &mut *this.data.get() };
252+
let f = unsafe { ManuallyDrop::take(&mut data.f) };
253+
let value = f();
254+
data.value = ManuallyDrop::new(value);
255+
});
256+
};
257+
258+
// If the internal `Once` is poisoned, it will panic with "Once instance has previously been
259+
// poisoned", but we want to panic with "LazyLock instance ..." instead.
260+
let result = panic::catch_unwind(AssertUnwindSafe(call_once_closure));
261+
262+
if let Err(payload) = result {
263+
// Check if this is the `Once` poison panic.
264+
if let Some(s) = payload.downcast_ref::<String>()
265+
&& s == ONCE_POISON_PANIC_MSG
266+
{
267+
panic_poisoned();
268+
} else {
269+
// This panic came from the closure `f`, so we don't need to change the message.
270+
panic::resume_unwind(payload);
271+
}
272+
}
254273

255274
// SAFETY:
256275
// There are four possible scenarios:
257276
// * the closure was called and initialized `value`.
258-
// * the closure was called and panicked, so this point is never reached.
277+
// * the closure was called and panicked, which we handled above.
259278
// * the closure was not called, but a previous call initialized `value`.
260-
// * the closure was not called because the Once is poisoned, so this point
261-
// is never reached.
279+
// * the closure was not called because the Once is poisoned, which we handled above.
262280
// So `value` has definitely been initialized and will not be modified again.
263281
unsafe { &*(*this.data.get()).value }
264282
}

library/std/src/sys/sync/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
mod condvar;
22
mod mutex;
3-
mod once;
3+
pub(crate) mod once; // For `ONCE_POISON_PANIC_MSG`.
44
mod once_box;
55
mod rwlock;
66
mod thread_parking;

library/std/src/sys/sync/once/futex.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ impl Once {
112112
COMPLETE => return,
113113
POISONED if !ignore_poisoning => {
114114
// Panic to propagate the poison.
115-
panic!("Once instance has previously been poisoned");
115+
panic!("{}", super::ONCE_POISON_PANIC_MSG);
116116
}
117117
_ => {
118118
// Set the QUEUED bit if it has not already been set.
@@ -147,7 +147,7 @@ impl Once {
147147
COMPLETE => return,
148148
POISONED if !ignore_poisoning => {
149149
// Panic to propagate the poison.
150-
panic!("Once instance has previously been poisoned");
150+
panic!("{}", super::ONCE_POISON_PANIC_MSG);
151151
}
152152
INCOMPLETE | POISONED => {
153153
// Try to register the current thread as the one running.

library/std/src/sys/sync/once/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77
// This also gives us the opportunity to optimize the implementation a bit which
88
// should help the fast path on call sites.
99

10+
/// The panic message used when a `Once` is accessed after being poisoned.
11+
///
12+
/// This message is checked by `LazyLock` to detect when a `Once` has been poisoned.
13+
pub(crate) const ONCE_POISON_PANIC_MSG: &str = "Once instance has previously been poisoned";
14+
1015
cfg_select! {
1116
any(
1217
all(target_os = "windows", not(target_vendor="win7")),

library/std/src/sys/sync/once/no_threads.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ impl Once {
7676
match state {
7777
State::Poisoned if !ignore_poisoning => {
7878
// Panic to propagate the poison.
79-
panic!("Once instance has previously been poisoned");
79+
panic!("{}", super::ONCE_POISON_PANIC_MSG);
8080
}
8181
State::Incomplete | State::Poisoned => {
8282
self.state.set(State::Running);

library/std/src/sys/sync/once/queue.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ impl Once {
159159
COMPLETE => return,
160160
POISONED if !ignore_poisoning => {
161161
// Panic to propagate the poison.
162-
panic!("Once instance has previously been poisoned");
162+
panic!("{}", super::ONCE_POISON_PANIC_MSG);
163163
}
164164
_ => {
165165
current = wait(&self.state_and_queue, current, !ignore_poisoning);
@@ -189,7 +189,7 @@ impl Once {
189189
COMPLETE => break,
190190
POISONED if !ignore_poisoning => {
191191
// Panic to propagate the poison.
192-
panic!("Once instance has previously been poisoned");
192+
panic!("{}", super::ONCE_POISON_PANIC_MSG);
193193
}
194194
POISONED | INCOMPLETE => {
195195
// Try to register this thread as the one RUNNING.

library/std/tests/sync/lazy_lock.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,3 +165,32 @@ fn lazy_force_mut() {
165165
p.clear();
166166
LazyLock::force_mut(&mut lazy);
167167
}
168+
169+
/// Verifies that when a `LazyLock` is poisoned, it panics with the correct error message ("LazyLock
170+
/// instance has previously been poisoned") instead of the underlying `Once` error message.
171+
#[test]
172+
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
173+
#[should_panic(expected = "LazyLock instance has previously been poisoned")]
174+
fn lazy_lock_poison_message() {
175+
let lazy: LazyLock<String> = LazyLock::new(|| panic!("initialization failed"));
176+
177+
// First access will panic during initialization.
178+
let _ = panic::catch_unwind(|| {
179+
let _ = &*lazy;
180+
});
181+
182+
// Second access should panic with the poisoned message.
183+
let _ = &*lazy;
184+
}
185+
186+
// Verifies that when the initialization closure panics with a custom message, that message is
187+
// preserved and not overridden by `LazyLock`.
188+
#[test]
189+
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
190+
#[should_panic(expected = "custom panic message from closure")]
191+
fn lazy_lock_preserves_closure_panic_message() {
192+
let lazy: LazyLock<String> = LazyLock::new(|| panic!("custom panic message from closure"));
193+
194+
// This should panic with the original message from the closure.
195+
let _ = &*lazy;
196+
}

0 commit comments

Comments
 (0)