diff --git a/src/libcore/libc.rs b/src/libcore/libc.rs index 945d08323b4..44864630f98 100644 --- a/src/libcore/libc.rs +++ b/src/libcore/libc.rs @@ -1725,6 +1725,7 @@ pub mod funcs { findFileData: HANDLE) -> BOOL; unsafe fn FindClose(findFile: HANDLE) -> BOOL; + unsafe fn CloseHandle(hObject: HANDLE) -> BOOL; unsafe fn TerminateProcess(hProcess: HANDLE, uExitCode: c_uint) -> BOOL; } } diff --git a/src/libcore/os.rs b/src/libcore/os.rs index 0e7131a86f3..0b73eb9dca3 100644 --- a/src/libcore/os.rs +++ b/src/libcore/os.rs @@ -60,7 +60,7 @@ pub mod rustrt { unsafe fn rust_get_argv() -> **c_char; unsafe fn rust_path_is_dir(path: *libc::c_char) -> c_int; unsafe fn rust_path_exists(path: *libc::c_char) -> c_int; - unsafe fn rust_process_wait(handle: c_int) -> c_int; + unsafe fn rust_process_wait(pid: c_int) -> c_int; unsafe fn rust_set_exit_status(code: libc::intptr_t); } } @@ -356,7 +356,11 @@ pub fn fsync_fd(fd: c_int, _l: io::fsync::Level) -> c_int { #[cfg(windows)] pub fn waitpid(pid: pid_t) -> c_int { unsafe { - return rustrt::rust_process_wait(pid); + let status = rustrt::rust_process_wait(pid); + if status < 0 { + fail!(fmt!("failure in rust_process_wait: %s", last_os_error())); + } + return status; } } diff --git a/src/libcore/run.rs b/src/libcore/run.rs index f9d7f4a229c..4c5da132532 100644 --- a/src/libcore/run.rs +++ b/src/libcore/run.rs @@ -24,8 +24,9 @@ use task; use vec; pub mod rustrt { - use libc::{c_int, c_void, pid_t}; + use libc::{c_int, c_void}; use libc; + use run; #[abi = "cdecl"] pub extern { @@ -34,11 +35,18 @@ pub mod rustrt { dir: *libc::c_char, in_fd: c_int, out_fd: c_int, - err_fd: c_int) - -> pid_t; + err_fd: c_int) -> run::RunProgramResult; } } +pub struct RunProgramResult { + // the process id of the program, or -1 if in case of errors + pid: pid_t, + // a handle to the process - on unix this will always be NULL, but on windows it will be a + // HANDLE to the process, which will prevent the pid being re-used until the handle is closed. + handle: *(), +} + /// A value representing a child process pub trait Program { /// Returns the process id of the program @@ -100,16 +108,24 @@ pub trait Program { * The process id of the spawned process */ pub fn spawn_process(prog: &str, args: &[~str], - env: &Option<~[(~str,~str)]>, - dir: &Option<~str>, - in_fd: c_int, out_fd: c_int, err_fd: c_int) - -> pid_t { + env: &Option<~[(~str,~str)]>, + dir: &Option<~str>, + in_fd: c_int, out_fd: c_int, err_fd: c_int) -> pid_t { + + let res = spawn_process_internal(prog, args, env, dir, in_fd, out_fd, err_fd); + free_handle(res.handle); + return res.pid; +} + +fn spawn_process_internal(prog: &str, args: &[~str], + env: &Option<~[(~str,~str)]>, + dir: &Option<~str>, + in_fd: c_int, out_fd: c_int, err_fd: c_int) -> RunProgramResult { unsafe { do with_argv(prog, args) |argv| { do with_envp(env) |envp| { do with_dirp(dir) |dirp| { - rustrt::rust_run_program(argv, envp, dirp, - in_fd, out_fd, err_fd) + rustrt::rust_run_program(argv, envp, dirp, in_fd, out_fd, err_fd) } } } @@ -195,6 +211,18 @@ priv unsafe fn fclose_and_null(f: &mut *libc::FILE) { } } +#[cfg(windows)] +priv fn free_handle(handle: *()) { + unsafe { + libc::funcs::extra::kernel32::CloseHandle(cast::transmute(handle)); + } +} + +#[cfg(unix)] +priv fn free_handle(_handle: *()) { + // unix has no process handle object, just a pid +} + /** * Spawns a process and waits for it to terminate * @@ -208,10 +236,13 @@ priv unsafe fn fclose_and_null(f: &mut *libc::FILE) { * The process's exit code */ pub fn run_program(prog: &str, args: &[~str]) -> int { - let pid = spawn_process(prog, args, &None, &None, - 0i32, 0i32, 0i32); - if pid == -1 as pid_t { fail!(); } - return waitpid(pid); + let res = spawn_process_internal(prog, args, &None, &None, + 0i32, 0i32, 0i32); + if res.pid == -1 as pid_t { fail!(); } + + let code = waitpid(res.pid); + free_handle(res.handle); + return code; } /** @@ -234,13 +265,13 @@ pub fn start_program(prog: &str, args: &[~str]) -> @Program { let pipe_input = os::pipe(); let pipe_output = os::pipe(); let pipe_err = os::pipe(); - let pid = - spawn_process(prog, args, &None, &None, - pipe_input.in, pipe_output.out, - pipe_err.out); + let res = + spawn_process_internal(prog, args, &None, &None, + pipe_input.in, pipe_output.out, + pipe_err.out); unsafe { - if pid == -1 as pid_t { fail!(); } + if res.pid == -1 as pid_t { fail!(); } libc::close(pipe_input.in); libc::close(pipe_output.out); libc::close(pipe_err.out); @@ -248,6 +279,7 @@ pub fn start_program(prog: &str, args: &[~str]) -> @Program { struct ProgRepr { pid: pid_t, + handle: *(), in_fd: c_int, out_file: *libc::FILE, err_file: *libc::FILE, @@ -317,6 +349,7 @@ pub fn start_program(prog: &str, args: &[~str]) -> @Program { finish_repr(cast::transmute(&self.r)); close_repr_outputs(cast::transmute(&self.r)); } + free_handle(self.r.handle); } } @@ -343,8 +376,9 @@ pub fn start_program(prog: &str, args: &[~str]) -> @Program { fn force_destroy(&mut self) { destroy_repr(&mut self.r, true); } } - let repr = ProgRepr { - pid: pid, + let mut repr = ProgRepr { + pid: res.pid, + handle: res.handle, in_fd: pipe_input.out, out_file: os::fdopen(pipe_output.in), err_file: os::fdopen(pipe_err.in), @@ -385,13 +419,13 @@ pub fn program_output(prog: &str, args: &[~str]) -> ProgramOutput { let pipe_in = os::pipe(); let pipe_out = os::pipe(); let pipe_err = os::pipe(); - let pid = spawn_process(prog, args, &None, &None, - pipe_in.in, pipe_out.out, pipe_err.out); + let res = spawn_process_internal(prog, args, &None, &None, + pipe_in.in, pipe_out.out, pipe_err.out); os::close(pipe_in.in); os::close(pipe_out.out); os::close(pipe_err.out); - if pid == -1i32 { + if res.pid == -1i32 { os::close(pipe_in.out); os::close(pipe_out.in); os::close(pipe_err.in); @@ -415,7 +449,10 @@ pub fn program_output(prog: &str, args: &[~str]) -> ProgramOutput { let output = readclose(pipe_out.in); ch_clone.send((1, output)); }; - let status = run::waitpid(pid); + + let status = waitpid(res.pid); + free_handle(res.handle); + let mut errs = ~""; let mut outs = ~""; let mut count = 2; @@ -563,6 +600,12 @@ mod tests { assert!(status == 1); } + #[test] + #[should_fail] + fn waitpid_non_existant_pid() { + run::waitpid(123456789); // assume that this pid doesn't exist + } + #[test] fn test_destroy_once() { let mut p = run::start_program("echo", []); diff --git a/src/rt/rust_run_program.cpp b/src/rt/rust_run_program.cpp index 0e8d1d09ee9..cf4beed1a00 100644 --- a/src/rt/rust_run_program.cpp +++ b/src/rt/rust_run_program.cpp @@ -15,6 +15,11 @@ #include #endif +struct RunProgramResult { + pid_t pid; + void* handle; +}; + #if defined(__WIN32__) #include @@ -78,7 +83,7 @@ void append_arg(char *& buf, char const *arg, bool last) { } } -extern "C" CDECL int +extern "C" CDECL RunProgramResult rust_run_program(const char* argv[], void* envp, const char* dir, @@ -88,19 +93,21 @@ rust_run_program(const char* argv[], si.cb = sizeof(STARTUPINFO); si.dwFlags = STARTF_USESTDHANDLES; + RunProgramResult result = {-1, NULL}; + HANDLE curproc = GetCurrentProcess(); HANDLE origStdin = (HANDLE)_get_osfhandle(in_fd ? in_fd : 0); if (!DuplicateHandle(curproc, origStdin, curproc, &si.hStdInput, 0, 1, DUPLICATE_SAME_ACCESS)) - return -1; + return result; HANDLE origStdout = (HANDLE)_get_osfhandle(out_fd ? out_fd : 1); if (!DuplicateHandle(curproc, origStdout, curproc, &si.hStdOutput, 0, 1, DUPLICATE_SAME_ACCESS)) - return -1; + return result; HANDLE origStderr = (HANDLE)_get_osfhandle(err_fd ? err_fd : 2); if (!DuplicateHandle(curproc, origStderr, curproc, &si.hStdError, 0, 1, DUPLICATE_SAME_ACCESS)) - return -1; + return result; size_t cmd_len = 0; for (const char** arg = argv; *arg; arg++) { @@ -124,18 +131,39 @@ rust_run_program(const char* argv[], CloseHandle(si.hStdError); free(cmd); - if (!created) return -1; - return (int)pi.hProcess; + if (!created) { + return result; + } + + // We close the thread handle because we don't care about keeping the thread id valid, + // and we aren't keeping the thread handle around to be able to close it later. We don't + // close the process handle however because we want the process id to stay valid at least + // until the calling rust code closes the process handle. + CloseHandle(pi.hThread); + result.pid = pi.dwProcessId; + result.handle = pi.hProcess; + return result; } extern "C" CDECL int -rust_process_wait(int proc) { +rust_process_wait(int pid) { + + HANDLE proc = OpenProcess(SYNCHRONIZE | PROCESS_QUERY_INFORMATION, FALSE, pid); + if (proc == NULL) { + return -1; + } + DWORD status; while (true) { - if (GetExitCodeProcess((HANDLE)proc, &status) && - status != STILL_ACTIVE) - return (int)status; - WaitForSingleObject((HANDLE)proc, INFINITE); + if (!GetExitCodeProcess(proc, &status)) { + CloseHandle(proc); + return -1; + } + if (status != STILL_ACTIVE) { + CloseHandle(proc); + return (int) status; + } + WaitForSingleObject(proc, INFINITE); } } @@ -151,13 +179,16 @@ rust_process_wait(int proc) { extern char **environ; #endif -extern "C" CDECL int +extern "C" CDECL RunProgramResult rust_run_program(const char* argv[], void* envp, const char* dir, int in_fd, int out_fd, int err_fd) { int pid = fork(); - if (pid != 0) return pid; + if (pid != 0) { + RunProgramResult result = {pid, NULL}; + return result; + } sigset_t sset; sigemptyset(&sset); @@ -187,7 +218,7 @@ rust_run_program(const char* argv[], } extern "C" CDECL int -rust_process_wait(int proc) { +rust_process_wait(int pid) { // FIXME: stub; exists to placate linker. (#2692) return 0; }