Skip to content

Commit

Permalink
Fix races in the get_cached_or_compile tests.
Browse files Browse the repository at this point in the history 
As part of 030ed02 I made `DiskCache::finish_put` write the cache entry to disk asynchronously, where previously it had been synchronous. This caused the `get_cached_or_compile` tests to be racy, specifically `test_compiler_get_cached_or_compile_cached`, which would store a cache entry and then immediately try to retrieve it. I've fixed this by making all the tests wait on the cache write future.
  • Loading branch information
@luser
luser committed Dec 13, 2016
1 parent aa5e965 commit 6d7efe9
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 20 deletions.
2 changes: 1 addition & 1 deletion lru-disk-cache/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fn get_all_files<P: AsRef<Path>>(path: P) -> Box<Iterator<Item=(PathBuf, u64)>>
.collect();
// Sort by last-modified-time, so oldest file first.
files.sort_by_key(|k| k.0);
Box::new(files.into_iter().map(|(mtime, path, size)| { println!("Found file {:?}, mtime {:?}, size {}", path, mtime, size); (path, size) }))
Box::new(files.into_iter().map(|(_mtime, path, size)| (path, size)))
}

/// An LRU cache of files on disk.
Expand Down
52 changes: 33 additions & 19 deletions src/compiler/compiler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ pub fn run_input_output<C: RunCommand>(mut command: C, input: Option<Vec<u8>>) -
mod test {
use super::*;
use cache::disk::DiskCache;
use futures::Future;
use mock_command::*;
use std::fs::{self,File};
use std::io::Write;
Expand Down Expand Up @@ -579,7 +580,7 @@ mod test {
}
let creator = new_creator();
let f = TestFixture::new();
let storage = DiskCache::new(&f.tempdir.path(), usize::MAX);
let storage = DiskCache::new(&f.tempdir.path().join("cache"), usize::MAX);
// Pretend to be GCC.
next_command(&creator, Ok(MockChild::new(exit_status(0), "gcc", "")));
let c = get_compiler_info(creator.clone(),
Expand Down Expand Up @@ -608,10 +609,13 @@ mod test {
let (cached, res) = c.get_cached_or_compile(creator.clone(), &storage, &arguments, &parsed_args, cwd, CacheControl::Default).unwrap();
// Ensure that the object file was created.
assert_eq!(true, fs::metadata(&obj).and_then(|m| Ok(m.len() > 0)).unwrap());
assert!(match cached {
CompileResult::CacheMiss(MissType::Normal, _) => true,
_ => false,
});
match cached {
CompileResult::CacheMiss(MissType::Normal, f) => {
// wait on cache write future so we don't race with it!
f.wait().unwrap().unwrap();
}
_ => assert!(false, "Unexpected compile result: {:?}", cached),
}
assert_eq!(exit_status(0), res.status);
assert_eq!(COMPILER_STDOUT, res.stdout.as_slice());
assert_eq!(COMPILER_STDERR, res.stderr.as_slice());
Expand Down Expand Up @@ -639,7 +643,7 @@ mod test {
}
let creator = new_creator();
let f = TestFixture::new();
let storage = DiskCache::new(&f.tempdir.path(), usize::MAX);
let storage = DiskCache::new(&f.tempdir.path().join("cache"), usize::MAX);
// Pretend to be GCC.
next_command(&creator, Ok(MockChild::new(exit_status(0), "gcc", "")));
let c = get_compiler_info(creator.clone(),
Expand Down Expand Up @@ -668,10 +672,14 @@ mod test {
let (cached, res) = c.get_cached_or_compile(creator.clone(), &storage, &arguments, &parsed_args, cwd, CacheControl::Default).unwrap();
// Ensure that the object file was created.
assert_eq!(true, fs::metadata(&obj).and_then(|m| Ok(m.len() > 0)).unwrap());
assert!(match cached {
CompileResult::CacheMiss(MissType::Normal, _) => true,
_ => false,
});
match cached {
CompileResult::CacheMiss(MissType::Normal, f) => {
// wait on cache write future so we don't race with it!
f.wait().unwrap().unwrap();
}
_ => assert!(false, "Unexpected compile result: {:?}", cached),
}

assert_eq!(exit_status(0), res.status);
assert_eq!(COMPILER_STDOUT, res.stdout.as_slice());
assert_eq!(COMPILER_STDERR, res.stderr.as_slice());
Expand All @@ -698,7 +706,7 @@ mod test {
}
let creator = new_creator();
let f = TestFixture::new();
let storage = DiskCache::new(&f.tempdir.path(), usize::MAX);
let storage = DiskCache::new(&f.tempdir.path().join("cache"), usize::MAX);
// Pretend to be GCC.
next_command(&creator, Ok(MockChild::new(exit_status(0), "gcc", "")));
let c = get_compiler_info(creator.clone(),
Expand Down Expand Up @@ -731,10 +739,13 @@ mod test {
let (cached, res) = c.get_cached_or_compile(creator.clone(), &storage, &arguments, &parsed_args, cwd, CacheControl::Default).unwrap();
// Ensure that the object file was created.
assert_eq!(true, fs::metadata(&obj).and_then(|m| Ok(m.len() > 0)).unwrap());
assert!(match cached {
CompileResult::CacheMiss(MissType::Normal, _) => true,
_ => false,
});
match cached {
CompileResult::CacheMiss(MissType::Normal, f) => {
// wait on cache write future so we don't race with it!
f.wait().unwrap().unwrap();
}
_ => assert!(false, "Unexpected compile result: {:?}", cached),
}
assert_eq!(exit_status(0), res.status);
assert_eq!(COMPILER_STDOUT, res.stdout.as_slice());
assert_eq!(COMPILER_STDERR, res.stderr.as_slice());
Expand All @@ -743,10 +754,13 @@ mod test {
let (cached, res) = c.get_cached_or_compile(creator.clone(), &storage, &arguments, &parsed_args, cwd, CacheControl::ForceRecache).unwrap();
// Ensure that the object file was created.
assert_eq!(true, fs::metadata(&obj).and_then(|m| Ok(m.len() > 0)).unwrap());
assert!(match cached {
CompileResult::CacheMiss(MissType::ForcedRecache, _) => true,
_ => false,
});
match cached {
CompileResult::CacheMiss(MissType::ForcedRecache, f) => {
// wait on cache write future so we don't race with it!
f.wait().unwrap().unwrap();
}
_ => assert!(false, "Unexpected compile result: {:?}", cached),
}
assert_eq!(exit_status(0), res.status);
assert_eq!(COMPILER_STDOUT, res.stdout.as_slice());
assert_eq!(COMPILER_STDERR, res.stderr.as_slice());
Expand Down

0 comments on commit 6d7efe9

Please sign in to comment.