Fix race between read/write
wait_for_file waits for "creation". The test is flaky due to the race
between read/write.
Added a WARNING note to the function and fixed the test to avoid the
race.
Verified that the current usages are okay since they wait for files
which are made visible by mounting or similar, not writing from other
processes/threads.
Bug: 292297891
Test: atest microdroid_manager_test
Change-Id: I05e99aa433fe7664c2d74ca1f4f34bc20bfd1939
diff --git a/microdroid_manager/src/ioutil.rs b/microdroid_manager/src/ioutil.rs
index d36e349..772941d 100644
--- a/microdroid_manager/src/ioutil.rs
+++ b/microdroid_manager/src/ioutil.rs
@@ -28,6 +28,10 @@
const SLEEP_DURATION: Duration = Duration::from_millis(5);
/// waits for a file with a timeout and returns it
+///
+/// WARNING: This only guarantees file creation. When there's another thread
+/// writing a file and you're waiting for the file, reading the file should be
+/// synchronized with other mechanism than just waiting for the creation.
pub fn wait_for_file<P: AsRef<Path> + Debug>(path: P, timeout: Duration) -> Result<File> {
debug!("waiting for {:?}...", path);
let begin = Instant::now();
@@ -64,15 +68,21 @@
#[cfg(test)]
mod tests {
use super::*;
+ use std::fs::rename;
use std::io::{Read, Write};
#[test]
fn test_wait_for_file() -> Result<()> {
let test_dir = tempfile::TempDir::new().unwrap();
let test_file = test_dir.path().join("test.txt");
+ let temp_file = test_dir.path().join("test.txt~");
thread::spawn(move || -> io::Result<()> {
+ // Sleep to ensure that `wait_for_file` actually waits
thread::sleep(Duration::from_secs(1));
- File::create(test_file)?.write_all(b"test")
+ // Write to a temp file and then rename it to avoid the race between
+ // write and read.
+ File::create(&temp_file)?.write_all(b"test")?;
+ rename(temp_file, test_file)
});
let test_file = test_dir.path().join("test.txt");