[HOTFIX] Resolve race condition in exit signal

- Reinstates SIGUSR1 coordination
- Corrects signal dispatch logic
- Simplifies SignalConfig

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "bimini"
-version = "0.11.0"
+version = "0.11.1"
 edition = "2021"
 
 [profile.release]

bimini-core/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "bimini-core"
-version = "0.11.0"
+version = "0.11.1"
 edition = "2021"
 
 [dependencies]

bimini-core/src/nix/signal_config.rs

@@ -1,36 +1,19 @@
-use nix::sys::signal;
+use nix::{sys::signal, Result as NixResult};
 
 use crate::error::BiminiResult;
 
 #[derive(Clone, Debug)]
 pub struct SignalConfig {
-    parent_signals: signal::SigSet,
-    source_signals: signal::SigSet,
+    sigset: signal::SigSet,
     sigttin_action: signal::SigAction,
     sigttou_action: signal::SigAction,
 }
 
 impl SignalConfig {
-    pub fn parent_signals(&self) -> &signal::SigSet {
-        &self.parent_signals
-    }
-
-    pub fn source_signals(&self) -> &signal::SigSet {
-        &self.source_signals
-    }
-
-    pub fn sigttin_action(&self) -> &signal::SigAction {
-        &self.sigttin_action
-    }
-
-    pub fn sigttou_action(&self) -> &signal::SigAction {
-        &self.sigttou_action
-    }
-
     pub fn new(protected_signals: Vec<signal::Signal>) -> Self {
-        let mut parent_signals = signal::SigSet::all();
+        let mut sigset = signal::SigSet::all();
         for signal in protected_signals {
-            parent_signals.remove(signal)
+            sigset.remove(signal)
         }
 
         let ignore_action = signal::SigAction::new(
@@ -40,19 +23,14 @@ impl SignalConfig {
         );
 
         SignalConfig {
-            parent_signals,
-            source_signals: signal::SigSet::empty(),
+            sigset,
             sigttin_action: ignore_action,
             sigttou_action: ignore_action,
         }
     }
 
     pub fn mask(&mut self) -> BiminiResult<()> {
-        signal::sigprocmask(
-            signal::SigmaskHow::SIG_SETMASK,
-            Some(&self.parent_signals),
-            Some(&mut self.source_signals),
-        )?;
+        self.thread_swap_mask()?;
 
         unsafe {
             self.sigttin_action = signal::sigaction(signal::SIGTTIN, &self.sigttin_action)?;
@@ -63,13 +41,7 @@ impl SignalConfig {
     }
 
     pub fn unmask(&mut self) -> BiminiResult<()> {
-        signal::sigprocmask(
-            signal::SigmaskHow::SIG_SETMASK,
-            Some(&self.source_signals),
-            None,
-        )?;
-
-        self.source_signals = signal::SigSet::empty();
+        self.thread_swap_mask()?;
 
         unsafe {
             signal::sigaction(signal::SIGTTIN, &self.sigttin_action)?;
@@ -78,6 +50,18 @@ impl SignalConfig {
 
         Ok(())
     }
+
+    pub fn thread_swap_mask(&mut self) -> BiminiResult<()> {
+        self.sigset = self
+            .sigset
+            .thread_swap_mask(signal::SigmaskHow::SIG_SETMASK)?;
+
+        Ok(())
+    }
+
+    pub fn wait(&self) -> NixResult<signal::Signal> {
+        self.sigset.wait()
+    }
 }
 
 impl Default for SignalConfig {

bimini-core/src/proc/child.rs

@@ -101,7 +101,7 @@ impl<'a> ToEnv for Child<'a> {
             (String::from("BIMINI"), String::from("true")),
             (
                 String::from("BIMINI_VERSION"),
-                String::from(env!("CARGO_PKG_VERSION")),
+                String::from(crate::PKG_VERSION),
             ),
             (
                 String::from("BIMINI_CHILD_PID"),

bimini-core/src/proc/controller.rs

@@ -8,20 +8,12 @@ use nix::{
     sys::{signal, wait},
     unistd,
 };
-use std::{
-    process::ExitCode,
-    sync::{
-        atomic::{AtomicBool, Ordering},
-        Arc,
-    },
-    thread,
-};
+use std::{process::ExitCode, thread};
 
 #[derive(Debug, Default)]
 pub struct Controller {
     signal_config: SignalConfig,
     child_pid: Option<unistd::Pid>,
-    sf_runlock: Arc<AtomicBool>,
     signal_forwarder: Option<std::thread::JoinHandle<BiminiResult<()>>>,
 }
 
@@ -62,33 +54,39 @@ impl Controller {
 
     #[tracing::instrument(skip_all)]
     fn signal_forwarder(
-        sf_runlock: Arc<AtomicBool>,
-        signal_config: SignalConfig,
+        mut signal_config: SignalConfig,
         child_pid: unistd::Pid,
     ) -> BiminiResult<()> {
         tracing::info!("Starting signal forwarding event loop");
 
-        signal::pthread_sigmask(
-            signal::SigmaskHow::SIG_SETMASK,
-            Some(signal_config.source_signals()),
-            None,
-        )?;
+        signal_config.thread_swap_mask()?;
+
+        loop {
+            let signal = signal_config.wait();
 
-        while sf_runlock.load(Ordering::Relaxed) {
-            match signal_config.parent_signals().wait() {
-                Ok(signal) if sf_runlock.load(Ordering::Relaxed) => {
-                    tracing::info!("Passing signal to child: {signal}");
+            tracing::info!("Received Signal({signal:?}), dispatching");
 
-                    signal::kill(child_pid, signal).map_err(|eno| {
-                        if eno == errno::Errno::ESRCH {
-                            tracing::warn!("Child was dead when forwarding signal");
-                        }
-                        eno
-                    })?;
+            match signal {
+                Ok(signal::Signal::SIGCHLD) => {
+                    tracing::info!("Received SIGCHLD, suppressing.");
+                }
+
+                Ok(signal::Signal::SIGUSR1) => {
+                    tracing::info!("Received SIGUSR1, terminating signal_forwarder loop.");
+                    return Ok(());
                 }
 
                 Ok(signal) => {
-                    tracing::trace!("Received signal {signal} after loop termination");
+                    tracing::info!("Passing Signal({signal}) to Child(pid={child_pid})");
+
+                    if let Err(errno) = signal::kill(child_pid, signal) {
+                        match errno {
+                            errno::Errno::ESRCH => tracing::warn!(
+                                "Child(pid={child_pid}) was dead when forwarding, Signal({signal}) dropped."
+                            ),
+                            errno => return Err(BiminiError::Errno(errno)),
+                        }
+                    }
                 }
 
                 Err(errno @ errno::Errno::EAGAIN | errno @ errno::Errno::EINTR) => {
@@ -101,8 +99,6 @@ impl Controller {
                 }
             }
         }
-
-        Ok(())
     }
 
     #[tracing::instrument(skip_all)]
@@ -115,14 +111,11 @@ impl Controller {
             ));
         }
 
-        self.sf_runlock.store(true, Ordering::Release);
-
-        let runlock_clone = Arc::clone(&self.sf_runlock);
         let signal_config = self.signal_config.clone();
         let child_pid = self.child_pid.unwrap();
 
         self.signal_forwarder = Some(thread::spawn(move || {
-            Controller::signal_forwarder(runlock_clone, signal_config, child_pid)
+            Controller::signal_forwarder(signal_config, child_pid)
         }));
 
         Ok(self)
@@ -146,7 +139,7 @@ impl Controller {
             match wait::waitpid(any_proc, None) {
                 Ok(wait::WaitStatus::Exited(pid, status)) if pid == child_pid => {
                     tracing::info!(
-                        "Controller child process {pid} exited normally with status {status}."
+                        "Controller child process Child(pid={pid}) exited normally with status {status}."
                     );
                     child_exitcode = status;
                 }
@@ -208,24 +201,22 @@ impl Controller {
     pub fn run_reaper(self) -> BiminiResult<ExitCode> {
         loop {
             tracing::debug!("Running zombie reaper loop.");
-
             let rc = self.reap_zombies()?;
 
-            if rc != -1 {
-                tracing::trace!("Received child status code: {rc}. Cleaning up");
-                if let Some(signal_forwarder) = self.signal_forwarder {
-                    tracing::trace!("Releasing signal_forwarding loop runlock");
-                    self.sf_runlock.store(false, Ordering::Release);
-
-                    tracing::trace!("Sending final SIGTERM to signal_forwarding thread");
-                    signal::kill(unistd::Pid::from_raw(0), signal::SIGTERM)?;
+            if rc == -1 {
+                continue;
+            }
 
-                    tracing::trace!("Joining signal_forwarding thread");
-                    signal_forwarder.join()??;
-                }
+            tracing::trace!("Received child status code: {rc}. Cleaning up");
+            if let Some(signal_forwarder) = self.signal_forwarder {
+                tracing::trace!("Sending termination signal to signal_forwarding thread");
+                signal::kill(unistd::getpid(), signal::SIGUSR1)?;
 
-                return Ok(ExitCode::from(TryInto::<u8>::try_into(rc)?));
+                tracing::trace!("Joining signal_forwarding thread");
+                signal_forwarder.join()??;
             }
+
+            return Ok(ExitCode::from(TryInto::<u8>::try_into(rc)?));
         }
     }
 }

bimini-macros/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "bimini-macros"
-version = "0.11.0"
+version = "0.11.1"
 edition = "2021"
 
 [lib]