libstd/sys/unix/process.rs: reap a zombie who didn't get through to exec(2).

After the library successfully called fork(2), the child does several
setup works such as setting UID, GID and current directory before it
calls exec(2).  When those setup works failed, the child exits but the
parent didn't call waitpid(2) and left it as a zombie.

This patch also add several sanity checks.  They shouldn't make any
noticeable impact to runtime performance.

The new test case run-pass/wait-forked-but-failed-child.rs calls the ps
command to check if the new code can really reap a zombie.  When
I intentionally create many zombies with my test program
./spawn-failure, The output of "ps -A -o pid,sid,command" should look
like this:

  PID   SID COMMAND
    1     1 /sbin/init
    2     0 [kthreadd]
    3     0 [ksoftirqd/0]
...
12562  9237 ./spawn-failure
12563  9237 [spawn-failure] <defunct>
12564  9237 [spawn-failure] <defunct>
...
12592  9237 [spawn-failure] <defunct>
12593  9237 ps -A -o pid,sid,command
12884 12884 /bin/zsh
12922 12922 /bin/zsh
...

Filtering the output with the "SID" (session ID) column is a quick way
to tell if a process (zombie) was spawned by my own test program.  Then
the number of "defunct" lines is the number of zombie children.

Signed-off-by: NODA, Kai <nodakai@gmail.com>

2 files changed, 112 insertions, 10 deletions

diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs
index 76c316076f9..7dde19a6476 100644
--- a/src/libstd/sys/unix/process.rs
+++ b/src/libstd/sys/unix/process.rs
@@ -11,7 +11,7 @@ use self::Req::*;
 
 use libc::{mod, pid_t, c_void, c_int};
 use c_str::CString;
-use io::{mod, IoResult, IoError};
+use io::{mod, IoResult, IoError, EndOfFile};
 use mem;
 use os;
 use ptr;
@@ -39,6 +39,8 @@ enum Req {
     NewChild(libc::pid_t, Sender<ProcessExit>, u64),
 }
 
