Fix potential race condition (#11)

Install signal handlers before launching the child process

.github/workflows/rust.yml

@@ -2,7 +2,7 @@ name: Rust
 
 on:
   push:
-    branches: [main]
+    branches: [master]
     tags:
       - '*' # Push events to matching v*, i.e. v1.0, v20.15.10
   pull_request:

Development.md

@@ -208,5 +208,8 @@ You will see that it would have exited:
 ``` shellsession
 App got SIGTERM 15, but will not exit
 App got SIGTERM 15, but will not exit
-error: Recipe `sigloop-test` failed on line 44 with exit code 143
+error: Recipe `sigloop-test` failed on line 44 with exit code 137
 ```
+
+You can confirm from the status code that the child process got killed
+by (137 - 128 = 9) SIGKIL as the application ignores SIGTERM.

README.md

@@ -7,17 +7,17 @@ process.
 
 This repository consists of two packages:
 - [pid1](./pid1/) crate: Library meant to be used by your Rust applications.
-- [pid1-exe](./pid1-exe) crate: Binary which internally uses pid1 crate used for
-  container deployments. The binary name is `pid1`.
+- [pid1-exe](./pid1-exe) crate: Binary which internally uses pid1
+  crate for container deployments. The binary name is `pid1`.
 
 ## pid1 Library Usage
 
 This library is used to simplify Rust deployment in a containerized
-environment. Instead of using something like [Haskell pid1](https://github.com/fpco/pid1-rs/actions/workflows/rust.yml)) or
-[tini](https://github.com/krallin/tini) binary in your container, you can directly use this crate.
+environment. Instead of using binaries like [Haskell's pid1](https://github.com/fpco/pid1-rs/actions/workflows/rust.yml)) or
+[tini](https://github.com/krallin/tini) in your container, you can use this crate directly.
 
-You need to ensure that the method `launch` should be the
-initial statement within your `main` function:
+You must ensure that the `launch` method is the first statement in
+your `main` function:
 
 ``` rust
 use std::time::Duration;
@@ -34,8 +34,9 @@ fn main() {
 }
 ```
 
-You can also see various example usages [here.](./examples/) This function is
-meant only for Unix systems and is a no-op in Windows.
+You can also see various example usages [here.](./examples/) This
+function is meant only for Unix systems and the above code is a no-op
+in Windows.
 
 ## Using pid1 binary
 

pid1-exe/Cargo.toml

@@ -8,6 +8,7 @@ readme = "../README.md"
 
 [dependencies]
 pid1 = { path = "../pid1"}
+signal-hook = "0.3.17"
 clap = { version = "4.4.6", default-features = false, features = ["std", "derive", "help"]}
 
 [[bin]]

pid1-exe/src/cli.rs

@@ -1,9 +1,16 @@
 use clap::Parser;
 #[cfg(target_family = "unix")]
-use std::os::unix::process::CommandExt;
-use std::{error::Error, ffi::OsString, path::PathBuf, time::Duration};
-
 use pid1::Pid1Settings;
+#[cfg(target_family = "unix")]
+use signal_hook::{
+    consts::{SIGCHLD, SIGINT, SIGTERM},
+    iterator::Signals,
+};
+#[cfg(target_family = "unix")]
+use std::os::unix::process::CommandExt;
+#[cfg(target_family = "unix")]
+use std::time::Duration;
+use std::{error::Error, ffi::OsString, path::PathBuf};
 
 #[derive(Parser, Debug, PartialEq)]
 pub(crate) struct Pid1App {
@@ -25,7 +32,8 @@ pub(crate) struct Pid1App {
 }
 
 impl Pid1App {
-    pub(crate) fn run(self) {
+    #[cfg(target_family = "unix")]
+    pub(crate) fn run(self) -> ! {
         let mut child = std::process::Command::new(&self.child_process[0]);
         let child = child.args(&self.child_process[1..]);
         if let Some(workdir) = &self.workdir {
@@ -36,16 +44,13 @@ impl Pid1App {
         }
         let pid = std::process::id();
         if pid != 1 {
-            #[cfg(target_family = "unix")]
-            {
-                let status = child.exec();
-                eprintln!("execvp failed with: {status:?}");
-            }
-            #[cfg(target_family = "windows")]
-            eprintln!("execvp not supported on windows");
+            let status = child.exec();
+            eprintln!("execvp failed with: {status:?}");
 
             std::process::exit(1);
         } else {
+            // Install signal handlers before launching child process
+            let signals = Signals::new([SIGTERM, SIGINT, SIGCHLD]).unwrap();
             let child = child.spawn();
             let child = match child {
                 Ok(child) => child,
@@ -61,9 +66,15 @@ impl Pid1App {
             Pid1Settings::new()
                 .enable_log(self.verbose)
                 .timeout(Duration::from_secs(self.timeout.into()))
-                .pid1_handling(child)
+                .pid1_handling(signals, child)
         }
     }
+
+    #[cfg(target_family = "windows")]
+    pub(crate) fn run(self) -> ! {
+        eprintln!("pid1: Not supported on Windows");
+        std::process::exit(1);
+    }
 }
 
 /// Parse a single key-value pair

pid1/src/lib.rs

@@ -10,6 +10,7 @@ use signal_hook::{
 };
 #[cfg(target_family = "unix")]
 use std::ffi::c_int;
+#[cfg(target_family = "unix")]
 use std::process::Child;
 use std::time::Duration;
 
@@ -91,11 +92,13 @@ impl Pid1Settings {
     pub fn launch(self) -> Result<(), Error> {
         let pid = std::process::id();
         if pid == 1 {
+            // Install signal handles before we launch child process
+            let signals = Signals::new([SIGTERM, SIGINT, SIGCHLD]).unwrap();
             let child = relaunch()?;
             if self.log {
                 eprintln!("pid1-rs: Process running as PID 1");
             }
-            pid1_handling(self, child)
+            pid1_handling(self, signals, child)
         } else {
             if self.log {
                 eprintln!("pid1-rs: Process not running as Pid 1: PID {pid}");
@@ -112,19 +115,10 @@ impl Pid1Settings {
     }
 
     /// Do proper reaping and signal handling on the [`Child`]
-    /// process. This is only applicable for Unix systems. For
-    /// Windows, this will terminate the current process with exit
-    /// code 1.
+    /// process. This method is only available for Unix systems.
     #[cfg(target_family = "unix")]
-    pub fn pid1_handling(self, child: Child) -> ! {
-        pid1_handling(self, child)
-    }
-    #[cfg(target_family = "windows")]
-    pub fn pid1_handling(self, _child: Child) -> ! {
-        if self.log {
-            eprintln!("pid1-rs: PID1 capability not supported for Windows");
-        }
-        std::process::exit(1);
+    pub fn pid1_handling(self, signals: Signals, child: Child) -> ! {
+        pid1_handling(self, signals, child)
     }
 }
 
@@ -165,8 +159,7 @@ fn gracefull_exit(settings: Pid1Settings, signal: c_int, child_pid: i32) -> Resu
 }
 
 #[cfg(target_family = "unix")]
-fn pid1_handling(settings: Pid1Settings, child: Child) -> ! {
-    let mut signals = Signals::new([SIGTERM, SIGINT, SIGCHLD]).unwrap();
+fn pid1_handling(settings: Pid1Settings, mut signals: Signals, child: Child) -> ! {
     let child = child.id() as i32;
     struct ProcessStatus {
         pid: Pid,