Skip to content

Commit 7128f15

Browse files
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.
1 parent 59d6b3c commit 7128f15

File tree

3 files changed

+29
-9
lines changed

3 files changed

+29
-9
lines changed

src/unistd.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,18 @@ impl ForkResult {
191191
/// Continuing execution in parent process, new child has pid: 1234
192192
/// I'm a new child process
193193
/// ```
194+
///
195+
/// # Safety
196+
///
197+
/// In a multithreaded program, only [async-signal-safe] functions like `pause`
198+
/// and `_exit` may be called by the child (the parent isn't restricted). Note
199+
/// that memory allocation may **not** be async-signal-safe and thus must be
200+
/// prevented.
201+
///
202+
/// Those functions are only a small subset of your operating system's API, so
203+
/// special care must be taken to only invoke code you can control and audit.
204+
///
205+
/// [async-signal-safe]: http://man7.org/linux/man-pages/man7/signal-safety.7.html
194206
#[inline]
195207
pub fn fork() -> Result<ForkResult> {
196208
use self::ForkResult::*;
@@ -687,7 +699,7 @@ pub fn gethostname<'a>(buffer: &'a mut [u8]) -> Result<&'a CStr> {
687699
/// use nix::unistd::close;
688700
///
689701
/// fn main() {
690-
/// let mut f = tempfile::tempfile().unwrap();
702+
/// let f = tempfile::tempfile().unwrap();
691703
/// close(f.as_raw_fd()).unwrap(); // Bad! f will also close on drop!
692704
/// }
693705
/// ```
@@ -700,7 +712,7 @@ pub fn gethostname<'a>(buffer: &'a mut [u8]) -> Result<&'a CStr> {
700712
/// use nix::unistd::close;
701713
///
702714
/// fn main() {
703-
/// let mut f = tempfile::tempfile().unwrap();
715+
/// let f = tempfile::tempfile().unwrap();
704716
/// close(f.into_raw_fd()).unwrap(); // Good. into_raw_fd consumes f
705717
/// }
706718
/// ```

test/sys/test_wait.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,16 @@ use nix::unistd::*;
22
use nix::unistd::ForkResult::*;
33
use nix::sys::signal::*;
44
use nix::sys::wait::*;
5-
use libc::exit;
5+
use libc::_exit;
66

77
#[test]
88
fn test_wait_signal() {
99
#[allow(unused_variables)]
1010
let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
1111

12+
// Safe: The child only calls `pause` and/or `_exit`, which are async-signal-safe.
1213
match fork() {
13-
Ok(Child) => pause().unwrap_or(()),
14+
Ok(Child) => pause().unwrap_or_else(|_| unsafe { _exit(123) }),
1415
Ok(Parent { child }) => {
1516
kill(child, Some(SIGKILL)).ok().expect("Error: Kill Failed");
1617
assert_eq!(waitpid(child, None), Ok(WaitStatus::Signaled(child, SIGKILL, false)));
@@ -25,8 +26,9 @@ fn test_wait_exit() {
2526
#[allow(unused_variables)]
2627
let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
2728

29+
// Safe: Child only calls `_exit`, which is async-signal-safe.
2830
match fork() {
29-
Ok(Child) => unsafe { exit(12); },
31+
Ok(Child) => unsafe { _exit(12); },
3032
Ok(Parent { child }) => {
3133
assert_eq!(waitpid(child, None), Ok(WaitStatus::Exited(child, 12)));
3234
},

test/test_unistd.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,17 @@ use std::io::Write;
1111
use std::os::unix::prelude::*;
1212
use tempfile::tempfile;
1313
use tempdir::TempDir;
14-
use libc::off_t;
15-
use std::process::exit;
14+
use libc::{_exit, off_t};
1615

1716
#[test]
1817
fn test_fork_and_waitpid() {
1918
#[allow(unused_variables)]
2019
let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
20+
21+
// Safe: Child only calls `_exit`, which is signal-safe
2122
let pid = fork();
2223
match pid {
23-
Ok(Child) => exit(0),
24+
Ok(Child) => unsafe { _exit(0) },
2425
Ok(Parent { child }) => {
2526
// assert that child was created and pid > 0
2627
let child_raw: ::libc::pid_t = child.into();
@@ -48,9 +49,11 @@ fn test_wait() {
4849
// Grab FORK_MTX so wait doesn't reap a different test's child process
4950
#[allow(unused_variables)]
5051
let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
52+
53+
// Safe: Child only calls `_exit`, which is signal-safe
5154
let pid = fork();
5255
match pid {
53-
Ok(Child) => exit(0),
56+
Ok(Child) => unsafe { _exit(0) },
5457
Ok(Parent { child }) => {
5558
let wait_status = wait();
5659

@@ -111,6 +114,9 @@ macro_rules! execve_test_factory(
111114
// data from `reader`.
112115
let (reader, writer) = pipe().unwrap();
113116

117+
// Safe: Child calls `exit`, `dup`, `close` and the provided `exec*` family function.
118+
// NOTE: Technically, this makes the macro unsafe to use because you could pass anything.
119+
// The tests make sure not to do that, though.
114120
match fork().unwrap() {
115121
Child => {
116122
#[cfg(not(target_os = "android"))]

0 commit comments

Comments
 (0)