Skip to content

Add std::os::windows::process::CommandExt. Fixes #37827 #38098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 5, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions src/libstd/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1159,4 +1159,61 @@ mod tests {
Ok(_) => panic!(),
}
}

/// Test that process creation flags work by debugging a process.
/// Other creation flags make it hard or impossible to detect
/// behavioral changes in the process.
#[test]
#[cfg(windows)]
fn test_creation_flags() {
use os::windows::process::CommandExt;
use sys::c::{BOOL, DWORD, INFINITE};
#[repr(C, packed)]
struct DEBUG_EVENT {
pub event_code: DWORD,
pub process_id: DWORD,
pub thread_id: DWORD,
// This is a union in the real struct, but we don't
// need this data for the purposes of this test.
pub _junk: [u8; 164],
}

extern "system" {
fn WaitForDebugEvent(lpDebugEvent: *mut DEBUG_EVENT, dwMilliseconds: DWORD) -> BOOL;
fn ContinueDebugEvent(dwProcessId: DWORD, dwThreadId: DWORD, dwContinueStatus: DWORD) -> BOOL;
Copy link
Member

Choose a reason for hiding this comment

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

I think tidy is complaining about this line being over 100 chars

}

const DEBUG_PROCESS: DWORD = 1;
const EXIT_PROCESS_DEBUG_EVENT: DWORD = 5;
const DBG_EXCEPTION_NOT_HANDLED: DWORD = 0x80010001;

let mut child = Command::new("cmd")
.add_creation_flags(DEBUG_PROCESS)
.stdin(Stdio::piped()).spawn().unwrap();
child.stdin.take().unwrap().write_all(b"exit\r\n").unwrap();
let mut events = 0;
let mut event = DEBUG_EVENT {
event_code: 0,
process_id: 0,
thread_id: 0,
_junk: [0; 164],
};
loop {
if unsafe { WaitForDebugEvent(&mut event as *mut DEBUG_EVENT, INFINITE) } == 0 {
panic!("WaitForDebugEvent failed!");
}
events += 1;

if event.event_code == EXIT_PROCESS_DEBUG_EVENT {
break;
}

if unsafe { ContinueDebugEvent(event.process_id,
event.thread_id,
DBG_EXCEPTION_NOT_HANDLED) } == 0 {
panic!("ContinueDebugEvent failed!");
}
}
assert!(events > 0);
}
}
31 changes: 30 additions & 1 deletion src/libstd/sys/windows/ext/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use os::windows::io::{FromRawHandle, RawHandle, AsRawHandle, IntoRawHandle};
use process;
use sys;
use sys_common::{AsInner, FromInner, IntoInner};
use sys_common::{AsInnerMut, AsInner, FromInner, IntoInner};

#[stable(feature = "process_extensions", since = "1.2.0")]
impl FromRawHandle for process::Stdio {
Expand Down Expand Up @@ -97,3 +97,32 @@ impl ExitStatusExt for process::ExitStatus {
process::ExitStatus::from_inner(From::from(raw))
}
}

/// Windows-specific extensions to the `std::process::Command` builder
#[unstable(feature = "windows_process_extensions", issue = "37827")]
pub trait CommandExt {
/// Sets the [process creation flags][1] to be passed to `CreateProcess`.
///
/// These will always be ORed with `CREATE_UNICODE_ENVIRONMENT`.
/// [1]: https://msdn.microsoft.com/en-us/library/windows/desktop/ms684863(v=vs.85).aspx
#[unstable(feature = "windows_process_extensions", issue = "37827")]
fn set_creation_flags(&mut self, flags: u32) -> &mut process::Command;
/// Add `flags` to the the [process creation flags][1] to be passed to `CreateProcess`.
///
/// These will always be ORed with `CREATE_UNICODE_ENVIRONMENT`.
/// [1]: https://msdn.microsoft.com/en-us/library/windows/desktop/ms684863(v=vs.85).aspx
#[unstable(feature = "windows_process_extensions", issue = "37827")]
fn add_creation_flags(&mut self, flags: u32) -> &mut process::Command;
Copy link
Member

@alexcrichton alexcrichton Dec 1, 2016

Choose a reason for hiding this comment

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

In terms of naming and functionality I think we'll want to follow the same pattern as
OpenOptions
for specifying Windows-specific features. In that sense perhaps we can call this creation_flag and avoid the set/add variants and have just the equivalent of set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had originally written it that way, but I worried about flexibility--it'd mean you couldn't attempt to set the creation flags in more than one place, since the last call would override. Perhaps I'm overthinking it? I suppose it's unlikely you'd be able to set creation flags from a library function in a useful manner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed it to creation_flags (with an s) because the parameter in the CreateProcess signature is named dwCreationFlags. (There seem to be a few _flags methods in OpenOptions as well.)

Copy link
Member

Choose a reason for hiding this comment

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

Ah yeah plural sounds good to me, and it's a good point about working with libraries. I'll add this as a concern to the tracking issue

}

#[unstable(feature = "windows_process_extensions", issue = "37827")]
impl CommandExt for process::Command {
fn set_creation_flags(&mut self, flags: u32) -> &mut process::Command {
self.as_inner_mut().set_creation_flags(flags);
self
}
fn add_creation_flags(&mut self, flags: u32) -> &mut process::Command {
self.as_inner_mut().add_creation_flags(flags);
self
}
}
10 changes: 9 additions & 1 deletion src/libstd/sys/windows/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ pub struct Command {
args: Vec<OsString>,
env: Option<HashMap<OsString, OsString>>,
cwd: Option<OsString>,
flags: u32,
detach: bool, // not currently exposed in std::process
stdin: Option<Stdio>,
stdout: Option<Stdio>,
Expand Down Expand Up @@ -84,6 +85,7 @@ impl Command {
args: Vec::new(),
env: None,
cwd: None,
flags: 0,
detach: false,
stdin: None,
stdout: None,
Expand Down Expand Up @@ -124,6 +126,12 @@ impl Command {
pub fn stderr(&mut self, stderr: Stdio) {
self.stderr = Some(stderr);
}
pub fn set_creation_flags(&mut self, flags: u32) {
self.flags = flags;
}
pub fn add_creation_flags(&mut self, flags: u32) {
self.flags = self.flags | flags;
}

pub fn spawn(&mut self, default: Stdio, needs_stdin: bool)
-> io::Result<(Process, StdioPipes)> {
Expand Down Expand Up @@ -157,7 +165,7 @@ impl Command {
cmd_str.push(0); // add null terminator

// stolen from the libuv code.
let mut flags = c::CREATE_UNICODE_ENVIRONMENT;
let mut flags = self.flags | c::CREATE_UNICODE_ENVIRONMENT;
if self.detach {
flags |= c::DETACHED_PROCESS | c::CREATE_NEW_PROCESS_GROUP;
}
Expand Down