From cdef1397102ae3d00a7656d2f71dc216442e219a Mon Sep 17 00:00:00 2001 From: SpecificProtagonist Date: Mon, 10 Mar 2025 22:16:14 +0100 Subject: [PATCH] Backtrace: std and threadsafe bevy_error_panic_hook (#18235) # Objective Make `bevy_error_panic_hook` threadsafe. As it relies on a global variable, it fails when multiple threads panic. ## Solution Switch from a global variable for storing whether an error message was printed to a thread-local one. `thread_local` is in `std`; the `backtrace` already relies on `std` APIs. It didn't depend on the `std` feature though, so I've added that. I've also put `bevy_error_panic_hook` behind the `backtrace` feature, since it relies on the thread local variable, which fixes #18231. ## Testing The following now loops instead of crashing: ```rust std::thread::scope(|s| { use bevy_ecs::error::*; #[derive(Debug)] struct E; impl std::fmt::Display for E { fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { todo!() } } impl std::error::Error for E {} std::panic::set_hook(Box::new(bevy_error_panic_hook(|_| { unreachable!(); }))); for _ in 0..2 { s.spawn(|| { loop { let _ = std::panic::catch_unwind(|| { panic!("{:?}", BevyError::from(E)); }); } }); } }); ``` --- crates/bevy_ecs/Cargo.toml | 2 +- crates/bevy_ecs/src/error/bevy_error.rs | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index 9a9c15de9f..2b8d2f0d54 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -37,7 +37,7 @@ reflect_functions = ["bevy_reflect", "bevy_reflect/functions"] configurable_error_handler = [] ## Enables automatic backtrace capturing in BevyError -backtrace = [] +backtrace = ["std"] # Debugging Features diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index 73777bad77..9632d89bc7 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -117,7 +117,7 @@ impl Debug for BevyError { } if !full_backtrace { if std::thread::panicking() { - SKIP_NORMAL_BACKTRACE.store(1, core::sync::atomic::Ordering::Relaxed); + SKIP_NORMAL_BACKTRACE.set(true); } writeln!(f, "{FILTER_MESSAGE}")?; } @@ -132,22 +132,23 @@ impl Debug for BevyError { const FILTER_MESSAGE: &str = "note: Some \"noisy\" backtrace lines have been filtered out. Run with `BEVY_BACKTRACE=full` for a verbose backtrace."; #[cfg(feature = "backtrace")] -static SKIP_NORMAL_BACKTRACE: core::sync::atomic::AtomicUsize = - core::sync::atomic::AtomicUsize::new(0); +std::thread_local! { + static SKIP_NORMAL_BACKTRACE: core::cell::Cell = + const { core::cell::Cell::new(false) }; +} /// When called, this will skip the currently configured panic hook when a [`BevyError`] backtrace has already been printed. -#[cfg(feature = "std")] +#[cfg(feature = "backtrace")] pub fn bevy_error_panic_hook( current_hook: impl Fn(&std::panic::PanicHookInfo), ) -> impl Fn(&std::panic::PanicHookInfo) { move |info| { - if SKIP_NORMAL_BACKTRACE.load(core::sync::atomic::Ordering::Relaxed) > 0 { + if SKIP_NORMAL_BACKTRACE.replace(false) { if let Some(payload) = info.payload().downcast_ref::<&str>() { std::println!("{payload}"); } else if let Some(payload) = info.payload().downcast_ref::() { std::println!("{payload}"); } - SKIP_NORMAL_BACKTRACE.store(0, core::sync::atomic::Ordering::Relaxed); return; }