Skip to content

Commit d1f2324

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 3 different files if we want to change it. Signed-off-by: Connor Tsui <[email protected]>
1 parent 6ba0ce4 commit d1f2324

File tree

6 files changed

+76
-39
lines changed

6 files changed

+76
-39
lines changed

library/std/src/sync/lazy_lock.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,21 +244,31 @@ impl<T, F: FnOnce() -> T> LazyLock<T, F> {
244244
#[inline]
245245
#[stable(feature = "lazy_cell", since = "1.80.0")]
246246
pub fn force(this: &LazyLock<T, F>) -> &T {
247-
this.once.call_once(|| {
247+
let mut is_poisoned = false;
248+
249+
this.once.call_once_force(|state| {
250+
if state.is_poisoned() {
251+
is_poisoned = true;
252+
return; // Return early and call `panic_poisoned`.
253+
}
254+
248255
// SAFETY: `call_once` only runs this closure once, ever.
249256
let data = unsafe { &mut *this.data.get() };
250257
let f = unsafe { ManuallyDrop::take(&mut data.f) };
251258
let value = f();
252259
data.value = ManuallyDrop::new(value);
253260
});
254261

262+
if is_poisoned {
263+
panic_poisoned()
264+
}
265+
255266
// SAFETY:
256267
// There are four possible scenarios:
257268
// * the closure was called and initialized `value`.
258269
// * the closure was called and panicked, so this point is never reached.
259270
// * 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.
271+
// * the closure was not called because the Once is poisoned, which we handled above.
262272
// So `value` has definitely been initialized and will not be modified again.
263273
unsafe { &*(*this.data.get()).value }
264274
}

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: 53 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,6 @@ fn lazy_default() {
3333
assert_eq!(CALLED.load(SeqCst), 1);
3434
}
3535

36-
#[test]
37-
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
38-
fn lazy_poisoning() {
39-
let x: LazyCell<String> = LazyCell::new(|| panic!("kaboom"));
40-
for _ in 0..2 {
41-
let res = panic::catch_unwind(panic::AssertUnwindSafe(|| x.len()));
42-
assert!(res.is_err());
43-
}
44-
}
45-
4636
#[test]
4737
#[cfg_attr(any(target_os = "emscripten", target_os = "wasi"), ignore)] // no threads
4838
fn sync_lazy_new() {
@@ -123,16 +113,6 @@ fn static_sync_lazy_via_fn() {
123113
assert_eq!(xs(), &vec![1, 2, 3]);
124114
}
125115

126-
#[test]
127-
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
128-
fn sync_lazy_poisoning() {
129-
let x: LazyLock<String> = LazyLock::new(|| panic!("kaboom"));
130-
for _ in 0..2 {
131-
let res = panic::catch_unwind(|| x.len());
132-
assert!(res.is_err());
133-
}
134-
}
135-
136116
// Check that we can infer `T` from closure's type.
137117
#[test]
138118
fn lazy_type_inference() {
@@ -145,17 +125,6 @@ fn is_sync_send() {
145125
assert_traits::<LazyLock<String>>();
146126
}
147127

148-
#[test]
149-
#[should_panic = "has previously been poisoned"]
150-
fn lazy_force_mut_panic() {
151-
let mut lazy = LazyLock::<String>::new(|| panic!());
152-
panic::catch_unwind(panic::AssertUnwindSafe(|| {
153-
let _ = LazyLock::force_mut(&mut lazy);
154-
}))
155-
.unwrap_err();
156-
let _ = &*lazy;
157-
}
158-
159128
#[test]
160129
fn lazy_force_mut() {
161130
let s = "abc".to_owned();
@@ -165,3 +134,56 @@ fn lazy_force_mut() {
165134
p.clear();
166135
LazyLock::force_mut(&mut lazy);
167136
}
137+
138+
#[test]
139+
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
140+
fn lazy_poisoning() {
141+
let x: LazyCell<String> = LazyCell::new(|| panic!("kaboom"));
142+
for _ in 0..2 {
143+
let res = panic::catch_unwind(panic::AssertUnwindSafe(|| x.len()));
144+
assert!(res.is_err());
145+
}
146+
}
147+
148+
/// Verifies that when a `LazyLock` is poisoned, it panics with the correct error message ("LazyLock
149+
/// instance has previously been poisoned") instead of the underlying `Once` error message.
150+
#[test]
151+
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
152+
#[should_panic(expected = "LazyLock instance has previously been poisoned")]
153+
fn lazy_lock_deref_panic() {
154+
let lazy: LazyLock<String> = LazyLock::new(|| panic!("initialization failed"));
155+
156+
// First access will panic during initialization.
157+
let _ = panic::catch_unwind(panic::AssertUnwindSafe(|| {
158+
let _ = &*lazy;
159+
}));
160+
161+
// Second access should panic with the poisoned message.
162+
let _ = &*lazy;
163+
}
164+
165+
#[test]
166+
#[should_panic(expected = "LazyLock instance has previously been poisoned")]
167+
fn lazy_lock_deref_mut_panic() {
168+
let mut lazy: LazyLock<String> = LazyLock::new(|| panic!("initialization failed"));
169+
170+
// First access will panic during initialization.
171+
let _ = panic::catch_unwind(panic::AssertUnwindSafe(|| {
172+
let _ = LazyLock::force_mut(&mut lazy);
173+
}));
174+
175+
// Second access should panic with the poisoned message.
176+
let _ = &*lazy;
177+
}
178+
179+
// Verifies that when the initialization closure panics with a custom message, that message is
180+
// preserved and not overridden by `LazyLock`.
181+
#[test]
182+
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
183+
#[should_panic(expected = "custom panic message from closure")]
184+
fn lazy_lock_preserves_closure_panic_message() {
185+
let lazy: LazyLock<String> = LazyLock::new(|| panic!("custom panic message from closure"));
186+
187+
// This should panic with the original message from the closure.
188+
let _ = &*lazy;
189+
}

0 commit comments

Comments
 (0)