Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
229 changes: 112 additions & 117 deletions Cargo.lock

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ package = [
"dep:dotenvy"
]

async = [ "snarkvm-ledger/async", "snarkvm-synthesizer/async" ]
async = [ "snarkvm-ledger?/async", "snarkvm-synthesizer?/async", "snarkvm-utilities?/async" ]
cuda = [ "snarkvm-algorithms/cuda" ]
history = [ "snarkvm-synthesizer/history" ]
history-staking-rewards = [ "snarkvm-synthesizer/history-staking-rewards" ]
Expand Down Expand Up @@ -529,6 +529,9 @@ default-features = false
[workspace.dependencies.smallvec]
version = "1.14"

[workspace.dependencies.tokio]
version = "1"

[workspace.dependencies.tempfile]
version = "3.15"

Expand Down
2 changes: 1 addition & 1 deletion ledger/narwhal/data/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ features = [ "preserve_order" ]

[dependencies.tokio]
optional = true
version = "1"
workspace = true
features = [ "rt" ]

[dev-dependencies.snarkvm-ledger-block]
Expand Down
47 changes: 28 additions & 19 deletions synthesizer/src/vm/helpers/sequential_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
use crate::vm::*;
use console::network::prelude::Network;

use snarkvm_utilities::catch_unwind;

use std::{fmt, thread};
use tokio::sync::oneshot;

Expand All @@ -29,25 +31,32 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
// Spawn a dedicated thread.
let vm = self.clone();
thread::spawn(move || {
// Sequentially process incoming operations.
while let Ok(request) = request_rx.recv() {
let SequentialOperationRequest { op, response_tx } = request;
trace!("Sequentially processing operation '{op}'");

// Perform the queued operation.
let ret = match op {
SequentialOperation::AddNextBlock(block) => {
let ret = vm.add_next_block_inner(block);
SequentialOperationResult::AddNextBlock(ret)
}
SequentialOperation::AtomicSpeculate(a, b, c, d, e, f) => {
let ret = vm.atomic_speculate_inner(a, b, c, d, e, f);
SequentialOperationResult::AtomicSpeculate(ret)
}
};

// Relay the results of the operation to the caller.
let _ = response_tx.send(ret);
let result = catch_unwind(move || {
// Sequentially process incoming operations.
while let Ok(request) = request_rx.recv() {
let SequentialOperationRequest { op, response_tx } = request;
debug!("Sequentially processing operation '{op}'");

// Perform the queued operation.
let ret = match op {
SequentialOperation::AddNextBlock(block) => {
let ret = vm.add_next_block_inner(block);
SequentialOperationResult::AddNextBlock(ret)
}
SequentialOperation::AtomicSpeculate(a, b, c, d, e, f) => {
let ret = vm.atomic_speculate_inner(a, b, c, d, e, f);
SequentialOperationResult::AtomicSpeculate(ret)
}
};

// Relay the results of the operation to the caller.
let _ = response_tx.send(ret);
}
});

if let Err((msg, backtrace)) = result {
error!("Sequential ops thread encountered a fatal error: {msg}");
error!("Backtrace: {backtrace:?}");
}
})
}
Expand Down
6 changes: 6 additions & 0 deletions utilities/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ workspace = true
[dependencies.colored]
workspace = true

[dependencies.tokio]
workspace = true
optional = true
features = ["rt"]

[dependencies.num_cpus]
version = "1"

Expand Down Expand Up @@ -89,6 +94,7 @@ workspace = true

[features]
default = [ "derive" ]
async = [ "tokio" ]
derive = [ "snarkvm-utilities-derives" ]
dev-print = [ ]
serial = [ "derive" ]
Expand Down
181 changes: 176 additions & 5 deletions utilities/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,26 @@
// limitations under the License.

use colored::Colorize;
use std::borrow::Borrow;

use std::{
any::Any,
backtrace::Backtrace,
borrow::Borrow,
cell::Cell,
panic,
sync::atomic::{AtomicBool, Ordering},
};

thread_local! {
/// The message backtrace of the last panic on this thread (if any).
///
/// 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.
/// 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.
static PANIC_INFO: Cell<Option<(String, Backtrace)>> = const { Cell::new(None) };
}

/// Keeps track of whether a panic hook was installed already.
static PANIC_HOOK_INSTALLED: AtomicBool = const { AtomicBool::new(false) };

/// Generates an `io::Error` from the given string.
#[inline]
Expand Down Expand Up @@ -80,7 +99,30 @@ pub trait PrettyUnwrap {
fn pretty_expect<S: ToString>(self, context: S) -> Self::Inner;
}

