From 57073dd5b0bb00a811277d6f2f0a117cdb60efbb Mon Sep 17 00:00:00 2001 From: Zachary Elliott Date: Fri, 17 Nov 2023 09:00:16 -0500 Subject: [PATCH] [HOTFIX] Resolve race condition in exit signal - Corretly boradcast signal to self - Return to SIGUSR1 as internal term coordination --- Cargo.lock | 6 ++-- Cargo.toml | 2 +- bimini-core/Cargo.lock | 4 +-- bimini-core/Cargo.toml | 2 +- bimini-core/src/proc/child.rs | 2 +- bimini-core/src/proc/controller.rs | 46 +++++++++--------------------- bimini-macros/Cargo.toml | 2 +- 7 files changed, 22 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8d85123..25fd3aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -166,7 +166,7 @@ dependencies = [ [[package]] name = "bimini" -version = "0.11.0" +version = "0.11.1" dependencies = [ "bimini-core", "clap", @@ -179,7 +179,7 @@ dependencies = [ [[package]] name = "bimini-core" -version = "0.11.0" +version = "0.11.1" dependencies = [ "aws-sigv4", "base64", @@ -198,7 +198,7 @@ dependencies = [ [[package]] name = "bimini-macros" -version = "0.11.0" +version = "0.11.1" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 6204c13..804cefb 100644 --- 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 index f0f2e23..78fb053 100644 --- a/bimini-core/Cargo.lock +++ b/bimini-core/Cargo.lock @@ -118,7 +118,7 @@ dependencies = [ [[package]] name = "bimini-core" -version = "0.11.0" +version = "0.11.1" dependencies = [ "aws-sigv4", "base64", @@ -137,7 +137,7 @@ dependencies = [ [[package]] name = "bimini-macros" -version = "0.11.0" +version = "0.11.1" dependencies = [ "proc-macro2", "quote", diff --git a/bimini-core/Cargo.toml b/bimini-core/Cargo.toml index e8d459e..048d967 100644 --- 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 index 0ecdbb7..4d07e4c 100644 --- 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 index fd8c578..9d6d355 100644 --- 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, - sf_runlock: Arc, signal_forwarder: Option>>, } @@ -61,11 +53,7 @@ impl Controller { } #[tracing::instrument(skip_all)] - fn signal_forwarder( - sf_runlock: Arc, - 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 index d6e8c63..801efef 100644 --- 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]