rollup merge of #19454: nodakai/libstd-reap-failed-child

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 in `libstd/io/process.rs` calls the ps command to check
if the new code can really reap a zombie.
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
...
```

where `./spawn-failure` is my test program which intentionally leaves many
zombies.  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.

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();
+}