From 782e06e0e379768d03e35acae16e7ee1e1841633 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Tue, 30 Apr 2013 17:58:24 -0700 Subject: [PATCH] core/std: Fix race condition in os::mkdir_recursive tests Added a change_dir_locked function to os, and use it in the mkdir_recursive tests so that the tests don't clobber each other's directory changes. --- src/libcore/os.rs | 30 +++++++++++++++++++++++++ src/libstd/tempfile.rs | 50 ++++++++++++++++++++++++------------------ 2 files changed, 59 insertions(+), 21 deletions(-) diff --git a/src/libcore/os.rs b/src/libcore/os.rs index f1962eeaa23..0455dabb7f0 100644 --- a/src/libcore/os.rs +++ b/src/libcore/os.rs @@ -818,6 +818,36 @@ fn chdir(p: &Path) -> bool { } } +/// Changes the current working directory to the specified +/// path while acquiring a global lock, then calls `action`. +/// If the change is successful, releases the lock and restores the +/// CWD to what it was before, returning true. +/// Returns false if the directory doesn't exist or if the directory change +/// is otherwise unsuccessful. +pub fn change_dir_locked(p: &Path, action: &fn()) -> bool { + use unstable::global::global_data_clone_create; + use unstable::{Exclusive, exclusive}; + + fn key(_: Exclusive<()>) { } + + let result = unsafe { + global_data_clone_create(key, || { + ~exclusive(()) + }) + }; + + do result.with_imm() |_| { + let old_dir = os::getcwd(); + if change_dir(p) { + action(); + change_dir(&old_dir) + } + else { + false + } + } +} + /// Copies a file from one location to another pub fn copy_file(from: &Path, to: &Path) -> bool { return do_copy_file(from, to); diff --git a/src/libstd/tempfile.rs b/src/libstd/tempfile.rs index eec91b68454..6da74834b1a 100644 --- a/src/libstd/tempfile.rs +++ b/src/libstd/tempfile.rs @@ -42,13 +42,18 @@ fn recursive_mkdir_rel() { use core::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR}; use core::os; - let root = mkdtemp(&os::tmpdir(), "temp").expect("recursive_mkdir_rel"); - os::change_dir(&root); - let path = Path("frob"); - assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); - assert!(os::path_is_dir(&path)); - assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); - assert!(os::path_is_dir(&path)); + let root = mkdtemp(&os::tmpdir(), "recursive_mkdir_rel"). + expect("recursive_mkdir_rel"); + assert!(do os::change_dir_locked(&root) { + let path = Path("frob"); + debug!("recursive_mkdir_rel: Making: %s in cwd %s [%?]", path.to_str(), + os::getcwd().to_str(), + os::path_exists(&path)); + assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + assert!(os::path_is_dir(&path)); + assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + assert!(os::path_is_dir(&path)); + }); } #[test] @@ -67,18 +72,21 @@ fn recursive_mkdir_rel_2() { use core::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR}; use core::os; - let root = mkdtemp(&os::tmpdir(), "temp").expect("recursive_mkdir_rel_2"); - os::change_dir(&root); - let path = Path("./frob/baz"); - debug!("...Making: %s in cwd %s", path.to_str(), os::getcwd().to_str()); - assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); - assert!(os::path_is_dir(&path)); - assert!(os::path_is_dir(&path.pop())); - let path2 = Path("quux/blat"); - debug!("Making: %s in cwd %s", path2.to_str(), os::getcwd().to_str()); - assert!(os::mkdir_recursive(&path2, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); - assert!(os::path_is_dir(&path2)); - assert!(os::path_is_dir(&path2.pop())); + let root = mkdtemp(&os::tmpdir(), "recursive_mkdir_rel_2"). + expect("recursive_mkdir_rel_2"); + assert!(do os::change_dir_locked(&root) { + let path = Path("./frob/baz"); + debug!("recursive_mkdir_rel_2: Making: %s in cwd %s [%?]", path.to_str(), + os::getcwd().to_str(), os::path_exists(&path)); + assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + assert!(os::path_is_dir(&path)); + assert!(os::path_is_dir(&path.pop())); + let path2 = Path("quux/blat"); + debug!("recursive_mkdir_rel_2: Making: %s in cwd %s", path2.to_str(), + os::getcwd().to_str()); + assert!(os::mkdir_recursive(&path2, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + assert!(os::path_is_dir(&path2)); + assert!(os::path_is_dir(&path2.pop())); + }); } - -} \ No newline at end of file +}