[HOTFIX] Resolve race condition in exit signal

- Corretly boradcast signal to self - Return to SIGUSR1 as internal term coordination
trialspark · Nov 17, 2023 · 57073dd · 57073dd
1 parent e33e3b2
commit 57073dd
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 42 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "bimini"
-version = "0.11.0"
+version = "0.11.1"
 edition = "2021"
 
 [profile.release]

diff --git a/bimini-core/Cargo.lock b/bimini-core/Cargo.lock
diff --git a/bimini-core/Cargo.toml b/bimini-core/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "bimini-core"
-version = "0.11.0"
+version = "0.11.1"
 edition = "2021"
 
 [dependencies]

diff --git a/bimini-core/src/proc/child.rs b/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"),

diff --git a/bimini-core/src/proc/controller.rs b/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<()>>>,
 }
 
@@ -61,11 +53,7 @@ impl Controller {
     }
 
     #[tracing::instrument(skip_all)]
-    fn signal_forwarder(
-        sf_runlock: Arc<AtomicBool>,
-        signal_config: SignalConfig,
-        child_pid: unistd::Pid,
-    ) -> BiminiResult<()> {
+    fn signal_forwarder(signal_config: SignalConfig, child_pid: unistd::Pid) -> BiminiResult<()> {
         tracing::info!("Starting signal forwarding event loop");
 
         signal::pthread_sigmask(
@@ -74,11 +62,15 @@ impl Controller {
             None,
         )?;
 
-        while sf_runlock.load(Ordering::Relaxed) {
+        loop {
             match signal_config.parent_signals().wait() {
-                Ok(signal) if sf_runlock.load(Ordering::Relaxed) => {
-                    tracing::info!("Passing signal to child: {signal}");
+                Ok(signal::Signal::SIGUSR1) => {
+                    tracing::info!("Received termination signal, exiting");
+                    return Ok(());
+                }
 
+                Ok(signal) => {
+                    tracing::info!("Passing Signal({signal}) to Child(pid={child_pid})");
                     signal::kill(child_pid, signal).map_err(|eno| {
                         if eno == errno::Errno::ESRCH {
                             tracing::warn!("Child was dead when forwarding signal");
@@ -87,10 +79,6 @@ impl Controller {
                     })?;
                 }
 
-                Ok(signal) => {
-                    tracing::trace!("Received signal {signal} after loop termination");
-                }
-
                 Err(errno @ errno::Errno::EAGAIN | errno @ errno::Errno::EINTR) => {
                     tracing::info!("Ignoring expected error: {}", errno.desc());
                 }
@@ -101,8 +89,6 @@ impl Controller {
                 }
             }
         }
-
-        Ok(())
     }
 
     #[tracing::instrument(skip_all)]
@@ -115,14 +101,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 +129,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;
                 }
@@ -214,11 +197,8 @@ impl Controller {
             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)?;
+                    tracing::trace!("Sending termination signal to signal_forwarding thread");
+                    signal::kill(unistd::getpid(), signal::SIGUSR1)?;
 
                     tracing::trace!("Joining signal_forwarding thread");
                     signal_forwarder.join()??;

diff --git a/bimini-macros/Cargo.toml b/bimini-macros/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "bimini-macros"
-version = "0.11.0"
+version = "0.11.1"
 edition = "2021"
 
 [lib]