From 68cb17097ae92dd50ff076d4b34d955836c4977c Mon Sep 17 00:00:00 2001 From: klutzy Date: Wed, 7 Jan 2015 00:42:26 +0900 Subject: [PATCH] std: prevent `CreateProcess()` race on Windows 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 --- src/libstd/sys/windows/process.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 7b667416b17..8e1f169b5cd 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -25,6 +25,8 @@ 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; @@ -32,6 +34,10 @@ 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 @@ -224,6 +230,7 @@ impl Process { with_dirp(cfg.cwd(), |dirp| { let mut cmd_str: Vec = 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(),