Fall back on previous panic hook when not in catch_unwind wrapper (#15319)

This fixes #15317. Our `catch_unwind` wrapper installs a panic hook that
captures (the rendered contents of) the panic info when a panic occurs.
Since the intent is that the caller will render the panic info in some
custom way, the hook silences the default stderr panic output.

However, the panic hook is a global resource, so if any one thread was
in the middle of a `catch_unwind` call, we would silence the default
panic output for _all_ threads.

The solution is to also keep a thread local that indicates whether the
current thread is in the middle of our `catch_unwind`, and to fall back
on the default panic hook if not.

## Test Plan

Artificially added an mdtest parse error, ran tests via `cargo test -p
red_knot_python_semantic` to run a large number of tests in parallel.
Before this patch, the panic message was swallowed as reported in
#15317. After, the panic message was shown.
This commit is contained in:
Douglas Creager 2025-01-08 11:34:51 -05:00 committed by GitHub
parent 450d4e0e0c
commit 2ca31e4b43
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -1,3 +1,6 @@
use std::cell::Cell;
use std::sync::OnceLock;
#[derive(Default, Debug)]
pub struct PanicError {
pub info: String,
@ -16,31 +19,61 @@ impl std::fmt::Display for PanicError {
}
thread_local! {
static LAST_PANIC: std::cell::Cell<Option<PanicError>> = const { std::cell::Cell::new(None) };
static CAPTURE_PANIC_INFO: Cell<bool> = const { Cell::new(false) };
static OUR_HOOK_RAN: Cell<bool> = const { Cell::new(false) };
static LAST_PANIC: Cell<Option<PanicError>> = const { Cell::new(None) };
}
/// [`catch_unwind`](std::panic::catch_unwind) wrapper that sets a custom [`set_hook`](std::panic::set_hook)
/// to extract the backtrace. The original panic-hook gets restored before returning.
fn install_hook() {
static ONCE: OnceLock<()> = OnceLock::new();
ONCE.get_or_init(|| {
let prev = std::panic::take_hook();
std::panic::set_hook(Box::new(move |info| {
OUR_HOOK_RAN.with(|cell| cell.set(true));
let should_capture = CAPTURE_PANIC_INFO.with(Cell::get);
if !should_capture {
return (*prev)(info);
}
let info = info.to_string();
let backtrace = std::backtrace::Backtrace::force_capture();
LAST_PANIC.with(|cell| {
cell.set(Some(PanicError {
info,
backtrace: Some(backtrace),
}));
});
}));
});
}
/// Invokes a closure, capturing and returning the cause of an unwinding panic if one occurs.
///
/// ### Thread safety
///
/// This is implemented by installing a custom [panic hook](std::panic::set_hook). This panic hook
/// is a global resource. The hook that we install captures panic info in a thread-safe manner,
/// and also ensures that any threads that are _not_ currently using this `catch_unwind` wrapper
/// still use the previous hook (typically the default hook, which prints out panic information to
/// stderr).
///
/// We assume that there is nothing else running in this process that needs to install a competing
/// panic hook. We are careful to install our custom hook only once, and we do not ever restore
/// the previous hook (since you can always retain the previous hook's behavior by not calling this
/// wrapper).
pub fn catch_unwind<F, R>(f: F) -> Result<R, PanicError>
where
F: FnOnce() -> R + std::panic::UnwindSafe,
{
let prev = std::panic::take_hook();
std::panic::set_hook(Box::new(|info| {
let info = info.to_string();
let backtrace = std::backtrace::Backtrace::force_capture();
LAST_PANIC.with(|cell| {
cell.set(Some(PanicError {
info,
backtrace: Some(backtrace),
}));
});
}));
let result = std::panic::catch_unwind(f)
.map_err(|_| LAST_PANIC.with(std::cell::Cell::take).unwrap_or_default());
std::panic::set_hook(prev);
install_hook();
OUR_HOOK_RAN.with(|cell| cell.set(false));
let prev_should_capture = CAPTURE_PANIC_INFO.with(|cell| cell.replace(true));
let result = std::panic::catch_unwind(f).map_err(|_| {
let our_hook_ran = OUR_HOOK_RAN.with(Cell::get);
if !our_hook_ran {
panic!("detected a competing panic hook");
}
LAST_PANIC.with(Cell::take).unwrap_or_default()
});
CAPTURE_PANIC_INFO.with(|cell| cell.set(prev_should_capture));
result
}