Add a way to way to wait for process groups. (#846)

When the `Pid` type changed to only accept positive pid values,
the `waitpid` function became unable to wait for process groups,
because that involves negative pids. Fully fixing this will
require a semver bump, but for now, we can add a new `waitpgid`
function to add support for waiting for a process group.

Also add a `Pgid` arm to `WaitId`, to support waiting for process
groups from `waitid` as well.

CONTRIBUTING.md

@@ -9,15 +9,15 @@ To keep compile times low, most features in rustix's API are behind cargo
 features. A special feature, `all-apis` enables all APIs, which is useful
 for testing.
 
-```
+```console
 cargo test --features=all-apis
 ```
 
 And, rustix has two backends, linux_raw and libc, and only one is used in
 any given build. To test with the libc backend explicitly, additionally
 enable the `use-libc` feature:
 
-```
+```console
 cargo test --features=all-apis,use-libc
 ```
 

src/backend/libc/process/syscalls.rs

@@ -386,6 +386,12 @@ pub(crate) fn waitpid(
     _waitpid(Pid::as_raw(pid), waitopts)
 }
 
+#[cfg(not(any(target_os = "espidf", target_os = "wasi")))]
+#[inline]
+pub(crate) fn waitpgid(pgid: Pid, waitopts: WaitOptions) -> io::Result<Option<(Pid, WaitStatus)>> {
+    _waitpid(-pgid.as_raw_nonzero().get(), waitopts)
+}
+
 #[cfg(not(any(target_os = "espidf", target_os = "wasi")))]
 #[inline]
 pub(crate) fn _waitpid(
@@ -411,6 +417,7 @@ pub(crate) fn waitid(id: WaitId<'_>, options: WaitidOptions) -> io::Result<Optio
     match id {
         WaitId::All => _waitid_all(options),
         WaitId::Pid(pid) => _waitid_pid(pid, options),
+        WaitId::Pgid(pgid) => _waitid_pgid(pgid, options),
         #[cfg(target_os = "linux")]
         WaitId::PidFd(fd) => _waitid_pidfd(fd, options),
         #[cfg(not(target_os = "linux"))]
@@ -464,6 +471,29 @@ fn _waitid_pid(pid: Pid, options: WaitidOptions) -> io::Result<Option<WaitidStat
     Ok(unsafe { cvt_waitid_status(status) })
 }
 
+#[cfg(not(any(
+    target_os = "espidf",
+    target_os = "redox",
+    target_os = "openbsd",
+    target_os = "wasi"
+)))]
+#[inline]
+fn _waitid_pgid(pgid: Option<Pid>, options: WaitidOptions) -> io::Result<Option<WaitidStatus>> {
+    // `waitid` can return successfully without initializing the struct (no
+    // children found when using `WNOHANG`)
+    let mut status = MaybeUninit::<c::siginfo_t>::zeroed();
+    unsafe {
+        ret(c::waitid(
+            c::P_PGID,
+            Pid::as_raw(pgid) as _,
+            status.as_mut_ptr(),
+            options.bits() as _,
+        ))?
+    };
+
+    Ok(unsafe { cvt_waitid_status(status) })
+}
+
 #[cfg(target_os = "linux")]
 #[inline]
 fn _waitid_pidfd(fd: BorrowedFd<'_>, options: WaitidOptions) -> io::Result<Option<WaitidStatus>> {

src/backend/linux_raw/c.rs

@@ -90,7 +90,7 @@ pub(crate) const EXIT_SIGNALED_SIGABRT: c_int = 128 + linux_raw_sys::general::SI
 pub(crate) use linux_raw_sys::{
     general::{
         CLD_CONTINUED, CLD_DUMPED, CLD_EXITED, CLD_KILLED, CLD_STOPPED, CLD_TRAPPED,
-        O_NONBLOCK as PIDFD_NONBLOCK, P_ALL, P_PID, P_PIDFD,
+        O_NONBLOCK as PIDFD_NONBLOCK, P_ALL, P_PGID, P_PID, P_PIDFD,
     },
     ioctl::TIOCSCTTY,
 };

src/backend/linux_raw/process/syscalls.rs

@@ -435,6 +435,11 @@ pub(crate) fn waitpid(
     _waitpid(Pid::as_raw(pid), waitopts)
 }
 
+#[inline]
+pub(crate) fn waitpgid(pgid: Pid, waitopts: WaitOptions) -> io::Result<Option<(Pid, WaitStatus)>> {
+    _waitpid(-pgid.as_raw_nonzero().get(), waitopts)
+}
+
 #[inline]
 pub(crate) fn _waitpid(
     pid: RawPid,
@@ -459,6 +464,7 @@ pub(crate) fn waitid(id: WaitId<'_>, options: WaitidOptions) -> io::Result<Optio
     match id {
         WaitId::All => _waitid_all(options),
         WaitId::Pid(pid) => _waitid_pid(pid, options),
+        WaitId::Pgid(pid) => _waitid_pgid(pid, options),
         WaitId::PidFd(fd) => _waitid_pidfd(fd, options),
     }
 }
@@ -501,6 +507,25 @@ fn _waitid_pid(pid: Pid, options: WaitidOptions) -> io::Result<Option<WaitidStat
     Ok(unsafe { cvt_waitid_status(status) })
 }
 
+#[inline]
+fn _waitid_pgid(pgid: Option<Pid>, options: WaitidOptions) -> io::Result<Option<WaitidStatus>> {
+    // `waitid` can return successfully without initializing the struct (no
+    // children found when using `WNOHANG`)
+    let mut status = MaybeUninit::<c::siginfo_t>::zeroed();
+    unsafe {
+        ret(syscall!(
+            __NR_waitid,
+            c_uint(c::P_PGID),
+            c_int(Pid::as_raw(pgid)),
+            by_mut(&mut status),
+            c_int(options.bits() as _),
+            zero()
+        ))?
+    };
+
+    Ok(unsafe { cvt_waitid_status(status) })
+}
+
 #[inline]
 fn _waitid_pidfd(fd: BorrowedFd<'_>, options: WaitidOptions) -> io::Result<Option<WaitidStatus>> {
     // `waitid` can return successfully without initializing the struct (no

src/process/wait.rs

@@ -254,40 +254,50 @@ impl WaitidStatus {
 #[non_exhaustive]
 pub enum WaitId<'a> {
     /// Wait on all processes.
+    #[doc(alias = "P_ALL")]
     All,
 
     /// Wait for a specific process ID.
+    #[doc(alias = "P_PID")]
     Pid(Pid),
 
+    /// Wait for a specific process group ID, or the calling process' group ID.
+    #[doc(alias = "P_PGID")]
+    Pgid(Option<Pid>),
+
     /// Wait for a specific process file descriptor.
     #[cfg(target_os = "linux")]
+    #[doc(alias = "P_PIDFD")]
     PidFd(BorrowedFd<'a>),
 
     /// Eat the lifetime for non-Linux platforms.
     #[doc(hidden)]
     #[cfg(not(target_os = "linux"))]
     __EatLifetime(core::marker::PhantomData<&'a ()>),
-    // TODO(notgull): Once this crate has the concept of PGIDs, add a WaitId::Pgid
 }
 
 /// `waitpid(pid, waitopts)`—Wait for a specific process to change state.
 ///
 /// If the pid is `None`, the call will wait for any child process whose
 /// process group id matches that of the calling process.
 ///
-/// If the pid is equal to `RawPid::MAX`, the call will wait for any child
-/// process.
-///
-/// Otherwise if the `wrapping_neg` of pid is less than pid, the call will wait
-/// for any child process with a group ID equal to the `wrapping_neg` of `pid`.
-///
 /// Otherwise, the call will wait for the child process with the given pid.
 ///
 /// On Success, returns the status of the selected process.
 ///
 /// If `NOHANG` was specified in the options, and the selected child process
 /// didn't change state, returns `None`.
 ///
+/// # Bugs
+///
+/// This function does not currently support waiting for given process group
+/// (the < 0 case of `waitpid`); to do that, currently the [`waitpgid`] or
+/// [`waitid`] function must be used.
+///
+/// This function does not currently support waiting for any process (the
+/// `-1` case of `waitpid`); to do that, currently the [`wait`] function must
+/// be used.
+///
 /// # References
 ///  - [POSIX]
 ///  - [Linux]
@@ -300,6 +310,28 @@ pub fn waitpid(pid: Option<Pid>, waitopts: WaitOptions) -> io::Result<Option<Wai
     Ok(backend::process::syscalls::waitpid(pid, waitopts)?.map(|(_, status)| status))
 }
 
+/// `waitpid(-pgid, waitopts)`—Wait for a process in a specific process group
+/// to change state.
+///
+/// The call will wait for any child process with the given pgid.
+///
+/// On Success, returns the status of the selected process.
+///
+/// If `NOHANG` was specified in the options, and no selected child process
+/// changed state, returns `None`.
+///
+/// # References
+///  - [POSIX]
+///  - [Linux]
+///
+/// [POSIX]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/wait.html
+/// [Linux]: https://man7.org/linux/man-pages/man2/waitpid.2.html
+#[cfg(not(target_os = "wasi"))]
+#[inline]
+pub fn waitpgid(pgid: Pid, waitopts: WaitOptions) -> io::Result<Option<WaitStatus>> {
+    Ok(backend::process::syscalls::waitpgid(pgid, waitopts)?.map(|(_, status)| status))
+}
+
 /// `wait(waitopts)`—Wait for any of the children of calling process to
 /// change state.
 ///

tests/process/wait.rs

@@ -10,7 +10,23 @@ use std::process::{Command, Stdio};
 
 #[test]
 #[serial]
-fn test_waitpid() {
+fn test_waitpid_none() {
+    let child = Command::new("yes")
+        .stdout(Stdio::null())
+        .stderr(Stdio::null())
+        .spawn()
+        .expect("failed to execute child");
+    unsafe { kill(child.id() as _, SIGSTOP) };
+
+    let status = process::waitpid(None, process::WaitOptions::UNTRACED)
+        .expect("failed to wait")
+        .unwrap();
+    assert!(status.stopped());
+}
+
+#[test]
+#[serial]
+fn test_waitpid_some() {
     let child = Command::new("yes")
         .stdout(Stdio::null())
         .stderr(Stdio::null())
@@ -25,6 +41,23 @@ fn test_waitpid() {
     assert!(status.stopped());
 }
 
+#[test]
+#[serial]
+fn test_waitpgid() {
+    let child = Command::new("yes")
+        .stdout(Stdio::null())
+        .stderr(Stdio::null())
+        .spawn()
+        .expect("failed to execute child");
+    unsafe { kill(child.id() as _, SIGSTOP) };
+
+    let pgid = process::getpgrp();
+    let status = process::waitpgid(pgid, process::WaitOptions::UNTRACED)
+        .expect("failed to wait")
+        .unwrap();
+    assert!(status.stopped());
+}
+
 #[cfg(not(any(
     target_os = "wasi",
     target_os = "emscripten",
@@ -39,9 +72,13 @@ fn test_waitid() {
         .stderr(Stdio::null())
         .spawn()
         .expect("failed to execute child");
+    let pid = process::Pid::from_child(&child);
+    let pgid = process::getpgid(Some(pid)).unwrap();
+
+    // Test waiting for the process by pid.
+
     unsafe { kill(child.id() as _, SIGSTOP) };
 
-    let pid = process::Pid::from_child(&child);
     let status = process::waitid(process::WaitId::Pid(pid), process::WaitidOptions::STOPPED)
         .expect("failed to wait")
         .unwrap();
@@ -58,6 +95,32 @@ fn test_waitid() {
 
     assert!(status.continued());
 
+    // Now do the same thing with the pgid.
+
+    unsafe { kill(child.id() as _, SIGSTOP) };
+
+    let status = process::waitid(
+        process::WaitId::Pgid(Some(pgid)),
+        process::WaitidOptions::STOPPED,
+    )
+    .expect("failed to wait")
+    .unwrap();
+
+    assert!(status.stopped());
+    #[cfg(not(any(target_os = "fuchsia", target_os = "netbsd")))]
+    assert_eq!(status.stopping_signal(), Some(SIGSTOP as _));
+
+    unsafe { kill(child.id() as _, SIGCONT) };
+
+    let status = process::waitid(
+        process::WaitId::Pgid(Some(pgid)),
+        process::WaitidOptions::CONTINUED,
+    )
+    .expect("failed to wait")
+    .unwrap();
+
+    assert!(status.continued());
+
     let status = process::waitid(
         process::WaitId::All,
         process::WaitidOptions::EXITED | process::WaitidOptions::NOHANG,