Document invariants of fork and fix tests

Some tests were invoking non-async-signal-safe functions from the child
process after a `fork`. Since they might be invoked in parallel, this
could lead to problems.

3 files changed, 29 insertions, 9 deletions

diff --git a/src/unistd.rs b/src/unistd.rs
index 25237a07..19f82ef6 100644
--- a/src/unistd.rs
+++ b/src/unistd.rs
@@ -191,6 +191,18 @@ impl ForkResult {
 /// Continuing execution in parent process, new child has pid: 1234
 /// I'm a new child process
 /// ```
+///
+/// # Safety
+///
+/// In a multithreaded program, only [async-signal-safe] functions like `pause`
+/// and `_exit` may be called by the child (the parent isn't restricted). Note
+/// that memory allocation may **not** be async-signal-safe and thus must be
+/// prevented.
+///
+/// Those functions are only a small subset of your operating system's API, so
+/// special care must be taken to only invoke code you can control and audit.
+///
+/// [async-signal-safe]: http://man7.org/linux/man-pages/man7/signal-safety.7.html
 #[inline]
 pub fn fork() -> Result<ForkResult> {
     use self::ForkResult::*;
@@ -687,7 +699,7 @@ pub fn gethostname<'a>(buffer: &'a mut [u8]) -> Result<&'a CStr> {
 /// use nix::unistd::close;
 ///
 /// fn main() {
-///     let mut f = tempfile::tempfile().unwrap();
+///     let f = tempfile::tempfile().unwrap();
 ///     close(f.as_raw_fd()).unwrap();   // Bad!  f will also close on drop!
 /// }
 /// ```
@@ -700,7 +712,7 @@ pub fn gethostname<'a>(buffer: &'a mut [u8]) -> Result<&'a CStr> {
 /// use nix::unistd::close;
 ///
 /// fn main() {
-///     let mut f = tempfile::tempfile().unwrap();
+///     let f = tempfile::tempfile().unwrap();
 ///     close(f.into_raw_fd()).unwrap(); // Good.  into_raw_fd consumes f
 /// }
 /// ```
diff --git a/test/sys/test_wait.rs b/test/sys/test_wait.rs
index 5f6c9231..e1d3464b 100644
--- a/test/sys/test_wait.rs
+++ b/test/sys/test_wait.rs
@@ -2,15 +2,16 @@ use nix::unistd::*;
 use nix::unistd::ForkResult::*;
 use nix::sys::signal::*;
 use nix::sys::wait::*;
-use libc::exit;
+use libc::_exit;
 
 #[test]
 fn test_wait_signal() {
     #[allow(unused_variables)]
     let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
+    // Safe: The child only calls `pause` and/or `_exit`, which are async-signal-safe.
     match fork() {
-      Ok(Child) => pause().unwrap_or(()),
+      Ok(Child) => pause().unwrap_or_else(|_| unsafe { _exit(123) }),
       Ok(Parent { child }) => {
           kill(child, Some(SIGKILL)).ok().expect("Error: Kill Failed");
           assert_eq!(waitpid(child, None), Ok(WaitStatus::Signaled(child, SIGKILL, false)));
@@ -25,8 +26,9 @@ fn test_wait_exit() {
     #[allow(unused_variables)]
     let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
+    // Safe: Child only calls `_exit`, which is async-signal-safe.
     match fork() {
-      Ok(Child) => unsafe { exit(12); },
+      Ok(Child) => unsafe { _exit(12); },
       Ok(Parent { child }) => {
           assert_eq!(waitpid(child, None), Ok(WaitStatus::Exited(child, 12)));
       },
diff --git a/test/test_unistd.rs b/test/test_unistd.rs
index 035af52c..4de56826 100644
--- a/test/test_unistd.rs
+++ b/test/test_unistd.rs
@@ -11,16 +11,17 @@ use std::io::Write;
 use std::os::unix::prelude::*;
 use tempfile::tempfile;
 use tempdir::TempDir;
-use libc::off_t;
-use std::process::exit;
+use libc::{_exit, off_t};
 
 #[test]
 fn test_fork_and_waitpid() {
     #[allow(unused_variables)]
     let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+
+    // Safe: Child only calls `_exit`, which is signal-safe
     let pid = fork();
     match pid {
-        Ok(Child) => exit(0),
+        Ok(Child) => unsafe { _exit(0) },
         Ok(Parent { child }) => {
             // assert that child was created and pid > 0
             let child_raw: ::libc::pid_t = child.into();
@@ -48,9 +49,11 @@ fn test_wait() {
     // Grab FORK_MTX so wait doesn't reap a different test's child process
     #[allow(unused_variables)]
     let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+
+    // Safe: Child only calls `_exit`, which is signal-safe
     let pid = fork();
     match pid {
-        Ok(Child) => exit(0),
+        Ok(Child) => unsafe { _exit(0) },
         Ok(Parent { child }) => {
             let wait_status = wait();
 
@@ -112,6 +115,9 @@ macro_rules! execve_test_factory(
         // data from `reader`.
         let (reader, writer) = pipe().unwrap();
 
+        // Safe: Child calls `exit`, `dup`, `close` and the provided `exec*` family function.
+        // NOTE: Technically, this makes the macro unsafe to use because you could pass anything.
+        //       The tests make sure not to do that, though.
         match fork().unwrap() {
             Child => {
                 #[cfg(not(target_os = "android"))]