From dda335c00ff1ba397745fd050cdadb56bebb89d8 Mon Sep 17 00:00:00 2001 From: Hoverbear Date: Thu, 6 Sep 2018 15:20:27 -0700 Subject: [PATCH 1/4] Refine CI script to ensure correctness. --- .travis.yml | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/.travis.yml b/.travis.yml index 603583c..8b68b72 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,18 +1,29 @@ -dist: trusty -sudo: false language: rust -os: - - linux - - osx +sudo: false +env: + global: + - RUST_BACKTRACE=1 + - RUSTFLAGS="-D warnings" +cache: cargo + rust: - - stable - - beta - - nightly + matrix: - allow_failures: - - rust: nightly + include: + # This build uses stable and checks rustfmt (clippy is not on stable). + - rust: stable + install: + - rustup component add rustfmt-preview + before_script: + - cargo fmt --all -- --check + - rust: nightly + install: + - rustup component add clippy-preview --toolchain nightly + before_script: + - cargo clippy --all -- -D clippy::all + + script: - - if [[ $TRAVIS_RUST_VERSION == "nightly" ]]; then rustup component add clippy-preview && cargo clippy; fi - - export RUSTFLAGS=-Dwarnings - - cargo build - - cargo test --all + - cargo test --all -- --nocapture + # Validate benches still work. + - cargo bench --all -- --test From 256a902a380f4b88f354c1823a4f6499d914346b Mon Sep 17 00:00:00 2001 From: Hoverbear Date: Thu, 6 Sep 2018 15:30:04 -0700 Subject: [PATCH 2/4] fmt --- src/lib.rs | 10 +++++----- tests/tests.rs | 13 +++++-------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e0f814f..0ebe01b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -66,8 +66,8 @@ use std::env::VarError; use std::str::FromStr; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Condvar, Mutex, RwLock, TryLockError}; -use std::{env, thread}; use std::time::{Duration, Instant}; +use std::{env, thread}; use rand::Closed01; @@ -358,9 +358,7 @@ pub fn list() -> Vec<(String, String)> { let registry = REGISTRY.registry.read().unwrap(); registry .iter() - .map(|(name, fp)| { - (name.to_string(), fp.actions_str.read().unwrap().clone()) - }) + .map(|(name, fp)| (name.to_string(), fp.actions_str.read().unwrap().clone())) .collect() } @@ -685,7 +683,9 @@ mod tests { assert_eq!(f1(), 1); let (tx, rx) = mpsc::channel(); - thread::spawn(move || { tx.send(f2()).unwrap(); }); + thread::spawn(move || { + tx.send(f2()).unwrap(); + }); assert!(rx.recv_timeout(Duration::from_millis(500)).is_err()); teardown(); diff --git a/tests/tests.rs b/tests/tests.rs index e771400..e61f4d7 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -36,9 +36,8 @@ fn test_off() { #[test] fn test_return() { let f = || { - fail_point!("return", |s: Option| { - s.map_or(2, |s| s.parse().unwrap()) - }); + fail_point!("return", |s: Option| s + .map_or(2, |s| s.parse().unwrap())); 0 }; assert_eq!(f(), 0); @@ -159,9 +158,8 @@ fn test_delay() { #[test] fn test_freq_and_count() { let f = || { - fail_point!("freq_and_count", |s: Option| { - s.map_or(2, |s| s.parse().unwrap()) - }); + fail_point!("freq_and_count", |s: Option| s + .map_or(2, |s| s.parse().unwrap())); 0 }; fail::cfg( @@ -192,8 +190,7 @@ fn test_condition() { #[test] fn test_list() { - assert!(!fail::list() - .contains(&("list".to_string(), "off".to_string()))); + assert!(!fail::list().contains(&("list".to_string(), "off".to_string()))); fail::cfg("list", "off").unwrap(); assert!(fail::list().contains(&("list".to_string(), "off".to_string()))); fail::cfg("list", "return").unwrap(); From c55f4220b4e4bc762b086f9429e574b6c2cebc13 Mon Sep 17 00:00:00 2001 From: Hoverbear Date: Thu, 6 Sep 2018 16:29:32 -0700 Subject: [PATCH 3/4] fix clippy lints --- src/lib.rs | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 0ebe01b..556cb64 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,6 +54,7 @@ //! //! If you want to disable all the fail points at compile time, you can enable features `no_fail`. #![deny(missing_docs, missing_debug_implementations)] +#![cfg_attr(feature = "cargo-clippy", feature(tool_lints))] #[macro_use] extern crate lazy_static; @@ -64,8 +65,8 @@ extern crate rand; use std::collections::HashMap; use std::env::VarError; use std::str::FromStr; -use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::{Arc, Condvar, Mutex, RwLock, TryLockError}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::{Arc, RwLock, TryLockError}; use std::time::{Duration, Instant}; use std::{env, thread}; @@ -118,8 +119,8 @@ impl PartialEq for Action { impl Action { fn new(task: Task, freq: f32, max_cnt: Option) -> Action { Action { - task: task, - freq: freq, + task, + freq, count: max_cnt.map(AtomicUsize::new), } } @@ -171,7 +172,7 @@ impl FromStr for Action { let (first, second) = partition(remain, '('); if let Some(second) = second { remain = first; - if !second.ends_with(")") { + if !second.ends_with(')') { return Err("parentheses not match".to_owned()); } args = Some(&second[..second.len() - 1]); @@ -198,9 +199,9 @@ impl FromStr for Action { } let parse_timeout = || match args { - None => return Err("sleep require timeout".to_owned()), + None => Err("sleep require timeout".to_owned()), Some(timeout_str) => match timeout_str.parse() { - Err(e) => return Err(format!("failed to parse timeout: {}", e)), + Err(e) => Err(format!("failed to parse timeout: {}", e)), Ok(timeout) => Ok(timeout), }, }; @@ -222,8 +223,7 @@ impl FromStr for Action { } struct FailPoint { - pause: Mutex, - pause_notifier: Condvar, + pause: AtomicBool, actions: RwLock>, actions_str: RwLock, } @@ -231,8 +231,7 @@ struct FailPoint { impl FailPoint { fn new() -> FailPoint { FailPoint { - pause: Mutex::new(false), - pause_notifier: Condvar::new(), + pause: AtomicBool::new(false), actions: RwLock::default(), actions_str: RwLock::default(), } @@ -251,22 +250,19 @@ impl FailPoint { Err(e) => panic!("unexpected poison: {:?}", e), } - let mut guard = self.pause.lock().unwrap(); - *guard = false; - self.pause_notifier.notify_all(); + self.pause.store(false, Ordering::Release); } } + #[cfg_attr(feature = "cargo-clippy", allow(clippy::option_option))] fn eval(&self, name: &str) -> Option> { let task = { let actions = self.actions.read().unwrap(); match actions.iter().filter_map(|a| a.get_task()).next() { Some(Task::Pause) => { - let mut guard = self.pause.lock().unwrap(); - *guard = true; + self.pause.store(true, Ordering::Release); loop { - guard = self.pause_notifier.wait(guard).unwrap(); - if !*guard { + if !self.pause.load(Ordering::Acquire) { break; } } @@ -326,7 +322,7 @@ pub fn setup() { Err(VarError::NotPresent) => return, Err(e) => panic!("invalid failpoints: {:?}", e), }; - for mut cfg in failpoints.trim().split(";") { + for mut cfg in failpoints.trim().split(';') { cfg = cfg.trim(); if cfg.trim().is_empty() { continue; @@ -344,7 +340,7 @@ pub fn setup() { /// All the paused fail points will be notified before they are deactivated. pub fn teardown() { let mut registry = REGISTRY.registry.write().unwrap(); - for (_, p) in &*registry { + for p in registry.values() { // wake up all pause failpoint. p.set_actions("", vec![]); } From 7b446a43125dbdc9f42fe718bb4fd5f81dc533ec Mon Sep 17 00:00:00 2001 From: Hoverbear Date: Fri, 14 Sep 2018 13:41:05 -0700 Subject: [PATCH 4/4] Revert atomic bool change --- src/lib.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 556cb64..5d0bf12 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -65,8 +65,8 @@ extern crate rand; use std::collections::HashMap; use std::env::VarError; use std::str::FromStr; -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; -use std::sync::{Arc, RwLock, TryLockError}; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, Condvar, Mutex, RwLock, TryLockError}; use std::time::{Duration, Instant}; use std::{env, thread}; @@ -223,15 +223,18 @@ impl FromStr for Action { } struct FailPoint { - pause: AtomicBool, + pause: Mutex, + pause_notifier: Condvar, actions: RwLock>, actions_str: RwLock, } +#[cfg_attr(feature = "cargo-clippy", allow(clippy::mutex_atomic))] impl FailPoint { fn new() -> FailPoint { FailPoint { - pause: AtomicBool::new(false), + pause: Mutex::new(false), + pause_notifier: Condvar::new(), actions: RwLock::default(), actions_str: RwLock::default(), } @@ -249,8 +252,9 @@ impl FailPoint { } Err(e) => panic!("unexpected poison: {:?}", e), } - - self.pause.store(false, Ordering::Release); + let mut guard = self.pause.lock().unwrap(); + *guard = false; + self.pause_notifier.notify_all(); } } @@ -260,9 +264,11 @@ impl FailPoint { let actions = self.actions.read().unwrap(); match actions.iter().filter_map(|a| a.get_task()).next() { Some(Task::Pause) => { - self.pause.store(true, Ordering::Release); + let mut guard = self.pause.lock().unwrap(); + *guard = true; loop { - if !self.pause.load(Ordering::Acquire) { + guard = self.pause_notifier.wait(guard).unwrap(); + if !*guard { break; } }