Python
/
uv
mirror of https://github.com/astral-sh/uv
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Use TTY detection to determine if SIGINT forwarding is enabled (#13925) 

Use TTY detection to determine when we should forward SIGINT instead of
counting signals, which can lead to various problems where multiple
SIGINTs are sent to a child after the first signal. Counting does not
make sense in interactive situations that do not exit on interrupt,
e.g., the Python REPL.

Closes https://github.com/astral-sh/uv/issues/13919
Closes https://github.com/astral-sh/uv/issues/12108
This commit is contained in:
Zanie Blue 2025-06-11 08:28:34 -05:00 committed by GitHub
parent 357fc91c3c
commit f530565323
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 13 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
crates/uv/src/commands/run.rs
View File

@ -17,12 +17,11 @@ pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result<ExitS
// its process group, in which case the terminal driver will _not_ send SIGINT to the child // its process group, in which case the terminal driver will _not_ send SIGINT to the child
// process and uv must take ownership of forwarding the signal. // process and uv must take ownership of forwarding the signal.
// //
// Note this assumes an interactive terminal. If a signal is sent directly to the uv parent // Note the above only applies in an interactive terminal. If a signal is sent directly to the
// process (e.g., `kill -2 <pid>`), the process group is not involved and a signal is not sent // uv parent process (e.g., `kill -2 <pid>`), the process group is not involved and a signal is
// to the child by default. In this context, uv must forward the signal to the child. We work // not sent to the child by default. In this context, uv must forward the signal to the child.
// around this by forwarding SIGINT if it is received more than once. We could attempt to infer // uv checks if stdin is a TTY as a heuristic to determine if uv is running in an interactive
// if the parent is a terminal using TTY detection(?), but there hasn't been sufficient // terminal. When not in an interactive terminal, uv will forward SIGINT to the child.
// motivation to explore alternatives yet.
// //
// Use of SIGTERM is also a bit complicated. If a terminal receives a SIGTERM, it just waits for // Use of SIGTERM is also a bit complicated. If a terminal receives a SIGTERM, it just waits for
// its children to exit — multiple SIGTERMs do not have any effect and the signals are not // its children to exit — multiple SIGTERMs do not have any effect and the signals are not
@ -38,6 +37,7 @@ pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result<ExitS
// to the child process or the process will not be killable. // to the child process or the process will not be killable.
#[cfg(unix)] #[cfg(unix)]
let status = { let status = {
use std::io::{IsTerminal, stdin};
use std::ops::Deref; use std::ops::Deref;
use anyhow::Context; use anyhow::Context;
@ -94,7 +94,6 @@ pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result<ExitS
let mut sigterm_handle = handle_signal(SignalKind::terminate())?; let mut sigterm_handle = handle_signal(SignalKind::terminate())?;
let mut sigint_handle = handle_signal(SignalKind::interrupt())?; 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. // The following signals are terminal by default, but can have user defined handlers.
// Forward them to the child process for handling. // Forward them to the child process for handling.
@ -130,6 +129,9 @@ pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result<ExitS
)))] )))]
let mut siginfo_handle = PlatformSpecificSignal::Dummy; let mut siginfo_handle = PlatformSpecificSignal::Dummy;
// Use TTY detection as a heuristic
let is_interactive = stdin().is_terminal();
// Notably, we do not handle `SIGCHLD` (sent when a child process status changes) and // 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 // `SIGIO` or `SIGPOLL` (sent when a file descriptor is ready for io) as they don't make
// sense for this parent / child relationship. // sense for this parent / child relationship.
@ -151,14 +153,11 @@ pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result<ExitS
// Check if the child pgid has changed // Check if the child pgid has changed
let child_pgid = getpgid(Some(child_pid)).context("Failed to get PID of child process")?; let child_pgid = getpgid(Some(child_pid)).context("Failed to get PID of child process")?;
// Increment the number of interrupts seen
sigint_count += 1;
// If the pgid _differs_ from the parent, the child will not receive a SIGINT // If the pgid _differs_ from the parent, the child will not receive a SIGINT
// and we should forward it. If we've received multiple SIGINTs, forward it // and we should forward it. If we think we're not in an interactive terminal,
// regardless. // forward the signal unconditionally.
if child_pgid == parent_pgid && sigint_count < 2 { if child_pgid == parent_pgid && is_interactive {
debug!("Received SIGINT, assuming the child received it as part of the process group"); debug!("Received SIGINT, assuming the child received it from the terminal driver");
continue; continue;
} }