Skip to content

Commit cec41dd

Browse files
committed
feat(utils/error): make catching panics work correctly in a multi-threaded setting
1 parent 1f45c10 commit cec41dd

File tree

3 files changed

+101
-64
lines changed

3 files changed

+101
-64
lines changed

utilities/src/errors.rs

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,14 @@
1515

1616
use colored::Colorize;
1717

18-
use std::{borrow::Borrow, fmt::Display};
18+
use std::{any::Any, backtrace::Backtrace, borrow::Borrow, cell::Cell, fmt::Display, panic};
19+
20+
thread_local! {
21+
/// The message backtrace of the last panic on this thread (if any).
22+
///
23+
/// We store this information here instead of directly processing it in a panic hook, because panic hooks are global whereas this can be processed on a per-thread basis.
24+
/// For example, one thread may execute a program where panics should *not* cause the entire process to terminate, while in another thread there is a panic due to a bug.
25+
static PANIC_INFO: Cell<Option<(String, Backtrace)>> = const { Cell::new(None) };}
1926

2027
/// Generates an `io::Error` from the given string.
2128
pub fn io_error<S: ToString>(err: S) -> std::io::Error {
@@ -127,6 +134,17 @@ pub trait PrettyUnwrap {
127134
fn pretty_unwrap(self) -> Self::Inner;
128135
}
129136

137+
/// Set the global panic hook for process. Should be called exactly once.
138+
pub fn set_panic_hook() {
139+
std::panic::set_hook(Box::new(|err| {
140+
let msg = err.to_string();
141+
142+
println!("Hook: {msg}");
143+
let trace = Backtrace::force_capture();
144+
PANIC_INFO.with(move |info| info.set(Some((msg, trace))));
145+
}));
146+
}
147+
130148
/// Helper for `PrettyUnwrap`:
131149
/// Creates a panic with the `anyhow::Error` nicely formatted.
132150
#[track_caller]
@@ -155,9 +173,58 @@ impl<T> PrettyUnwrap for anyhow::Result<T> {
155173
}
156174
}
157175

176+
/// `try_vm_runtime` executes the given closure in an environment which will safely halt
177+
/// without producing logs that look like unexpected behavior.
178+
/// In debug mode, it prints to stderr using the format: "VM safely halted at {location}: {halt message}".
179+
///
180+
/// Note: For this to work as expected, panics must be set to `unwind` during compilation (default), and the closure cannot invoke any async code that may potentially execute in a different OS thread.
181+
#[inline]
182+
#[track_caller]
183+
pub fn try_vm_runtime<R, F: FnMut() -> R>(f: F) -> Result<R, Box<dyn Any + Send>> {
184+
// Perform the operation that may panic.
185+
let result = std::panic::catch_unwind(panic::AssertUnwindSafe(f));
186+
187+
if result.is_err() {
188+
// Get the stored panic and backtrace from the thread-local variable.
189+
let (msg, _) = PANIC_INFO.with(|info| info.take()).expect("No panic information stored?");
190+
191+
#[cfg(debug_assertions)]
192+
{
193+
// Remove all words up to "panicked".
194+
// And prepend with "VM Safely halted"
195+
let msg = msg
196+
.to_string()
197+
.split_ascii_whitespace()
198+
.skip_while(|&word| word != "panicked")
199+
.collect::<Vec<&str>>()
200+
.join(" ")
201+
.replacen("panicked", "VM safely halted", 1);
202+
203+
eprintln!("{msg}");
204+
}
205+
#[cfg(not(debug_assertions))]
206+
{
207+
// Discard message
208+
let _ = msg;
209+
}
210+
}
211+
212+
// Return the result, allowing regular error-handling.
213+
result
214+
}
215+
216+
/// `catch_unwind` calls the given closure `f` and, if `f` panics, returns the panic message and backtrace.
217+
pub fn catch_unwind<R, F: FnMut() -> R>(f: F) -> Result<R, (String, Backtrace)> {
218+
// Perform the operation that may panic.
219+
std::panic::catch_unwind(panic::AssertUnwindSafe(f)).map_err(|_| {
220+
// Get the stored panic and backtrace from the thread-local variable.
221+
PANIC_INFO.with(|info| info.take()).expect("No panic information stored?")
222+
})
223+
}
224+
158225
#[cfg(test)]
159226
mod tests {
160-
use super::{PrettyUnwrap, flatten_anyhow_error, pretty_panic};
227+
use super::{PrettyUnwrap, catch_unwind, flatten_anyhow_error, pretty_panic, set_panic_hook, try_vm_runtime};
161228

162229
use anyhow::{Context, Result, anyhow, bail};
163230
use colored::Colorize;
@@ -215,14 +282,31 @@ mod tests {
215282
assert_eq!(*result.downcast::<String>().expect("Error was not a string"), expected);
216283
}
217284

285+
// Ensure catch_unwind stores the panic message as expected.
286+
#[test]
287+
fn test_catch_unwind() {
288+
set_panic_hook();
289+
let result = catch_unwind(move || {
290+
panic!("This is my message");
291+
});
292+
// Remove hook so test asserts work normally again.
293+
let _ = std::panic::take_hook();
294+
295+
let (msg, bt) = result.expect_err("No panic caught");
296+
assert!(msg.ends_with("This is my message"));
297+
298+
// This function should be in the panics backtrace
299+
assert!(bt.to_string().contains("test_catch_unwind"));
300+
}
301+
218302
/// Ensure catch_unwind does not break `try_vm_runtime`.
219303
#[test]
220304
fn test_nested_with_try_vm_runtime() {
221-
use crate::try_vm_runtime;
305+
set_panic_hook();
222306

223307
let result = std::panic::catch_unwind(|| {
224308
// try_vm_runtime uses catch_unwind internally
225-
let vm_result = try_vm_runtime!(|| {
309+
let vm_result = try_vm_runtime(|| {
226310
panic!("VM operation failed!");
227311
});
228312

utilities/src/lib.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@ pub use bytes::*;
3434
pub mod defer;
3535
pub use defer::*;
3636

37-
mod vm_error;
38-
pub use vm_error::*;
39-
4037
pub mod iterator;
4138
pub use iterator::*;
4239

@@ -63,3 +60,16 @@ pub use task::*;
6360

6461
/// Use old name for backward-compatibility.
6562
pub use errors::io_error as error;
63+
64+
/// This macro provides a VM runtime environment which will safely halt
65+
/// without producing logs that look like unexpected behavior.
66+
/// In debug mode, it prints to stderr using the format: "VM safely halted at {location}: {halt message}".
67+
///
68+
/// It is more efficient to set the panic hook once and directly use `errors::try_vm_runtime`.
69+
#[macro_export]
70+
macro_rules! try_vm_runtime {
71+
($e:expr) => {{
72+
$crate::errors::set_panic_hook();
73+
$crate::errors::try_vm_runtime($e)
74+
}};
75+
}

utilities/src/vm_error.rs

Lines changed: 0 additions & 57 deletions
This file was deleted.

0 commit comments

Comments
 (0)