Skip to content

Commit

Permalink
std: prevent CreateProcess() race on Windows
Browse files Browse the repository at this point in the history
Believe or not, `CreateProcess()` is racy if several threads create
child processes: [0], [1], [2].

This caused some tests show crash dialogs during
`make check-stage#-rpass`.

More explanation:

On Windows, `SetErrorMode()` controls display of error dialogs: it
accepts new error mode and returns old error mode.
The error mode is process-global and automatically inherited to child
process when created.

MSYS2 bash shell internally sets it to not show error dialogs, therefore
`make check-stage#-rpass` should not show them either.

However, [1] says that `CreateProcess()` internally invokes
`SetErrorMode()` twice: at first it sets mode `0x8001` and saves
original mode, and at second it restores original mode.
So if two threads simultaneously call `CreateProcess()`, the first
thread sets error mode to `0x8001` then the second thread recognizes
that current error mode is `0x8001`. Therefore, The second thread will
create process with wrong error mode.

This really occurs inside `compiletest`: it creates several processes on
each thread, so some `run-pass` tests are invoked with wrong error mode
therefore show crash dialog.

This commit adds `StaticMutex` for `CreateProcess()` call. This seems
to fix the "dialog annoyance" issue.

[0]: http://support.microsoft.com/kb/315939
[1]: https://code.google.com/p/nativeclient/issues/detail?id=2968
[2]: https://ghc.haskell.org/trac/ghc/ticket/2650
  • Loading branch information
klutzy committed Jan 6, 2015
1 parent 8efd990 commit 68cb170
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/libstd/sys/windows/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,19 @@ use path::BytesContainer;
use ptr;
use str;
use sys::fs::FileDesc;
use sync::{StaticMutex, MUTEX_INIT};

use sys::fs;
use sys::{self, retry, c, wouldblock, set_nonblocking, ms_to_timeval, timer};
use sys_common::helper_thread::Helper;
use sys_common::{AsInner, mkerr_libc, timeout};

pub use sys_common::ProcessConfig;

// `CreateProcess` is racy!
// http://support.microsoft.com/kb/315939
static CREATE_PROCESS_LOCK: StaticMutex = MUTEX_INIT;

/// A value representing a child process.
///
/// The lifetime of this value is linked to the lifetime of the actual
Expand Down Expand Up @@ -224,6 +230,7 @@ impl Process {
with_dirp(cfg.cwd(), |dirp| {
let mut cmd_str: Vec<u16> = cmd_str.utf16_units().collect();
cmd_str.push(0);
let _lock = CREATE_PROCESS_LOCK.lock().unwrap();
let created = CreateProcessW(ptr::null(),
cmd_str.as_mut_ptr(),
ptr::null_mut(),
Expand Down

1 comment on commit 68cb170

@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+

Please sign in to comment.