Change the behaviour of core::run::Program.destroy to forcibly terminate the program

src/libcore/libc.rs

@@ -863,6 +863,8 @@ 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 static SIGTERM : int = 15;
         }
         pub mod posix01 {
         }
@@ -930,6 +932,8 @@ 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 static SIGTERM : int = 15;
         }
         pub mod posix01 {
         }
@@ -998,6 +1002,8 @@ 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 static SIGTERM : int = 15;
         }
         pub mod posix01 {
         }
@@ -1482,6 +1488,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")]
@@ -1623,6 +1640,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};
@@ -1663,6 +1681,7 @@ pub mod funcs {
                                        findFileData: HANDLE)
                     -> BOOL;
                 unsafe fn FindClose(findFile: HANDLE) -> BOOL;
+                unsafe fn TerminateProcess(hProcess: HANDLE, uExitCode: c_uint) -> BOOL;
             }
         }
 

src/libcore/run.rs

@@ -1,4 +1,4 @@
-// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
+// Copyright 2012-2013 The Rust Project Developers. See the COPYRIGHT
 // file at the top-level directory of this distribution and at
 // http://rust-lang.org/COPYRIGHT.
 //
@@ -44,13 +44,13 @@ pub trait Program {
     /// Returns the process id of the program
     fn get_id(&mut self) -> pid_t;
 
-    /// Returns an io::writer that can be used to write to stdin
+    /// Returns an io::Writer that can be used to write to stdin
     fn input(&mut self) -> @io::Writer;
 
-    /// Returns an io::reader that can be used to read from stdout
+    /// Returns an io::Reader that can be used to read from stdout
     fn output(&mut self) -> @io::Reader;
 
-    /// Returns an io::reader that can be used to read from stderr
+    /// Returns an io::Reader that can be used to read from stderr
     fn err(&mut self) -> @io::Reader;
 
     /// Closes the handle to the child processes standard input
@@ -62,8 +62,23 @@ pub trait Program {
      */
     fn finish(&mut self) -> int;
 
-    /// Closes open handles
+    /**
+     * Terminate the program, giving it a chance to clean itself up if
+     * this is supported by the operating system.
+     *
+     * On Posix OSs SIGTERM will be sent to the process. On Win32
+     * TerminateProcess(..) will be called.
+     */
     fn destroy(&mut self);
+
+    /**
+     * Terminate the program as soon as possible without giving it a
+     * chance to clean itself up.
+     *
+     * On Posix OSs SIGKILL will be sent to the process. On Win32
+     * TerminateProcess(..) will be called.
+     */
+    fn force_destroy(&mut self);
 }
 
 
@@ -172,6 +187,14 @@ fn with_dirp<T>(d: &Option<~str>,
     }
 }
 
+/// helper function that closes non-NULL files and then makes them NULL
+priv unsafe fn fclose_and_null(f: &mut *libc::FILE) {
+    if *f != 0 as *libc::FILE {
+        libc::fclose(*f);
+        *f = 0 as *libc::FILE;
+    }
+}
+
 /**
  * Spawns a process and waits for it to terminate
  *
@@ -192,9 +215,9 @@ pub fn run_program(prog: &str, args: &[~str]) -> int {
 }
 
 /**
- * Spawns a process and returns a program
+ * Spawns a process and returns a Program
  *
- * The returned value is a boxed class containing a <program> object that can
+ * The returned value is a boxed class containing a <Program> object that can
  * be used for sending and receiving data over the standard file descriptors.
  * The class will ensure that file descriptors are closed properly.
  *
@@ -240,28 +263,59 @@ 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);
-            libc::fclose(r.out_file);
-            libc::fclose(r.err_file);
+
+    fn destroy_repr(r: &mut ProgRepr, force: bool) {
+        killpid(r.pid, force);
+        finish_repr(&mut *r);
+        close_repr_outputs(&mut *r);
+
+        #[cfg(windows)]
+        fn killpid(pid: pid_t, _force: bool) {
+            unsafe {
+                libc::funcs::extra::kernel32::TerminateProcess(
+                    cast::transmute(pid), 1);
+            }
+        }
+
+        #[cfg(unix)]
+        fn killpid(pid: pid_t, force: bool) {
+
+            let signal = if force {
+                libc::consts::os::posix88::SIGKILL
+            } else {
+                libc::consts::os::posix88::SIGTERM
+            };
+
+            unsafe {
+                libc::funcs::posix88::signal::kill(pid, signal 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));
             }
         }
     }
@@ -285,8 +339,10 @@ pub fn start_program(prog: &str, args: &[~str]) -> @Program {
         }
         fn close_input(&mut self) { close_repr_input(&mut self.r); }
         fn finish(&mut self) -> int { finish_repr(&mut self.r) }
-        fn destroy(&mut self) { destroy_repr(&mut self.r); }
+        fn destroy(&mut self) { destroy_repr(&mut self.r, false); }
+        fn force_destroy(&mut self) { destroy_repr(&mut self.r, true); }
     }
+
     let mut repr = ProgRepr {
         pid: pid,
         in_fd: pipe_input.out,
@@ -458,8 +514,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;
 
@@ -507,6 +565,50 @@ mod tests {
         assert!(status == 1);
     }
 
+    #[test]
+    pub fn test_destroy_once() {
+        let mut p = run::start_program("echo", []);
+        p.destroy(); // this shouldn't crash (and nor should the destructor)
+    }
+
+    #[test]
+    pub fn test_destroy_twice() {
+        let mut p = run::start_program("echo", []);
+        p.destroy(); // this shouldnt crash...
+        p.destroy(); // ...and nor should this (and nor should the destructor)
+    }
+
+    #[cfg(unix)] // there is no way to sleep on windows from inside libcore...
+    pub fn test_destroy_actually_kills(force: bool) {
+        let path = Path(fmt!("test/core-run-test-destroy-actually-kills-%?.tmp", force));
+
+        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());
+    }
+
+    #[test]
+    #[cfg(unix)]
+    pub fn test_unforced_destroy_actually_kills() {
+        test_destroy_actually_kills(false);
+    }
+
+    #[test]
+    #[cfg(unix)]
+    pub fn test_forced_destroy_actually_kills() {
+        test_destroy_actually_kills(true);
+    }
 }
 
 // Local Variables: