feat: Handle signal coalescing issue and add more tests

.github/workflows/release.yml

@@ -22,7 +22,7 @@ jobs:
         os:
         - ubuntu-latest
         toolchain:
-        - 1.88.0
+        - 1.91.0
 
     steps:
     - uses: actions/checkout@v4

.github/workflows/rust.yml

@@ -26,7 +26,7 @@ jobs:
         - macos-latest
         - windows-latest
         toolchain:
-        - 1.88.0
+        - 1.91.0
         - stable
         - nightly
     steps:
@@ -58,4 +58,4 @@ jobs:
       if: runner.os == 'Linux'
     - name: Lint
       run: just lint
-      if: matrix.toolchain == '1.88.0'
+      if: matrix.toolchain == '1.91.0'

CHANGELOG.md

@@ -1,3 +1,8 @@
+# v0.1.6
+
+- Bump versions.
+- Improve test suite by fixing multiple drop invocation.
+
 # v0.1.5
 
 - Bump versions.

Development.md

@@ -1,7 +1,6 @@
 # Testing
 
-This needs to be automated in future. But as of now they have to be
-mostly tested manually.
+All of these tests have been automated in [sanity.rs](./pid1/tests/sanity.rs).
 
 There are some programs under the directory `examples`, that will be
 used for testing this library.
@@ -34,8 +33,6 @@ PID   USER     TIME  COMMAND
 
 ## Basic functionality
 
