Skip to content

Commit

Permalink
[fix] #3155: Fix panic-hook for tests
Browse files Browse the repository at this point in the history
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
  • Loading branch information
Arjentix authored and appetrosyan committed Feb 15, 2023
1 parent fe826be commit 4ee707a
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 46 deletions.
102 changes: 65 additions & 37 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ dev-telemetry = ["iroha_core/dev-telemetry", "iroha_telemetry"]
schema-endpoint = ["iroha_schema_gen"]
# Support internal testing infrastructure for integration tests.
# Disable in production.
test-network = []
test-network = ["thread-local-panic-hook"]

[badges]
is-it-maintained-issue-resolution = { repository = "https://github.com/hyperledger/iroha" }
Expand Down Expand Up @@ -65,6 +65,7 @@ serial_test = "0.8.0"
lazy_static = "1.4.0"
owo-colors = { version = "3.5.0", features = ["supports-colors"] }
supports-color = "2.0.0"
thread-local-panic-hook = { version = "0.1.0", optional = true }

[build-dependencies]
anyhow = "1.0.68"
Expand Down
41 changes: 33 additions & 8 deletions cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
clippy::std_instead_of_core,
clippy::std_instead_of_alloc
)]
use std::{panic, sync::Arc};
use std::sync::Arc;

use color_eyre::eyre::{eyre, Result, WrapErr};
use eyre::ContextCompat as _;
Expand Down Expand Up @@ -221,10 +221,30 @@ impl Iroha {
}

fn prepare_panic_hook(notify_shutdown: Arc<Notify>) {
let hook = panic::take_hook();
panic::set_hook(Box::new(move |info| {
hook(info);

#[cfg(not(feature = "test-network"))]
use std::panic::set_hook;

// This is a hot-fix for tests
//
// # Problem
//
// When running tests in parallel `std::panic::set_hook()` will be set
// the same for all threads. That means, that panic in one test can
// cause another test shutdown, which we don't want.
//
// # Downside
//
// A downside of this approach is that this panic hook will not work for
// threads created by Iroha itself (e.g. Sumeragi thread).
//
// # TODO
//
// Remove this when all Rust integrations tests will be converted to a
// separate Python tests.
#[cfg(feature = "test-network")]
use thread_local_panic_hook::set_hook;

set_hook(Box::new(move |info| {
// What clippy suggests is much less readable in this case
#[allow(clippy::option_if_let_else)]
let panic_message = if let Some(message) = info.payload().downcast_ref::<&str>() {
Expand All @@ -240,7 +260,7 @@ impl Iroha {
|location| format!("{}:{}", location.file(), location.line()),
);

iroha_logger::error!(panic_message, location, "A panic occured, shutting down");
iroha_logger::error!(panic_message, location, "A panic occurred, shutting down");

// NOTE: shutdown all currently listening waiters
notify_shutdown.notify_waiters();
Expand Down Expand Up @@ -464,8 +484,12 @@ impl Iroha {

let handle = task::spawn(async move {
tokio::select! {
_ = sigint.recv() => {},
_ = sigterm.recv() => {},
_ = sigint.recv() => {
iroha_logger::info!("SIGINT received, shutting down...");
},
_ = sigterm.recv() => {
iroha_logger::info!("SIGTERM received, shutting down...");
},
}

// NOTE: shutdown all currently listening waiters
Expand Down Expand Up @@ -563,6 +587,7 @@ pub mod style {
}
}

#[cfg(not(feature = "test-network"))]
#[cfg(test)]
mod tests {
use std::{iter::repeat, panic, thread};
Expand Down

0 comments on commit 4ee707a

Please sign in to comment.