/// Helper for `PrettyUnwrap`, which creates a panic with the `anyhow::Error` nicely formatted and also logs the panic.
/// Set the global panic hook for the process.
///
/// This function should be called once at startup. Subsequent calls to it have no effect.
pub fn set_panic_hook() {
// Check if the hook was already installed.
// Note, that this allows for a small race condition, where the hook is installed by another thread after the check, but before the load.
// However, that is safe as the installed hook will be indentical, and this check merely exists for performance reasons.
if PANIC_HOOK_INSTALLED.load(Ordering::Acquire) {
return;
}

// Install the hook.
std::panic::set_hook(Box::new(|err| {
let msg = err.to_string();
let trace = Backtrace::force_capture();
PANIC_INFO.with(move |info| info.set(Some((msg, trace))));
}));

// Mark the hook as installed.
PANIC_HOOK_INSTALLED.store(true, Ordering::Release);
}

/// Helper for `PrettyUnwrap`:
/// Creates a panic with the `anyhow::Error` nicely formatted.
#[track_caller]
#[inline]
fn pretty_panic(error: &anyhow::Error) -> ! {
Expand All @@ -97,6 +139,7 @@ impl<T> PrettyUnwrap for anyhow::Result<T> {
type Inner = T;

#[track_caller]
#[inline]
fn pretty_unwrap(self) -> Self::Inner {
match self {
Ok(result) => result,
Expand All @@ -117,9 +160,59 @@ impl<T> PrettyUnwrap for anyhow::Result<T> {
}
}

/// `try_vm_runtime` executes the given closure in an environment which will safely halt
/// without producing logs that look like unexpected behavior.
/// In debug mode, it prints to stderr using the format: "VM safely halted at {location}: {halt message}".
///
/// 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.
#[track_caller]
#[inline]
pub fn try_vm_runtime<R, F: FnMut() -> R>(f: F) -> Result<R, Box<dyn Any + Send>> {
// Perform the operation that may panic.
let result = std::panic::catch_unwind(panic::AssertUnwindSafe(f));

if result.is_err() {
// Get the stored panic and backtrace from the thread-local variable.
let (msg, _) = PANIC_INFO.with(|info| info.take()).expect("No panic information stored?");

#[cfg(debug_assertions)]
{
// Remove all words up to "panicked".
// And prepend with "VM Safely halted"
let msg = msg
.to_string()
.split_ascii_whitespace()
.skip_while(|&word| word != "panicked")
.collect::<Vec<&str>>()
.join(" ")
.replacen("panicked", "VM safely halted", 1);

eprintln!("{msg}");
}
#[cfg(not(debug_assertions))]
{
// Discard message
let _ = msg;
}
}

// Return the result, allowing regular error-handling.
result
}

/// `catch_unwind` calls the given closure `f` and, if `f` panics, returns the panic message and backtrace.
#[inline]
pub fn catch_unwind<R, F: FnMut() -> R>(f: F) -> Result<R, (String, Backtrace)> {
Comment thread
kaimast marked this conversation as resolved.
// Perform the operation that may panic.
std::panic::catch_unwind(panic::AssertUnwindSafe(f)).map_err(|_| {
// Get the stored panic and backtrace from the thread-local variable.
PANIC_INFO.with(|info| info.take()).expect("No panic information stored?")
})
}

#[cfg(test)]
mod tests {
use super::{PrettyUnwrap, flatten_error, pretty_panic};
use super::{PrettyUnwrap, catch_unwind, flatten_error, pretty_panic, set_panic_hook, try_vm_runtime};

use anyhow::{Context, Result, anyhow, bail};
use colored::Colorize;
Expand Down Expand Up @@ -177,14 +270,92 @@ mod tests {
assert_eq!(*result.downcast::<String>().expect("Error was not a string"), expected);
}

// Ensure catch_unwind stores the panic message as expected.
#[test]
fn test_catch_unwind() {
set_panic_hook();
let result = catch_unwind(move || {
panic!("This is my message");
});

let (msg, bt) = result.expect_err("No panic caught");
assert!(msg.ends_with("This is my message"));

// This function should be in the panics backtrace
assert!(bt.to_string().contains("test_catch_unwind"));
}

// Ensure top-level `catch_unwind` captures a non-VM panic with the correct message and backtrace.
//
// This mirrors the usage in the sequential ops thread, where `catch_unwind` wraps the entire
// thread body and may catch panics unrelated to `try_vm_runtime` (e.g. storage panics).
#[test]
fn test_top_level_panic_captured() {
set_panic_hook();
let result = catch_unwind(|| {
panic!("Non-VM top-level panic");
});
let (msg, bt) = result.expect_err("Should have caught a panic");
assert!(msg.ends_with("Non-VM top-level panic"), "Unexpected message: {msg}");
assert!(bt.to_string().contains("test_top_level_panic_captured"), "Backtrace missing caller");
}

// Ensure `catch_unwind` correctly captures a fresh top-level panic after `try_vm_runtime`
// has already consumed a VM panic from `PANIC_INFO`.
#[test]
fn test_catch_unwind_after_vm_panic() {
set_panic_hook();

// Simulate a VM panic caught and consumed by `try_vm_runtime`.
let vm_result = try_vm_runtime(|| panic!("VM execution failed"));
assert!(vm_result.is_err(), "try_vm_runtime should catch VM panics");

// A subsequent top-level panic must be captured with fresh data, not stale VM info.
let result = catch_unwind(|| {
panic!("Subsequent top-level panic");
});
let (msg, _) = result.expect_err("Should have caught a panic");
assert!(msg.ends_with("Subsequent top-level panic"), "Got stale or wrong message: {msg}");
}

// Ensure a top-level panic (not caught by our wrappers) still propagates normally
// when the panic hook is installed, i.e. the hook does not swallow panics.
#[test]
fn test_top_level_panic_propagates_with_hook() {
set_panic_hook();

// Use std::panic::catch_unwind directly so we can observe propagation without
// going through our wrappers.
let result = std::panic::catch_unwind(|| {
panic!("Propagating top-level panic");
});
assert!(result.is_err(), "Panic should propagate to the caller");
}

// Ensure `catch_unwind` captures top-level panics correctly even when a preceding
// `try_vm_runtime` completed successfully (no prior panic in PANIC_INFO).
#[test]
fn test_catch_unwind_after_successful_vm_runtime() {
set_panic_hook();

// try_vm_runtime succeeds without panicking.
let vm_result = try_vm_runtime(|| 42u32);
assert_eq!(vm_result.unwrap(), 42);

// A top-level panic that follows should still be captured correctly.
let result = catch_unwind(|| panic!("Top-level panic after successful VM run"));
let (msg, _) = result.expect_err("Should have caught a panic");
assert!(msg.ends_with("Top-level panic after successful VM run"), "Got: {msg}");
}

/// Ensure catch_unwind does not break `try_vm_runtime`.
#[test]
fn test_nested_with_try_vm_runtime() {
use crate::try_vm_runtime;
set_panic_hook();

let result = std::panic::catch_unwind(|| {
// try_vm_runtime uses catch_unwind internally
let vm_result = try_vm_runtime!(|| {
let vm_result = try_vm_runtime(|| {
panic!("VM operation failed!");
});

Expand Down
22 changes: 19 additions & 3 deletions utilities/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ pub use bytes::*;
pub mod defer;
pub use defer::*;

mod vm_error;
pub use vm_error::*;

pub mod iterator;
pub use iterator::*;

Expand All @@ -58,5 +55,24 @@ pub use serialize::*;
pub mod errors;
pub use errors::*;

#[cfg(feature = "async")]
/// Helpers to spawn async tasks.
pub mod task;
#[cfg(feature = "async")]
pub use task::*;

/// Use old name for backward-compatibility.
pub use errors::io_error as error;

/// This macro provides a VM runtime environment which will safely halt
/// without producing logs that look like unexpected behavior.
/// In debug mode, it prints to stderr using the format: "VM safely halted at {location}: {halt message}".
///
/// It is more efficient to set the panic hook once and directly use `errors::try_vm_runtime`.
#[macro_export]
macro_rules! try_vm_runtime {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't all the finalize instances still call the macro try_vm_runtime! instead of errors::try_vm_runtime()?

This would call set_panic_hook multiple times rather than the one time you claim "Should be called exactly once."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I clarified the documentation that it is safe to call set_panic_hook multiple times. It will just not have any effect after the first invocation. The most recent commit also adds a boolean flag that is set once the handler is installed, to make sure there is no performance impact of calling set_panic_hook multiple times.

It would be great if we could replace that macro eventually, but that would require users of snarkVM to call set_panic_hook themselves.

($e:expr) => {{
$crate::errors::set_panic_hook();
$crate::errors::try_vm_runtime($e)
}};
}
Loading