[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,40 @@
-use nix::sys::signal;
-
 use crate::error::BiminiResult;
+use nix::{errno, sys::signal};
 
-#[derive(Clone, Debug)]
+#[derive(Debug, Clone)]
 pub struct SignalConfig {
-    parent_signals: signal::SigSet,
-    source_signals: signal::SigSet,
+    mask_set: signal::SigSet,
+    source_set: 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
+impl Default for SignalConfig {
+    fn default() -> Self {
+        Self::new(&[
+            // Un-blockable signals
+            signal::SIGKILL,
+            signal::SIGSTOP,
+            // Program signals
+            signal::SIGABRT,
+            signal::SIGBUS,
+            #[cfg(not(target_os = "linux"))]
+            signal::SIGEMT,
+            signal::SIGFPE,
+            signal::SIGILL,
+            signal::SIGIOT,
+            signal::SIGSEGV,
+            signal::SIGSYS,
+            signal::SIGTRAP,
+        ])
     }
+}
 
-    pub fn new(protected_signals: Vec<signal::Signal>) -> Self {
-        let mut parent_signals = signal::SigSet::all();
-        for signal in protected_signals {
-            parent_signals.remove(signal)
+impl SignalConfig {
+    pub fn new(protected_signals: &[signal::Signal]) -> Self {
+        let mut mask = signal::SigSet::all();
+        for sig in protected_signals {
+            mask.remove(*sig);
         }
 
         let ignore_action = signal::SigAction::new(
@@ -39,59 +43,49 @@ impl SignalConfig {
             signal::SigSet::empty(),
         );
 
-        SignalConfig {
-            parent_signals,
-            source_signals: signal::SigSet::empty(),
+        Self {
+            mask_set: mask,
+            source_set: signal::SigSet::empty(),
             sigttin_action: ignore_action,
             sigttou_action: ignore_action,
         }
     }
 
-    pub fn mask(&mut self) -> BiminiResult<()> {
-        signal::sigprocmask(
+    pub fn mask(&mut self) -> BiminiResult<&mut Self> {
+        signal::pthread_sigmask(
             signal::SigmaskHow::SIG_SETMASK,
-            Some(&self.parent_signals),
-            Some(&mut self.source_signals),
+            Some(&self.mask_set),
+            Some(&mut self.source_set),
         )?;
 
         unsafe {
             self.sigttin_action = signal::sigaction(signal::SIGTTIN, &self.sigttin_action)?;
             self.sigttou_action = signal::sigaction(signal::SIGTTOU, &self.sigttou_action)?;
         }
 
-        Ok(())
+        Ok(self)
     }
 
-    pub fn unmask(&mut self) -> BiminiResult<()> {
-        signal::sigprocmask(
+    pub fn unmask(&mut self) -> BiminiResult<&mut Self> {
+        signal::pthread_sigmask(
             signal::SigmaskHow::SIG_SETMASK,
-            Some(&self.source_signals),
+            Some(&self.source_set),
             None,
         )?;
 
-        self.source_signals = signal::SigSet::empty();
-
         unsafe {
             signal::sigaction(signal::SIGTTIN, &self.sigttin_action)?;
             signal::sigaction(signal::SIGTTOU, &self.sigttou_action)?;
         }
 
-        Ok(())
+        Ok(self)
     }
-}
 
-impl Default for SignalConfig {
-    fn default() -> Self {
-        SignalConfig::new(vec![
-            signal::SIGFPE,
-            signal::SIGILL,
-            signal::SIGSEGV,
-            signal::SIGBUS,
-            signal::SIGABRT,
-            signal::SIGTRAP,
-            signal::SIGSYS,
-            signal::SIGTTIN,
-            signal::SIGTTOU,
-        ])
+    pub fn mask_wait(&self) -> Result<signal::Signal, errno::Errno> {
+        self.mask_set.wait()
+    }
+
+    pub fn source_wait(&self) -> Result<signal::Signal, errno::Errno> {
+        self.source_set.wait()
     }
 }

bimini-core/src/proc/child.rs

@@ -82,7 +82,10 @@ impl<'a> Child<'a> {
             proc.envs(spawn_directory.to_env());
         }
 
+        tracing::error!("1");
         self.isolate()?;
+
+        tracing::error!("2");
         self.signal_config.unmask()?;
 
         let error = proc.exec();
@@ -101,7 +104,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,40 @@ 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,
-        )?;
+        loop {
+            let signal = signal_config.mask_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 => {
+                                tracing::error!("Received un-handled error passing signal to child: {errno}");
+                                return Err(BiminiError::Errno(errno))
+                            },
+                        }
+                    }
                 }
 
                 Err(errno @ errno::Errno::EAGAIN | errno @ errno::Errno::EINTR) => {
@@ -101,8 +100,6 @@ impl Controller {
                 }
             }
         }
-
-        Ok(())
     }
 
     #[tracing::instrument(skip_all)]
@@ -115,14 +112,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 +140,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 +202,29 @@ 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);
+            if rc == -1 {
+                continue;
+            }
 
-                    tracing::trace!("Sending final SIGTERM to signal_forwarding thread");
-                    signal::kill(unistd::Pid::from_raw(0), signal::SIGTERM)?;
+            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");
 
-                    tracing::trace!("Joining signal_forwarding thread");
-                    signal_forwarder.join()??;
+                let mut sigset = signal::SigSet::empty();
+                signal::pthread_sigmask(signal::SigmaskHow::SIG_SETMASK, None, Some(&mut sigset))?;
+                for sig in &sigset {
+                    tracing::error!("{sig}");
                 }
 
-                return Ok(ExitCode::from(TryInto::<u8>::try_into(rc)?));
+                signal::kill(unistd::getpid(), signal::SIGUSR1)?;
+
+                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]