diff --git a/Cargo.lock b/Cargo.lock index 87d3ac0..e15672d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -48,6 +48,7 @@ dependencies = [ "sha2", "tempfile", "thiserror", + "two-rusty-forks", "uuid", "vfs", "walkdir", @@ -813,6 +814,18 @@ dependencies = [ "winapi", ] +[[package]] +name = "two-rusty-forks" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21ad7c05d9d393a945f0320da5bc22906e58b90f1f5ac0163f92778a4f4dbbe6" +dependencies = [ + "fnv", + "quick-error", + "tempfile", + "wait-timeout", +] + [[package]] name = "typenum" version = "1.12.0" diff --git a/Cargo.toml b/Cargo.toml index 2f9912a..892cdc0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ walkdir = "2.3" [dev-dependencies] proptest = "0.10" +two-rusty-forks = "0.4.0" [dev-dependencies.cargo-husky] version = "1" diff --git a/src/index/lock.rs b/src/index/lock.rs index 8cf0c34..7323aee 100644 --- a/src/index/lock.rs +++ b/src/index/lock.rs @@ -1,8 +1,7 @@ use anyhow::Result; -#[cfg(feature = "failpoints")] use anyhow::*; use fail::fail_point; -use std::io::Write; +use std::{io::Write, time::Instant}; use uuid::Uuid; use vfs::VfsPath; @@ -13,12 +12,18 @@ pub struct Lock { path: VfsPath, } +const MAX_TIMEOUT_MILLIS: u16 = 8192; + impl Lock { pub fn lock(index_directory: &VfsPath) -> Result { + Lock::lock_with_timeout(index_directory, MAX_TIMEOUT_MILLIS) + } + + pub fn lock_with_timeout(index_directory: &VfsPath, max_timeout_millis: u16) -> Result { let mut buffer = [0u8; 16]; OsRng.fill_bytes(&mut buffer); let id = Uuid::from_bytes(buffer); - Lock::wait_to_have_sole_lock(id, index_directory)?; + Lock::wait_to_have_sole_lock(id, index_directory, max_timeout_millis)?; let path = Lock::lock_file_path(index_directory, id)?; Ok(Lock { path }) } @@ -35,14 +40,20 @@ impl Lock { Ok(()) } - fn wait_to_have_sole_lock(lock_id: Uuid, index_directory: &VfsPath) -> Result<()> { + fn wait_to_have_sole_lock(lock_id: Uuid, index_directory: &VfsPath, max_timeout_millis: u16) -> Result<()> { + let start_time = Instant::now(); let _ = Lock::create_lock_file(lock_id, index_directory); while !Lock::sole_lock(lock_id, index_directory)? { let path = Lock::lock_file_path(index_directory, lock_id)?; - path.remove_file()?; - let sleep_duration = time::Duration::from_millis((OsRng.next_u32() % 256).into()); + if path.exists() { + path.remove_file()?; + } + let sleep_duration = time::Duration::from_millis((OsRng.next_u32() % 64).into()); thread::sleep(sleep_duration); let _ = Lock::create_lock_file(lock_id, index_directory); + if start_time.elapsed().as_millis() > max_timeout_millis.into() { + return Err(anyhow!("timed out waiting on lock")); + } } Ok(()) } @@ -51,12 +62,15 @@ impl Lock { let my_lock_file_path = Lock::lock_file_path(index_directory, lock_id)?; let locks = Lock::all_locks(index_directory)?; let mut only_mine = true; - for path in locks { - if path != my_lock_file_path { + for path in &locks { + if path != &my_lock_file_path { only_mine = false; break; } } + if locks.iter().count() == 0 { + return Ok(false); + } Ok(only_mine) } @@ -98,6 +112,8 @@ mod must { use anyhow::*; #[cfg(feature = "failpoints")] use fail::FailScenario; + #[cfg(feature = "failpoints")] + use two_rusty_forks::rusty_fork_test; use vfs::{MemoryFS, VfsPath}; #[test] @@ -112,19 +128,34 @@ mod must { Ok(()) } - #[test] #[cfg(feature = "failpoints")] - fn be_able_to_lock_under_high_contention() -> Result<()> { - let scenario = FailScenario::setup(); - fail::cfg("create-lock-file", "90%10*return(some lock file creation error)->off").map_err(|e| anyhow!(e))?; + rusty_fork_test! { + #[test] + fn be_able_to_lock_when_creating_lock_file_fails_sometimes() { + let scenario = FailScenario::setup(); + fail::cfg("create-lock-file", "90%10*return(some lock file creation error)->off").map_err(|e| anyhow!(e)).unwrap(); - { - let path = MemoryFS::new().into(); - let lock = Lock::lock(&path)?; - lock.release()?; + { + let path = MemoryFS::new().into(); + let lock = Lock::lock(&path).unwrap(); + lock.release().unwrap(); + } + + scenario.teardown(); } + } - scenario.teardown(); - Ok(()) + #[cfg(feature = "failpoints")] + rusty_fork_test! { + #[test] + fn know_to_give_up_when_creating_lock_file_always_fails() { + let scenario = FailScenario::setup(); + fail::cfg("create-lock-file", "return(persistent lock file creation error)").map_err(|e| anyhow!(e)).unwrap(); + + let path = MemoryFS::new().into(); + assert!(Lock::lock_with_timeout(&path, 1).is_err()); + + scenario.teardown(); + } } }