Skip to content

Commit 4067da0

Browse files
committed
fix(ty_visitor): drop global panic hook swap, keep catch_unwind
TIL (thanks Copilot!), the previous approach temporarily replaced the process-wide panic hook with a no-op to suppress backtraces from caught layout panics. Turns out that's a thread-safety footgun: rustc's own worker threads could panic while the no-op hook is installed, silently swallowing unrelated diagnostics. The hook swap was also racy with anything else that calls set_hook concurrently. catch_unwind is what actually keeps the process alive; the hook swap was purely cosmetic (suppressing stderr noise). Dropped it entirely and accepted the default backtrace output for caught panics. The end-of-run LayoutPanic summary still reports everything it did before. My research surfaced a way to suppress the stderr noise that I decided was too much for a little noise, but I'm including here for completeness. Set teh hook once at startup (not per-call) to a hook that checks a thread-local flag: ```rust thread_local! { static SUPPRESS_PANIC_OUTPUT: Cell<bool> = const { Cell::new(false) }; } // Called once, e.g. in driver setup: std::panic::set_hook(Box::new(|info| { SUPPRESS_PANIC_OUTPUT.with(|flag| { if !flag.get() { eprintln!("{info}"); } }); })); // Then in try_layout_shape, just toggle the flag: SUPPRESS_PANIC_OUTPUT.with(|f| f.set(true)); let result = catch_unwind(AssertUnwindSafe(|| ty.layout()...)); SUPPRESS_PANIC_OUTPUT.with(|f| f.set(false)); This is thread-safe (thread-local, not global), no race conditions, no risk of swallowing other threads' panics, and our collected LayoutPanic report at teh end works exactly as before.
1 parent 1829855 commit 4067da0

1 file changed

Lines changed: 7 additions & 6 deletions

File tree

src/printer/ty_visitor.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use crate::compat::stable_mir;
1313

1414
use std::collections::{HashMap, HashSet};
1515
use std::ops::ControlFlow;
16-
use std::panic::{catch_unwind, set_hook, take_hook, AssertUnwindSafe};
16+
use std::panic::{catch_unwind, AssertUnwindSafe};
1717

1818
use stable_mir::mir::mono::Instance;
1919
use stable_mir::ty::{RigidTy, TyKind};
@@ -37,12 +37,13 @@ pub(super) struct LayoutPanic {
3737
fn try_layout_shape(
3838
ty: &stable_mir::ty::Ty,
3939
) -> Result<Option<stable_mir::abi::LayoutShape>, String> {
40-
// Temporarily suppress the default panic hook so caught panics don't
41-
// spray backtraces to stderr; we report them in our own summary.
42-
let prev_hook = take_hook();
43-
set_hook(Box::new(|_| {}));
40+
// catch_unwind keeps the process alive when rustc's layout engine panics
41+
// (e.g. on certain `dyn Trait` types). The default panic hook will still
42+
// print a backtrace to stderr for each caught panic; that's noisy but
43+
// harmless, and avoids the thread-safety issues of swapping the global
44+
// hook per-call. We collect the messages and report them in our own
45+
// summary at the end.
4446
let result = catch_unwind(AssertUnwindSafe(|| ty.layout().ok().map(|l| l.shape())));
45-
set_hook(prev_hook);
4647

4748
match result {
4849
Ok(shape) => Ok(shape),

0 commit comments

Comments
 (0)