+const CLOEXEC_MSG_FOOTER: &'static [u8] = b"NOEX";
+
 impl Process {
     pub fn id(&self) -> pid_t {
         self.pid
@@ -106,18 +108,36 @@ impl Process {
                 if pid < 0 {
                     return Err(super::last_error())
                 } else if pid > 0 {
+                    #[inline]
+                    fn combine(arr: &[u8]) -> i32 {
+                        let a = arr[0] as u32;
+                        let b = arr[1] as u32;
+                        let c = arr[2] as u32;
+                        let d = arr[3] as u32;
+
+                        ((a << 24) | (b << 16) | (c << 8) | (d << 0)) as i32
+                    }
+
+                    let p = Process{ pid: pid };
                     drop(output);
-                    let mut bytes = [0, ..4];
+                    let mut bytes = [0, ..8];
                     return match input.read(&mut bytes) {
-                        Ok(4) => {
-                            let errno = (bytes[0] as i32 << 24) |
-                                        (bytes[1] as i32 << 16) |
-                                        (bytes[2] as i32 <<  8) |
-                                        (bytes[3] as i32 <<  0);
+                        Ok(8) => {
+                            assert!(combine(CLOEXEC_MSG_FOOTER) == combine(bytes.slice(4, 8)),
+                                "Validation on the CLOEXEC pipe failed: {}", bytes);
+                            let errno = combine(bytes.slice(0, 4));
+                            assert!(p.wait(0).is_ok(), "wait(0) should either return Ok or panic");
                             Err(super::decode_error(errno))
                         }
-                        Err(..) => Ok(Process { pid: pid }),
-                        Ok(..) => panic!("short read on the cloexec pipe"),
+                        Err(ref e) if e.kind == EndOfFile => Ok(p),
+                        Err(e) => {
+                            assert!(p.wait(0).is_ok(), "wait(0) should either return Ok or panic");
+                            panic!("the CLOEXEC pipe failed: {}", e)
+                        },
+                        Ok(..) => { // pipe I/O up to PIPE_BUF bytes should be atomic
+                            assert!(p.wait(0).is_ok(), "wait(0) should either return Ok or panic");
+                            panic!("short read on the CLOEXEC pipe")
+                        }
                     };
                 }
 
@@ -154,13 +174,16 @@ impl Process {
                 let _ = libc::close(input.fd());
 
                 fn fail(output: &mut FileDesc) -> ! {
-                    let errno = sys::os::errno();
+                    let errno = sys::os::errno() as u32;
                     let bytes = [
                         (errno >> 24) as u8,
                         (errno >> 16) as u8,
                         (errno >>  8) as u8,
                         (errno >>  0) as u8,
+                        CLOEXEC_MSG_FOOTER[0], CLOEXEC_MSG_FOOTER[1],
+                        CLOEXEC_MSG_FOOTER[2], CLOEXEC_MSG_FOOTER[3]
                     ];
+                    // pipe I/O up to PIPE_BUF bytes should be atomic
                     assert!(output.write(&bytes).is_ok());
                     unsafe { libc::_exit(1) }
                 }
diff --git a/src/test/run-pass/wait-forked-but-failed-child.rs b/src/test/run-pass/wait-forked-but-failed-child.rs
new file mode 100644
index 00000000000..17dfb9e3319
--- /dev/null
+++ b/src/test/run-pass/wait-forked-but-failed-child.rs
@@ -0,0 +1,79 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+
+extern crate libc;
+
+use std::io::process::Command;
+use std::iter::IteratorExt;
+
+use libc::funcs::posix88::unistd;
+
+
+// "ps -A -o pid,sid,command" with GNU ps should output something like this:
+//   PID   SID COMMAND
+//     1     1 /sbin/init
+//     2     0 [kthreadd]
+//     3     0 [ksoftirqd/0]
+// ...
+// 12562  9237 ./spawn-failure
+// 12563  9237 [spawn-failure] <defunct>
+// 12564  9237 [spawn-failure] <defunct>
+// ...
+// 12592  9237 [spawn-failure] <defunct>
+// 12593  9237 ps -A -o pid,sid,command
+// 12884 12884 /bin/zsh
+// 12922 12922 /bin/zsh
+// ...
+
+#[cfg(unix)]
+fn find_zombies() {
+    // http://man.freebsd.org/ps(1)
+    // http://man7.org/linux/man-pages/man1/ps.1.html
+    #[cfg(not(target_os = "macos"))]
+    const FIELDS: &'static str = "pid,sid,command";
+
+    // https://developer.apple.com/library/mac/documentation/Darwin/
+    // Reference/ManPages/man1/ps.1.html
+    #[cfg(target_os = "macos")]
+    const FIELDS: &'static str = "pid,sess,command";
+
+    let my_sid = unsafe { unistd::getsid(0) };
+
+    let ps_cmd_output = Command::new("ps").args(&["-A", "-o", FIELDS]).output().unwrap();
+    let ps_output = String::from_utf8_lossy(ps_cmd_output.output.as_slice());
+
+    let found = ps_output.split('\n').enumerate().any(|(line_no, line)|
+        0 < line_no && 0 < line.len() &&
+        my_sid == from_str(line.split(' ').filter(|w| 0 < w.len()).nth(1)
+            .expect("1st column should be Session ID")
+            ).expect("Session ID string into integer") &&
+        line.contains("defunct") && {
+            println!("Zombie child {}", line);
+            true
+        }
+    );
+
+    assert!( ! found, "Found at least one zombie child");
+}
+
+#[cfg(windows)]
+fn find_zombies() { }
+
+fn main() {
+    let too_long = format!("/NoSuchCommand{:0300}", 0u8);
+
+    for _ in range(0u32, 100) {
+        let invalid = Command::new(too_long.as_slice()).spawn();
+        assert!(invalid.is_err());
+    }
+
+    find_zombies();
+}