Skip to content

Commit

Permalink
Change the behaviour of core::run::Program.destroy to
Browse files Browse the repository at this point in the history
forcibly terminate the program (as suggested in issue rust-lang#5632)
  • Loading branch information
gareth authored and gareth committed Apr 6, 2013
1 parent 622bb63 commit 483e95a
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 7 deletions.
16 changes: 16 additions & 0 deletions src/libcore/libc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,7 @@ pub mod consts {
pub static F_TEST : int = 3;
pub static F_TLOCK : int = 2;
pub static F_ULOCK : int = 0;
pub static SIGKILL : int = 9;
}
pub mod posix01 {
}
Expand Down Expand Up @@ -930,6 +931,7 @@ pub mod consts {
pub static F_TEST : int = 3;
pub static F_TLOCK : int = 2;
pub static F_ULOCK : int = 0;
pub static SIGKILL : int = 9;
}
pub mod posix01 {
}
Expand Down Expand Up @@ -998,6 +1000,7 @@ pub mod consts {
pub static F_TEST : int = 3;
pub static F_TLOCK : int = 2;
pub static F_ULOCK : int = 0;
pub static SIGKILL : int = 9;
}
pub mod posix01 {
}
Expand Down Expand Up @@ -1482,6 +1485,17 @@ pub mod funcs {
-> ssize_t;
}
}

#[nolink]
#[abi = "cdecl"]
pub mod signal {
use libc::types::os::arch::c95::{c_int};
use libc::types::os::arch::posix88::{pid_t};

pub extern {
unsafe fn kill(pid: pid_t, sig: c_int) -> c_int;
}
}
}

#[cfg(target_os = "linux")]
Expand Down Expand Up @@ -1623,6 +1637,7 @@ pub mod funcs {
pub mod extra {

pub mod kernel32 {
use libc::types::os::arch::c95::{c_uint};
use libc::types::os::arch::extra::{BOOL, DWORD, HMODULE};
use libc::types::os::arch::extra::{LPCWSTR, LPWSTR, LPTCH};
use libc::types::os::arch::extra::{LPSECURITY_ATTRIBUTES};
Expand Down Expand Up @@ -1663,6 +1678,7 @@ pub mod funcs {
findFileData: HANDLE)
-> BOOL;
unsafe fn FindClose(findFile: HANDLE) -> BOOL;
unsafe fn TerminateProcess(hProcess: HANDLE, uExitCode: c_uint) -> BOOL;
}
}

Expand Down
66 changes: 59 additions & 7 deletions src/libcore/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ pub trait Program {
*/
fn finish(&mut self) -> int;

/// Closes open handles
/**
* Forcibly terminate the program. On Posix OSs SIGKILL will be sent
* to the process. On Win32 TerminateProcess(..) will be called.
*/
fn destroy(&mut self);
}

Expand Down Expand Up @@ -248,28 +251,53 @@ pub fn start_program(prog: &str, args: &[~str]) -> @Program {
r.in_fd = invalid_fd;
}
}

fn close_repr_outputs(r: &mut ProgRepr) {
unsafe {
fclose_and_null(&mut r.out_file);
fclose_and_null(&mut r.err_file);
}
}

fn finish_repr(r: &mut ProgRepr) -> int {
if r.finished { return 0; }
r.finished = true;
close_repr_input(&mut *r);
return waitpid(r.pid);
}

fn destroy_repr(r: &mut ProgRepr) {
unsafe {
finish_repr(&mut *r);
fclose_and_null(&mut r.out_file);
fclose_and_null(&mut r.err_file);
killpid(r.pid);
finish_repr(&mut *r);
close_repr_outputs(&mut *r);

#[cfg(windows)]
fn killpid(pid: pid_t) {
unsafe {
libc::funcs::extra::kernel32::TerminateProcess(
cast::transmute(pid), 1);
}
}

#[cfg(unix)]
fn killpid(pid: pid_t) {
unsafe {
libc::funcs::posix88::signal::kill(
pid, libc::consts::os::posix88::SIGKILL as c_int);
}
}
}

struct ProgRes {
r: ProgRepr,
}

impl Drop for ProgRes {
fn finalize(&self) {
unsafe {
// FIXME #4943: This is bad.
destroy_repr(cast::transmute(&self.r));
// FIXME #4943: transmute is bad.
finish_repr(cast::transmute(&self.r));
close_repr_outputs(cast::transmute(&self.r));
}
}
}
Expand All @@ -295,6 +323,7 @@ pub fn start_program(prog: &str, args: &[~str]) -> @Program {
fn finish(&mut self) -> int { finish_repr(&mut self.r) }
fn destroy(&mut self) { destroy_repr(&mut self.r); }
}

let mut repr = ProgRepr {
pid: pid,
in_fd: pipe_input.out,
Expand Down Expand Up @@ -466,8 +495,10 @@ pub fn waitpid(pid: pid_t) -> int {

#[cfg(test)]
mod tests {
use libc;
use option::None;
use os;
use path::Path;
use run::{readclose, writeclose};
use run;

Expand Down Expand Up @@ -528,6 +559,27 @@ mod tests {
p.destroy(); // ...and nor should this (and nor should the destructor)
}

#[test]
#[cfg(unix)] // there is no way to sleep on windows from inside libcore...
pub fn test_destroy_actually_kills() {
let path = Path("test/core-run-test-destroy-actually-kills.tmp");

os::remove_file(&path);

let cmd = fmt!("sleep 5 && echo MurderDeathKill > %s", path.to_str());
let mut p = run::start_program("sh", [~"-c", cmd]);

p.destroy(); // destroy the program before it has a chance to echo its message

unsafe {
// wait to ensure the program is really destroyed and not just waiting itself
libc::sleep(10);
}

// the program should not have had chance to echo its message
assert!(!path.exists());
}

}

// Local Variables:
Expand Down

0 comments on commit 483e95a

Please sign in to comment.