Fix issue #5976 - HANDLE leaks and undefined/bad behavour

on windows.
This commit is contained in:
gareth 2013-04-22 20:05:07 +01:00
parent d9896d592b
commit 91aeecf7e3
4 changed files with 119 additions and 40 deletions

View File

@ -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;
}
}

View File

@ -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;
}
}

View File

@ -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", []);

View File

@ -15,6 +15,11 @@
#include <crt_externs.h>
#endif
struct RunProgramResult {
pid_t pid;
void* handle;
};
#if defined(__WIN32__)
#include <process.h>
@ -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;
}