Skip to content

Commit

Permalink
[HOTFIX] Resolve race condition in exit signal
Browse files Browse the repository at this point in the history
- Verify child lively-ness before forward
- Swaps to SeqCst atomic access
  • Loading branch information
zellio committed Nov 17, 2023
1 parent 67118f6 commit 9d41434
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 11 deletions.
2 changes: 1 addition & 1 deletion bimini-core/src/proc/child.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
20 changes: 10 additions & 10 deletions bimini-core/src/proc/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,10 @@ impl Controller {
None,
)?;

while sf_runlock.load(Ordering::Relaxed) {
while sf_runlock.load(Ordering::SeqCst) {
match signal_config.parent_signals().wait() {
Ok(signal) if sf_runlock.load(Ordering::Relaxed) => {
tracing::info!("Passing signal to child: {signal}");

Ok(signal) if unistd::getpgid(Some(child_pid)).is_ok() => {
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");
Expand All @@ -88,7 +87,8 @@ impl Controller {
}

Ok(signal) => {
tracing::trace!("Received signal {signal} after loop termination");
tracing::warn!("Child(pid={child_pid}) is dead, dropping Signal({signal})");
return Ok(());
}

Err(errno @ errno::Errno::EAGAIN | errno @ errno::Errno::EINTR) => {
Expand All @@ -115,7 +115,7 @@ impl Controller {
));
}

self.sf_runlock.store(true, Ordering::Release);
self.sf_runlock.store(true, Ordering::SeqCst);

let runlock_clone = Arc::clone(&self.sf_runlock);
let signal_config = self.signal_config.clone();
Expand Down Expand Up @@ -146,7 +146,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;
}
Expand Down Expand Up @@ -215,10 +215,10 @@ impl Controller {
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);
self.sf_runlock.swap(false, Ordering::SeqCst);

tracing::trace!("Sending final SIGTERM to signal_forwarding thread");
signal::kill(unistd::Pid::from_raw(0), signal::SIGCHLD)?;
tracing::trace!("Sending final SIGCHLD to signal_forwarding thread");
signal::kill(unistd::getpid(), signal::SIGCHLD)?;

tracing::trace!("Joining signal_forwarding thread");
signal_forwarder.join()??;
Expand Down

0 comments on commit 9d41434

Please sign in to comment.