diff --git a/src/exec/no_pty.rs b/src/exec/no_pty.rs index 68ff6543d..87f28905d 100644 --- a/src/exec/no_pty.rs +++ b/src/exec/no_pty.rs @@ -14,7 +14,7 @@ use crate::{ }, }; use crate::{ - exec::{handle_sigchld, opt_fmt, signal_fmt}, + exec::{handle_sigchld, signal_fmt}, log::{dev_error, dev_info, dev_warn}, system::{ fork, getpgid, getpgrp, @@ -237,12 +237,7 @@ impl ExecClosure { } }; - dev_info!( - "received{} {} from {}", - opt_fmt(info.is_user_signaled(), " user signaled"), - info.signal(), - info.pid() - ); + dev_info!("received{}", info); let Some(command_pid) = self.command_pid else { dev_info!("command was terminated, ignoring signal"); @@ -255,9 +250,11 @@ impl ExecClosure { // FIXME: we should handle SIGWINCH here if we want to support I/O plugins that // react on window change events. - // Skip the signal if it was sent by the user and it is self-terminating. - if info.is_user_signaled() && self.is_self_terminating(info.pid()) { - return; + if let Some(pid) = info.signaler_pid() { + if self.is_self_terminating(pid) { + // Skip the signal if it was sent by the user and it is self-terminating. + return; + } } if signal == SIGALRM { diff --git a/src/exec/use_pty/monitor.rs b/src/exec/use_pty/monitor.rs index 057247346..14bb8a395 100644 --- a/src/exec/use_pty/monitor.rs +++ b/src/exec/use_pty/monitor.rs @@ -380,12 +380,7 @@ impl<'a> MonitorClosure<'a> { } }; - dev_info!( - "monitor received{} {} from {}", - opt_fmt(info.is_user_signaled(), " user signaled"), - info.signal(), - info.pid() - ); + dev_info!("monitor received{}", info); // Don't do anything if the command has terminated already let Some(command_pid) = self.command_pid else { @@ -395,10 +390,16 @@ impl<'a> MonitorClosure<'a> { match info.signal() { SIGCHLD => handle_sigchld(self, registry, "command", command_pid), - // Skip the signal if it was sent by the user and it is self-terminating. - _ if info.is_user_signaled() - && is_self_terminating(info.pid(), command_pid, self.command_pgrp) => {} - signal => self.send_signal(signal, command_pid, false), + signal => { + if let Some(pid) = info.signaler_pid() { + if is_self_terminating(pid, command_pid, self.command_pgrp) { + // Skip the signal if it was sent by the user and it is self-terminating. + return; + } + } + + self.send_signal(signal, command_pid, false) + } } } } diff --git a/src/exec/use_pty/parent.rs b/src/exec/use_pty/parent.rs index c5ae13be9..05a54b189 100644 --- a/src/exec/use_pty/parent.rs +++ b/src/exec/use_pty/parent.rs @@ -7,7 +7,7 @@ use crate::exec::event::{EventHandle, EventRegistry, PollEvent, Process, StopRea use crate::exec::use_pty::monitor::exec_monitor; use crate::exec::use_pty::SIGCONT_FG; use crate::exec::{ - cond_fmt, handle_sigchld, opt_fmt, signal_fmt, terminate_process, ExecOutput, HandleSigchld, + cond_fmt, handle_sigchld, signal_fmt, terminate_process, ExecOutput, HandleSigchld, ProcessOutput, }; use crate::exec::{ @@ -616,12 +616,7 @@ impl ParentClosure { } }; - dev_info!( - "parent received{} {} from {}", - opt_fmt(info.is_user_signaled(), " user signaled"), - info.signal(), - info.pid() - ); + dev_info!("parent received{}", info); let Some(monitor_pid) = self.monitor_pid else { dev_info!("monitor was terminated, ignoring signal"); @@ -638,10 +633,17 @@ impl ParentClosure { dev_warn!("cannot resize terminal: {}", err); } } - // Skip the signal if it was sent by the user and it is self-terminating. - _ if info.is_user_signaled() && self.is_self_terminating(info.pid()) => {} - // FIXME: check `send_command_status` - signal => self.schedule_signal(signal, registry), + signal => { + if let Some(pid) = info.signaler_pid() { + if self.is_self_terminating(pid) { + // Skip the signal if it was sent by the user and it is self-terminating. + return; + } + } + + // FIXME: check `send_command_status` + self.schedule_signal(signal, registry) + } } } diff --git a/src/system/signal/info.rs b/src/system/signal/info.rs index 667762638..26c3a937c 100644 --- a/src/system/signal/info.rs +++ b/src/system/signal/info.rs @@ -1,3 +1,5 @@ +use std::fmt; + use crate::system::interface::ProcessId; use super::SignalNumber; @@ -13,20 +15,18 @@ impl SignalInfo { /// Returns whether the signal was sent by the user or not. pub(crate) fn is_user_signaled(&self) -> bool { - // FIXME: we should check if si_code is equal to SI_USER but for some reason the latter it - // is not available in libc. + // This matches the definition of the SI_FROMUSER macro. self.info.si_code <= 0 } /// Gets the PID that sent the signal. - pub(crate) fn pid(&self) -> ProcessId { - // FIXME: some signals don't set si_pid. - // - // SAFETY: this just fetches the `si_pid` field; since this is an integer, - // even if the information is nonsense it will not cause UB. Note that - // that a `ProcessId` does not have as type invariant that it always holds a valid - // process id, only that it is the appropriate type for storing such ids. - unsafe { self.info.si_pid() } + pub(crate) fn signaler_pid(&self) -> Option { + if self.is_user_signaled() { + // SAFETY: si_pid is always initialized if the signal is user signaled. + unsafe { Some(self.info.si_pid()) } + } else { + None + } } /// Gets the signal number. @@ -34,3 +34,23 @@ impl SignalInfo { self.info.si_signo } } + +impl fmt::Display for SignalInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{} {} from ", + if self.is_user_signaled() { + " user signaled" + } else { + "" + }, + self.signal(), + )?; + if let Some(pid) = self.signaler_pid() { + write!(f, "{pid}") + } else { + write!(f, "") + } + } +}