From: David Roundy Date: Tue, 1 Dec 2015 22:29:56 +0000 (-0500) Subject: Fix race condition in fs::create_dir_all X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=db00ba9eb2afa1a3db24e208cbd63125d0157090;p=rust.git Fix race condition in fs::create_dir_all It is more robust to not fail if any directory in a path was created concurrently. This change lifts rustc internal `create_dir_racy` that was created to handle such conditions to be new `create_dir_all` implementation. --- diff --git a/src/librustc/util/fs.rs b/src/librustc/util/fs.rs index da6a202e5af..090753b18c0 100644 --- a/src/librustc/util/fs.rs +++ b/src/librustc/util/fs.rs @@ -109,23 +109,3 @@ pub fn rename_or_copy_remove, Q: AsRef>(p: P, } } } - -// Like std::fs::create_dir_all, except handles concurrent calls among multiple -// threads or processes. -pub fn create_dir_racy(path: &Path) -> io::Result<()> { - match fs::create_dir(path) { - Ok(()) => return Ok(()), - Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => return Ok(()), - Err(ref e) if e.kind() == io::ErrorKind::NotFound => (), - Err(e) => return Err(e), - } - match path.parent() { - Some(p) => try!(create_dir_racy(p)), - None => return Err(io::Error::new(io::ErrorKind::Other, "failed to create whole tree")), - } - match fs::create_dir(path) { - Ok(()) => Ok(()), - Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => Ok(()), - Err(e) => Err(e), - } -} diff --git a/src/librustc_incremental/persist/fs.rs b/src/librustc_incremental/persist/fs.rs index 2ad37e98c70..2a4a01cd4a4 100644 --- a/src/librustc_incremental/persist/fs.rs +++ b/src/librustc_incremental/persist/fs.rs @@ -461,7 +461,7 @@ fn generate_session_dir_path(crate_dir: &Path) -> PathBuf { } fn create_dir(sess: &Session, path: &Path, dir_tag: &str) -> Result<(),()> { - match fs_util::create_dir_racy(path) { + match std_fs::create_dir_all(path) { Ok(()) => { debug!("{} directory created successfully", dir_tag); Ok(()) diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index 111c8370be2..1cc73a3dce0 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -885,7 +885,7 @@ pub fn process_crate<'l, 'tcx>(tcx: TyCtxt<'l, 'tcx, 'tcx>, }, }; - if let Err(e) = rustc::util::fs::create_dir_racy(&root_path) { + if let Err(e) = std::fs::create_dir_all(&root_path) { tcx.sess.err(&format!("Could not create directory {}: {}", root_path.display(), e)); diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index b074e8b86b9..bb8b4064fab 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1534,6 +1534,12 @@ pub fn create_dir>(path: P) -> io::Result<()> { /// error conditions for when a directory is being created (after it is /// determined to not exist) are outlined by `fs::create_dir`. /// +/// Notable exception is made for situations where any of the directories +/// specified in the `path` could not be created as it was created concurrently. +/// Such cases are considered success. In other words: calling `create_dir_all` +/// concurrently from multiple threads or processes is guaranteed to not fail +/// due to race itself. +/// /// # Examples /// /// ``` @@ -1769,11 +1775,21 @@ fn _create(&self, path: &Path) -> io::Result<()> { } fn create_dir_all(&self, path: &Path) -> io::Result<()> { - if path == Path::new("") || path.is_dir() { return Ok(()) } - if let Some(p) = path.parent() { - self.create_dir_all(p)? + match self.inner.mkdir(path) { + Ok(()) => return Ok(()), + Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => return Ok(()), + Err(ref e) if e.kind() == io::ErrorKind::NotFound => {} + Err(e) => return Err(e), + } + match path.parent() { + Some(p) => try!(create_dir_all(p)), + None => return Err(io::Error::new(io::ErrorKind::Other, "failed to create whole tree")), + } + match self.inner.mkdir(path) { + Ok(()) => Ok(()), + Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => Ok(()), + Err(e) => Err(e), } - self.inner.mkdir(path) } } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 1ec0838d45f..2865fa6a792 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -25,7 +25,7 @@ use std::collections::HashSet; use std::env; use std::fmt; -use std::fs::{self, File}; +use std::fs::{self, File, create_dir_all}; use std::io::prelude::*; use std::io::{self, BufReader}; use std::path::{Path, PathBuf}; @@ -395,7 +395,7 @@ fn make_typecheck_args(&self) -> ProcArgs { let out_dir = self.output_base_name().with_extension("pretty-out"); let _ = fs::remove_dir_all(&out_dir); - self.create_dir_racy(&out_dir); + create_dir_all(&out_dir).unwrap(); // FIXME (#9639): This needs to handle non-utf8 paths let mut args = vec!["-".to_owned(), @@ -1269,7 +1269,7 @@ fn compute_aux_test_paths(&self, rel_ab: &str) -> TestPaths { fn compose_and_run_compiler(&self, args: ProcArgs, input: Option) -> ProcRes { if !self.props.aux_builds.is_empty() { - self.create_dir_racy(&self.aux_output_dir_name()); + create_dir_all(&self.aux_output_dir_name()).unwrap(); } let aux_dir = self.aux_output_dir_name(); @@ -1340,22 +1340,6 @@ fn compose_and_run_compiler(&self, args: ProcArgs, input: Option) -> Pro input) } - // Like std::fs::create_dir_all, except handles concurrent calls among multiple - // threads or processes. - fn create_dir_racy(&self, path: &Path) { - match fs::create_dir(path) { - Ok(()) => return, - Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => return, - Err(ref e) if e.kind() == io::ErrorKind::NotFound => {} - Err(e) => panic!("failed to create dir {:?}: {}", path, e), - } - self.create_dir_racy(path.parent().unwrap()); - match fs::create_dir(path) { - Ok(()) => {} - Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => {} - Err(e) => panic!("failed to create dir {:?}: {}", path, e), - } - } fn compose_and_run(&self, ProcArgs{ args, prog }: ProcArgs, @@ -1435,7 +1419,7 @@ fn make_compile_args(&self, let mir_dump_dir = self.get_mir_dump_dir(); - self.create_dir_racy(mir_dump_dir.as_path()); + create_dir_all(mir_dump_dir.as_path()).unwrap(); let mut dir_opt = "dump-mir-dir=".to_string(); dir_opt.push_str(mir_dump_dir.to_str().unwrap()); debug!("dir_opt: {:?}", dir_opt); @@ -1923,7 +1907,7 @@ fn run_rustdoc_test(&self) { let out_dir = self.output_base_name(); let _ = fs::remove_dir_all(&out_dir); - self.create_dir_racy(&out_dir); + create_dir_all(&out_dir).unwrap(); let proc_res = self.document(&out_dir); if !proc_res.status.success() { @@ -2299,7 +2283,7 @@ fn run_rmake_test(&self) { if tmpdir.exists() { self.aggressive_rm_rf(&tmpdir).unwrap(); } - self.create_dir_racy(&tmpdir); + create_dir_all(&tmpdir).unwrap(); let host = &self.config.host; let make = if host.contains("bitrig") || host.contains("dragonfly") ||