-This is also tested in CI, so you can likely skip this. But here are the steps:
-
 ``` shellsession
 ❯ just test
 ...
@@ -182,7 +179,7 @@ print anything since it is not handled.
 
 ## SIGTERM ignore
 
-This is for testing an application where it ignore SIGTERM and
+This is for testing an application where it ignores SIGTERM and
 continus on doing some work. This library should be able to force kill
 such processes.
 

pid1-exe/Cargo.toml

@@ -22,4 +22,4 @@ clap = { version = "4.5.41", default-features = false, features = [
   "std",
 ] }
 pid1 = { version = "0.1.5", path = "../pid1" }
-signal-hook = "0.3.18"
+signal-hook = "0.4.3"

pid1/Cargo.toml

@@ -13,9 +13,9 @@ categories = ["command-line-utilities"]
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-nix = { version = "0.29.0", features = ["process", "signal"] }
-signal-hook = "0.3.18"
-thiserror = "1"
+nix = { version = "0.31.2", features = ["process", "signal"] }
+signal-hook = "0.4.3"
+thiserror = "2.0.18"
 
 [dev-dependencies]
 rand = "0.8.5"

pid1/examples/simple.rs

@@ -9,19 +9,25 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         .launch()?;
     let id = std::process::id();
     println!("In the simple process, going to sleep. Process ID is {id}");
-    let args = std::env::args().collect::<Vec<_>>();
+    let args: Vec<String> = std::env::args().collect();
     println!("Args: {args:?}");
 
-    for _ in 0..4 {
-        std::thread::sleep(std::time::Duration::from_secs(2));
-        std::process::Command::new("date").spawn().unwrap();
+    if args.iter().any(|arg| arg == "--create-grandchildren") {
+        println!("Spawning 3 grandchildren processes that will be orphaned.");
+        for _ in 0..3 {
+            // These children will be orphaned when `simple` exits, and then
+            // adopted and reaped by pid1.
+            std::process::Command::new("sleep").arg("1").spawn()?;
+        }
     }
 
-    if args.len() > 1 {
-        let duration = &args[2];
-        let duration = duration.parse().expect("Expected int value");
-        println!("Going to sleep {duration} seconds");
-        std::thread::sleep(std::time::Duration::from_secs(duration));
+    if let Some(pos) = args.iter().position(|r| r == "--sleep") {
+        if let Some(duration_str) = args.get(pos + 1) {
+            if let Ok(duration) = duration_str.parse::<u64>() {
+                println!("Going to sleep {duration} seconds");
+                std::thread::sleep(std::time::Duration::from_secs(duration));
+            }
+        }
     }
 
     Ok(())

pid1/examples/zombie.rs

@@ -12,11 +12,15 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             println!("Parent process: going to sleep and exit");
             // We are sleeping so that the child process that exits
             // without being waited upon appears in the process table
-            // as zombie.
-            std::thread::sleep(Duration::from_secs(20));
+            // as zombie. When this parent process exits, the zombie child
+            // will be orphaned and adopted by pid1.
+            std::thread::sleep(Duration::from_secs(1));
             std::process::exit(0);
         }
         Ok(ForkResult::Child) => {
+            let id = std::process::id();
+            println!("Child Process ID is {id}");
+
             std::process::exit(0);
         }
         Err(_) => println!("Fork failed"),

pid1/justfile

@@ -48,4 +48,4 @@ sigloop-test:
 
 # Exec into that docker container
 exec-shell:
-	docker exec -it pid1rs sh
+	docker exec -it pid1rs bash

pid1/src/lib.rs

@@ -141,7 +141,7 @@ fn relaunch() -> Result<Child, Error> {
 /// Graceful exit: We dispatch the singal that got to the application,
 /// followed by SIGTERM and SIGKILL.
 #[cfg(target_family = "unix")]
-fn gracefull_exit(settings: Pid1Settings, signal: c_int, child_pid: i32) -> Result<(), Error> {
+fn graceful_exit(settings: Pid1Settings, signal: c_int, child_pid: i32) -> Result<(), Error> {
     if signal == SIGINT {
         let _ = kill(Pid::from_raw(child_pid), Some(nix::sys::signal::SIGINT));
         std::thread::sleep(settings.timeout);
@@ -177,49 +177,73 @@ fn pid1_handling(settings: Pid1Settings, mut signals: Signals, child: Child) ->
                 // pid1 exits as soon as possible
                 if let ShutdownThreadStatus::NotTriggered = shutdown_thread {
                     shutdown_thread = ShutdownThreadStatus::Triggered;
-                    let _ = std::thread::spawn(move || gracefull_exit(settings, signal, child));
+                    let _ = std::thread::spawn(move || graceful_exit(settings, signal, child));
                 }
                 // We do not exit here since we want the SIGCHLD
                 // handler to be invoked appropriately.
             }
             if signal == SIGCHLD {
-                let child_process_status = match nix::sys::wait::wait().unwrap() {
-                    WaitStatus::Exited(pid, exit_code) => {
-                        let process_status = ProcessStatus { pid, exit_code };
-                        Some(process_status)
-                    }
-                    WaitStatus::Signaled(pid, signal, _) => {
-                        let process_status = ProcessStatus {
-                            pid,
+                let mut main_child_exit_code = None;
+                // Multiple child processes can exit in quick succession, but the
+                // operating system may only deliver a single SIGCHLD signal.
+                // This is known as signal coalescing. To handle this, we loop
+                // with a non-blocking `waitpid` call to reap all zombies.
+                // Using a blocking `wait` would hang if there are no more
+                // children to reap, preventing us from handling other signals.
+                // Reference: https://stackoverflow.com/a/8398491/1651941
+                loop {
+                    let wait_status = match nix::sys::wait::waitpid(
+                        None,
+                        Some(nix::sys::wait::WaitPidFlag::WNOHANG),
+                    ) {
+                        Ok(status) => status,
+                        Err(nix::errno::Errno::ECHILD) => {
+                            // No more children to wait for
+                            break;
+                        }
+                        Err(e) => {
+                            if settings.log {
+                                eprintln!("pid1-rs: Error in waitpid: {e}");
+                            }
+                            break;
+                        }
+                    };
+
+                    let child_process_status = match wait_status {
+                        WaitStatus::Exited(pid, exit_code) => {
+                            Some(ProcessStatus { pid, exit_code })
+                        }
+                        WaitStatus::Signaled(pid, signal, _) => {
                             // Translate signal to exit code
-                            exit_code: signal as i32 + 128,
-                        };
-                        Some(process_status)
-                    }
-                    WaitStatus::Stopped(_, _) => None,
-                    #[cfg(any(target_os = "linux", target_os = "android"))]
-                    WaitStatus::PtraceEvent(_, _, _) => None,
-                    #[cfg(any(target_os = "linux", target_os = "android"))]
-                    WaitStatus::PtraceSyscall(_) => None,
-                    WaitStatus::Continued(_) => None,
-                    WaitStatus::StillAlive => None,
-                };
-                (|| {
-                    let child_process = child_process_status?;
-                    let child_pid = child_process.pid;
-                    let child_pid = child_pid.as_raw();
-                    if child_pid == child {
-                        // At this point, there could be other
-                        // processes running. But once the main
-                        // process dies, everything else is guaranteed
-                        // to die. So we just exit here with the child's exit status code
-                        std::process::exit(child_process.exit_code);
-                    }
-                    if settings.log {
-                        eprintln!("pid1-rs: Reaped PID {child_pid}");
+                            let exit_code = signal as i32 + 128;
+                            Some(ProcessStatus { pid, exit_code })
+                        }
+                        WaitStatus::StillAlive => {
+                            // No more children to reap now
+                            break;
+                        }
+                        WaitStatus::Stopped(..) => None,
+                        #[cfg(any(target_os = "linux", target_os = "android"))]
+                        WaitStatus::PtraceEvent(..) | WaitStatus::PtraceSyscall(..) => None,
+                        WaitStatus::Continued(..) => None,
+                    };
+
+                    if let Some(child_process) = child_process_status {
+                        let child_pid = child_process.pid.as_raw();
+                        if child_pid == child {
+                            // Main child has exited. We'll exit with its status code,
+                            // but only after reaping any other children that may have
+                            // exited in this same signal batch.
+                            main_child_exit_code = Some(child_process.exit_code);
+                        }
+                        if settings.log {
+                            eprintln!("pid1-rs: Reaped PID {child_pid}");
+                        }
                     }
-                    Some(())
-                })();
+                }
+                if let Some(exit_code) = main_child_exit_code {
+                    std::process::exit(exit_code);
+                }
             }
         }
     }