mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-08-03 10:33:49 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Forward additional signals to the child process in uv run (#13017)

Browse source 
As I suspected quite some time ago
(https://github.com/astral-sh/uv/pull/6738#issuecomment-2315466033),
it's problematic that we don't handle _every_ signal here. This PR adds
handling for all of the Unix signals except `SIGCHLD`, `SIGIO`, and
`SIGPOLL` which seem incorrect to forward. Also notable, we _cannot_
handle `SIGKILL` so if someone sends that to the PID instead of the
PGID, they will leave dangling subprocesses.

Instead, we could use `exec` and avoid this handling. However, we'd lose
the ability to add nice error message on failure (e.g., as someone is
trying to add in https://github.com/astral-sh/uv/pull/12201) and, more
critically, we'd need to figure out how to clean up resources properly
(i.e., temporary directories) which currently happens on `Drop`. In the
long-term, we'll probably want an option to use `exec` — but we'll need
to figure out when to clean up resources or accept that they will
dangle. This was last discussed in
https://github.com/astral-sh/uv/issues/3095 — discussion on that
approach should continue there.

A note on the implementation: I spent time time trying to write the
handler using a tokio stream, so we could dynamically iterate over a
list of signals instead of copy/pasting the implementation — I couldn't
get it to work though and it didn't seem critical.

Closes https://github.com/astral-sh/uv/issues/12830
This commit is contained in:
Zanie Blue 2025-04-21 14:47:30 -05:00 • committed by GitHub
parent 2803e6fc5f
commit 8717fbe469
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 144 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

145
crates/uv/src/commands/run.rs
View file

@ -42,7 +42,24 @@ pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result<ExitS
use nix::sys::signal;
use nix::unistd::{getpgid, Pid};
use tokio::select;
use tokio::signal::unix::{signal as handle_signal, SignalKind};
use tokio::signal::unix::{signal as handle_signal, Signal, SignalKind};
/// A wrapper for a signal handler that is not available on all platforms, `recv` never
/// returns on unsupported platforms.
#[allow(dead_code)]
enum PlatformSpecificSignal {
Signal(Signal),
Dummy,
}
impl PlatformSpecificSignal {
async fn recv(&mut self) -> Option<()> {
match self {
PlatformSpecificSignal::Signal(ref mut signal) => signal.recv().await,
PlatformSpecificSignal::Dummy => std::future::pending().await,
}
}
}
/// Simple new type for `Pid` allowing construction from [`Child`].
///
@ -77,6 +94,44 @@ pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result<ExitS
let mut sigint_handle = handle_signal(SignalKind::interrupt())?;
let mut sigint_count = 0;
// The following signals are terminal by default, but can have user defined handlers.
// Forward them to the child process for handling.
let mut sigusr1_handle = handle_signal(SignalKind::user_defined1())?;
let mut sigusr2_handle = handle_signal(SignalKind::user_defined2())?;
let mut sighup_handle = handle_signal(SignalKind::hangup())?;
let mut sigalrm_handle = handle_signal(SignalKind::alarm())?;
let mut sigquit_handle = handle_signal(SignalKind::quit())?;
// The following signals are ignored by default, but can be have user defined handlers.
// Forward them to the child process for handling.
let mut sigwinch_handle = handle_signal(SignalKind::window_change())?;
let mut sigpipe_handle = handle_signal(SignalKind::pipe())?;
// This signal is only available on some platforms, copied from `tokio::signal::unix`
#[cfg(any(
target_os = "dragonfly",
target_os = "freebsd",
target_os = "macos",
target_os = "netbsd",
target_os = "openbsd",
target_os = "illumos",
))]
let mut siginfo_handle = PlatformSpecificSignal::Signal(handle_signal(SignalKind::info())?);
#[cfg(not(any(
target_os = "dragonfly",
target_os = "freebsd",
target_os = "macos",
target_os = "netbsd",
target_os = "openbsd",
target_os = "illumos",
)))]
let mut siginfo_handle = PlatformSpecificSignal::Dummy;
// Notably, we do not handle `SIGCHLD` (sent when a child process status changes) and
// `SIGIO` or `SIGPOLL` (sent when a file descriptor is ready for io) as they don't make
// sense for this parent / child relationship.
loop {
select! {
result = handle.wait() => {
@ -120,6 +175,94 @@ pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result<ExitS
debug!("Received SIGTERM, forwarding to child at {child_pid}");
let _ = signal::kill(child_pid, signal::Signal::SIGTERM);
}
_ = sigusr1_handle.recv() => {
let Some(child_pid) = *ChildPid::from(&handle) else {
debug!("Received SIGUSR1, but the child has already exited");
continue;
};
// We unconditionally forward SIGUSR1 to the child process.
debug!("Received SIGUSR1, forwarding to child at {child_pid}");
let _ = signal::kill(child_pid, signal::Signal::SIGUSR1);
}
_ = sigusr2_handle.recv() => {
let Some(child_pid) = *ChildPid::from(&handle) else {
debug!("Received SIGUSR2, but the child has already exited");
continue;
};
// We unconditionally forward SIGUSR2 to the child process.
debug!("Received SIGUSR2, forwarding to child at {child_pid}");
let _ = signal::kill(child_pid, signal::Signal::SIGUSR2);
}
_ = sighup_handle.recv() => {
let Some(child_pid) = *ChildPid::from(&handle) else {
debug!("Received SIGHUP, but the child has already exited");
continue;
};
// We unconditionally forward SIGHUP to the child process.
debug!("Received SIGHUP, forwarding to child at {child_pid}");
let _ = signal::kill(child_pid, signal::Signal::SIGHUP);
}
_ = sigalrm_handle.recv() => {
let Some(child_pid) = *ChildPid::from(&handle) else {
debug!("Received SIGALRM, but the child has already exited");
continue;
};
// We unconditionally forward SIGALRM to the child process.
debug!("Received SIGALRM, forwarding to child at {child_pid}");
let _ = signal::kill(child_pid, signal::Signal::SIGALRM);
}
_ = sigquit_handle.recv() => {
let Some(child_pid) = *ChildPid::from(&handle) else {
debug!("Received SIGQUIT, but the child has already exited");
continue;
};
// We unconditionally forward SIGQUIT to the child process.
debug!("Received SIGQUIT, forwarding to child at {child_pid}");
let _ = signal::kill(child_pid, signal::Signal::SIGQUIT);
}
_ = sigwinch_handle.recv() => {
let Some(child_pid) = *ChildPid::from(&handle) else {
debug!("Received SIGWINCH, but the child has already exited");
continue;
};
// We unconditionally forward SIGWINCH to the child process.
debug!("Received SIGWINCH, forwarding to child at {child_pid}");
let _ = signal::kill(child_pid, signal::Signal::SIGWINCH);
}
_ = sigpipe_handle.recv() => {
let Some(child_pid) = *ChildPid::from(&handle) else {
debug!("Received SIGPIPE, but the child has already exited");
continue;
};
// We unconditionally forward SIGPIPE to the child process.
debug!("Received SIGPIPE, forwarding to child at {child_pid}");
let _ = signal::kill(child_pid, signal::Signal::SIGPIPE);
}
_ = siginfo_handle.recv() => {
let Some(child_pid) = *ChildPid::from(&handle) else {
debug!("Received SIGINFO, but the child has already exited");
continue;
};
// We unconditionally forward SIGINFO to the child process.
debug!("Received SIGINFO, forwarding to child at {child_pid}");
#[cfg(any(
target_os = "dragonfly",
target_os = "freebsd",
target_os = "macos",
target_os = "netbsd",
target_os = "openbsd",
target_os = "illumos",
))]
let _ = signal::kill(child_pid, signal::Signal::SIGINFO);
}
};
}
}?;