Skip to content

Commit

Permalink
Fix a race condition between remove_from_env and other io::process te…
Browse files Browse the repository at this point in the history
…sts.
  • Loading branch information
eddyb committed Oct 3, 2014
1 parent 9a2286d commit ef69388
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 26 deletions.
26 changes: 0 additions & 26 deletions src/libstd/io/process.rs
Expand Up @@ -1024,32 +1024,6 @@ mod tests {
"didn't find RUN_TEST_NEW_ENV inside of:\n\n{}", output);
}

#[test]
fn test_remove_from_env() {
use os;

// save original environment
let old_env = os::getenv("RUN_TEST_NEW_ENV");

os::setenv("RUN_TEST_NEW_ENV", "123");
let prog = env_cmd().env_remove("RUN_TEST_NEW_ENV").spawn().unwrap();
let result = prog.wait_with_output().unwrap();
let output = str::from_utf8_lossy(result.output.as_slice()).into_string();

// restore original environment
match old_env {
None => {
os::unsetenv("RUN_TEST_NEW_ENV");
}
Some(val) => {
os::setenv("RUN_TEST_NEW_ENV", val.as_slice());
}
}

assert!(!output.as_slice().contains("RUN_TEST_NEW_ENV"),
"found RUN_TEST_NEW_ENV inside of:\n\n{}", output);
}

#[cfg(unix)]
pub fn sleeper() -> Process {
Command::new("sleep").arg("1000").spawn().unwrap()
Expand Down
53 changes: 53 additions & 0 deletions src/test/run-pass/process-remove-from-env.rs
@@ -0,0 +1,53 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::io::Command;
use std::os;

#[cfg(all(unix, not(target_os="android")))]
pub fn env_cmd() -> Command {
Command::new("env")
}
#[cfg(target_os="android")]
pub fn env_cmd() -> Command {
let mut cmd = Command::new("/system/bin/sh");
cmd.arg("-c").arg("set");
cmd
}

#[cfg(windows)]
pub fn env_cmd() -> Command {
let mut cmd = Command::new("cmd");
cmd.arg("/c").arg("set");
cmd
}

fn main() {
// save original environment
let old_env = os::getenv("RUN_TEST_NEW_ENV");

os::setenv("RUN_TEST_NEW_ENV", "123");

let mut cmd = env_cmd();
cmd.env_remove("RUN_TEST_NEW_ENV");

// restore original environment
match old_env {
None => os::unsetenv("RUN_TEST_NEW_ENV"),
Some(val) => os::setenv("RUN_TEST_NEW_ENV", val.as_slice())
}

let prog = cmd.spawn().unwrap();
let result = prog.wait_with_output().unwrap();
let output = String::from_utf8_lossy(result.output.as_slice());

assert!(!output.as_slice().contains("RUN_TEST_NEW_ENV"),
"found RUN_TEST_NEW_ENV inside of:\n\n{}", output);
}

9 comments on commit ef69388

@alexcrichton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+ p=10

@bors
Copy link
Contributor

@bors bors commented on ef69388 Oct 3, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from alexcrichton
at eddyb@ef69388

@bors
Copy link
Contributor

@bors bors commented on ef69388 Oct 3, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging eddyb/rust/fix-process-test = ef69388 into auto

@bors
Copy link
Contributor

@bors bors commented on ef69388 Oct 3, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eddyb/rust/fix-process-test = ef69388 merged ok, testing candidate = 449191fe

@bors
Copy link
Contributor

@bors bors commented on ef69388 Oct 3, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from alexcrichton
at eddyb@ef69388

@bors
Copy link
Contributor

@bors bors commented on ef69388 Oct 3, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging eddyb/rust/fix-process-test = ef69388 into auto

@bors
Copy link
Contributor

@bors bors commented on ef69388 Oct 3, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eddyb/rust/fix-process-test = ef69388 merged ok, testing candidate = 8c16885c

@bors
Copy link
Contributor

@bors bors commented on ef69388 Oct 3, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.