From 32aa933b1da48b0730dd79fbb15d864643391072 Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Wed, 12 Feb 2020 15:37:28 +1100 Subject: CHROMIUM: avl_tool: more gracefully handle termination by SIGINT Since interrupting the test process may be dangerous (leaving the flash in an inconsistent state), we'll catch SIGINT and print a warning the first time, also using it as a signal that we should stop at a convenient time. Any following SIGINT will be handled as normal (killing the process). BUG=b:143251344 TEST=Run tool and verify it exits after a test with a single ^C, exits immediately given two. BRANCH=None Original-Cq-Depend: chromium:2059548 Original-Change-Id: Ib8a7799cba6dbca57dc7f1d3c87521f132c21818 Original-Signed-off-by: Peter Marheine Original-Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+/2050050 Original-Tested-by: Edward O'Callaghan Original-Reviewed-by: Edward O'Callaghan Change-Id: If43aea0580fcc7e698daad2ffe085a3c9da5bc41 Signed-off-by: Edward O'Callaghan Reviewed-on: https://review.coreboot.org/c/flashrom/+/49915 Tested-by: build bot (Jenkins) Reviewed-by: Angel Pons --- util/flashrom_tester/Cargo.toml | 1 + util/flashrom_tester/src/main.rs | 38 ++++++++++++++++++++++++++++++++++++++ util/flashrom_tester/src/tester.rs | 9 +++++++++ util/flashrom_tester/src/tests.rs | 4 +++- 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/util/flashrom_tester/Cargo.toml b/util/flashrom_tester/Cargo.toml index 50f2c4a7d..0898d3c4f 100644 --- a/util/flashrom_tester/Cargo.toml +++ b/util/flashrom_tester/Cargo.toml @@ -19,6 +19,7 @@ chrono = { version = "0.4", optional = true } clap = { version = "2.33", default-features = false, optional = true } flashrom = { path = "flashrom/" } log = { version = "0.4", features = ["std"] } +nix = "0.14.1" rand = "0.6.4" serde_json = "1" sys-info = "0.5.7" diff --git a/util/flashrom_tester/src/main.rs b/util/flashrom_tester/src/main.rs index 1cc525e62..e589ee1e6 100644 --- a/util/flashrom_tester/src/main.rs +++ b/util/flashrom_tester/src/main.rs @@ -42,6 +42,7 @@ use clap::{App, Arg}; use flashrom::FlashChip; use flashrom_tester::{tester, tests}; use std::path::PathBuf; +use std::sync::atomic::AtomicBool; pub mod built_info { include!(concat!(env!("OUT_DIR"), "/built.rs")); @@ -136,8 +137,45 @@ fn main() { print_layout, output_format, test_names, + Some(handle_sigint()), ) { eprintln!("Failed to run tests: {:?}", e); std::process::exit(1); } } + +/// Catch exactly one SIGINT, printing a message in response and setting a flag. +/// +/// The returned value is false by default, becoming true after a SIGINT is +/// trapped. +/// +/// Once a signal is trapped, the default behavior is restored (terminating +/// the process) for future signals. +fn handle_sigint() -> &'static AtomicBool { + use nix::libc::c_int; + use nix::sys::signal::{self, SigHandler, Signal}; + use std::sync::atomic::Ordering; + + unsafe { + let _ = signal::signal(Signal::SIGINT, SigHandler::Handler(sigint_handler)); + } + static TERMINATE_FLAG: AtomicBool = AtomicBool::new(false); + + extern "C" fn sigint_handler(_: c_int) { + const STDERR_FILENO: c_int = 2; + static MESSAGE: &[u8] = b" +WARNING: terminating tests prematurely may leave Flash in an inconsistent state, +rendering your machine unbootable. Testing will end on completion of the current +test, or press ^C again to exit immediately (possibly bricking your machine). +"; + + // Use raw write() because signal-safety is a very hard problem + let _ = nix::unistd::write(STDERR_FILENO, MESSAGE); + unsafe { + let _ = signal::signal(Signal::SIGINT, SigHandler::SigDfl); + } + TERMINATE_FLAG.store(true, Ordering::Release); + } + + &TERMINATE_FLAG +} diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs index fbef2016e..3150a4354 100644 --- a/util/flashrom_tester/src/tester.rs +++ b/util/flashrom_tester/src/tester.rs @@ -39,6 +39,7 @@ use super::utils::{self, LayoutSizes}; use flashrom::{FlashChip, Flashrom, FlashromCmd}; use serde_json::json; use std::mem::MaybeUninit; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Mutex; // type-signature comes from the return type of lib.rs workers. @@ -480,6 +481,7 @@ pub fn run_all_tests( chip: FlashChip, cmd: &FlashromCmd, ts: TS, + terminate_flag: Option<&AtomicBool>, ) -> Vec<(String, (TestConclusion, Option))> where T: TestCase + Copy, @@ -489,6 +491,13 @@ where let mut results = Vec::new(); for t in ts { + if terminate_flag + .map(|b| b.load(Ordering::Acquire)) + .unwrap_or(false) + { + break; + } + let result = decode_test_result(env.run_test(t), t.expected_result()); results.push((t.get_name().into(), result)); } diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index dd756893d..9ef98e5c7 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -40,6 +40,7 @@ use flashrom::{FlashChip, Flashrom, FlashromCmd}; use std::collections::{HashMap, HashSet}; use std::fs::File; use std::io::{BufRead, Write}; +use std::sync::atomic::AtomicBool; const LAYOUT_FILE: &'static str = "/tmp/layout.file"; @@ -82,6 +83,7 @@ pub fn generic<'a, TN: Iterator>( print_layout: bool, output_format: OutputFormat, test_names: Option, + terminate_flag: Option<&AtomicBool>, ) -> Result<(), Box> { let p = path.to_string(); let cmd = FlashromCmd { path: p, fc }; @@ -142,7 +144,7 @@ pub fn generic<'a, TN: Iterator>( // ------------------------. // Run all the tests and collate the findings: - let results = tester::run_all_tests(fc, &cmd, tests); + let results = tester::run_all_tests(fc, &cmd, tests, terminate_flag); // Any leftover filtered names were specified to be run but don't exist for leftover in filter_names.iter().flatten() { -- cgit v1.2.3