From f645499dbdac62d2fd00acaa3997f94669d6b9a4 Mon Sep 17 00:00:00 2001 From: konsti Date: Fri, 24 Jan 2025 16:20:32 +0100 Subject: [PATCH] Child exit with signal n returns 128+n (#10781) --- crates/uv/src/commands/mod.rs | 1 + crates/uv/src/commands/project/run.rs | 58 ++-------------------- crates/uv/src/commands/run.rs | 71 +++++++++++++++++++++++++++ crates/uv/src/commands/tool/run.rs | 58 ++-------------------- crates/uv/tests/it/run.rs | 18 ++++++- 5 files changed, 95 insertions(+), 111 deletions(-) create mode 100644 crates/uv/src/commands/run.rs diff --git a/crates/uv/src/commands/mod.rs b/crates/uv/src/commands/mod.rs index a3fb800b7..df7a76dc0 100644 --- a/crates/uv/src/commands/mod.rs +++ b/crates/uv/src/commands/mod.rs @@ -69,6 +69,7 @@ mod project; mod publish; mod python; pub(crate) mod reporters; +mod run; #[cfg(feature = "self-update")] mod self_update; mod tool; diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 65ef8b76f..bf1477553 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -51,6 +51,7 @@ use crate::commands::project::{ EnvironmentSpecification, ProjectError, ScriptInterpreter, WorkspacePython, }; use crate::commands::reporters::PythonDownloadReporter; +use crate::commands::run::run_to_completion; use crate::commands::{diagnostics, project, ExitStatus}; use crate::printer::Printer; use crate::settings::ResolverInstallerSettings; @@ -1119,64 +1120,11 @@ pub(crate) async fn run( // Spawn and wait for completion // Standard input, output, and error streams are all inherited // TODO(zanieb): Throw a nicer error message if the command is not found - let mut handle = process + let handle = process .spawn() .with_context(|| format!("Failed to spawn: `{}`", command.display_executable()))?; - // Ignore signals in the parent process, deferring them to the child. This is safe as long as - // the command is the last thing that runs in this process; otherwise, we'd need to restore the - // signal handlers after the command completes. - let _handler = tokio::spawn(async { while tokio::signal::ctrl_c().await.is_ok() {} }); - - // Exit based on the result of the command. - #[cfg(unix)] - let status = { - use tokio::select; - use tokio::signal::unix::{signal, SignalKind}; - - let mut term_signal = signal(SignalKind::terminate())?; - loop { - select! { - result = handle.wait() => { - break result; - }, - - // `SIGTERM` - _ = term_signal.recv() => { - let _ = terminate_process(&mut handle); - } - }; - } - }?; - - #[cfg(not(unix))] - let status = handle.wait().await?; - - if let Some(code) = status.code() { - debug!("Command exited with code: {code}"); - if let Ok(code) = u8::try_from(code) { - Ok(ExitStatus::External(code)) - } else { - #[allow(clippy::exit)] - std::process::exit(code); - } - } else { - #[cfg(unix)] - { - use std::os::unix::process::ExitStatusExt; - debug!("Command exited with signal: {:?}", status.signal()); - } - Ok(ExitStatus::Failure) - } -} - -#[cfg(unix)] -fn terminate_process(child: &mut tokio::process::Child) -> anyhow::Result<()> { - use nix::sys::signal::{self, Signal}; - use nix::unistd::Pid; - - let pid = child.id().context("Failed to get child process ID")?; - signal::kill(Pid::from_raw(pid.try_into()?), Signal::SIGTERM).context("Failed to send SIGTERM") + run_to_completion(handle).await } /// Returns `true` if we can skip creating an additional ephemeral environment in `uv run`. diff --git a/crates/uv/src/commands/run.rs b/crates/uv/src/commands/run.rs new file mode 100644 index 000000000..a06025c18 --- /dev/null +++ b/crates/uv/src/commands/run.rs @@ -0,0 +1,71 @@ +use crate::commands::ExitStatus; +use tokio::process::Child; +use tracing::debug; + +/// Wait for the child process to complete, handling signals and error codes. +pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result { + // Ignore signals in the parent process, deferring them to the child. This is safe as long as + // the command is the last thing that runs in this process; otherwise, we'd need to restore the + // signal handlers after the command completes. + let _handler = tokio::spawn(async { while tokio::signal::ctrl_c().await.is_ok() {} }); + + // Exit based on the result of the command. + #[cfg(unix)] + let status = { + use tokio::select; + use tokio::signal::unix::{signal, SignalKind}; + + let mut term_signal = signal(SignalKind::terminate())?; + loop { + select! { + result = handle.wait() => { + break result; + }, + + // `SIGTERM` + _ = term_signal.recv() => { + let _ = terminate_process(&mut handle); + } + }; + } + }?; + + #[cfg(not(unix))] + let status = handle.wait().await?; + + if let Some(code) = status.code() { + debug!("Command exited with code: {code}"); + if let Ok(code) = u8::try_from(code) { + Ok(ExitStatus::External(code)) + } else { + #[allow(clippy::exit)] + std::process::exit(code); + } + } else { + #[cfg(unix)] + { + use std::os::unix::process::ExitStatusExt; + debug!("Command exited with signal: {:?}", status.signal()); + // Following https://tldp.org/LDP/abs/html/exitcodes.html, a fatal signal n gets the + // exit code 128+n + if let Some(mapped_code) = status + .signal() + .and_then(|signal| u8::try_from(signal).ok()) + .and_then(|signal| 128u8.checked_add(signal)) + { + return Ok(ExitStatus::External(mapped_code)); + } + } + Ok(ExitStatus::Failure) + } +} + +#[cfg(unix)] +fn terminate_process(child: &mut Child) -> anyhow::Result<()> { + use anyhow::Context; + use nix::sys::signal::{self, Signal}; + use nix::unistd::Pid; + + let pid = child.id().context("Failed to get child process ID")?; + signal::kill(Pid::from_raw(pid.try_into()?), Signal::SIGTERM).context("Failed to send SIGTERM") +} diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index 36fc154c7..b18258e9d 100644 --- a/crates/uv/src/commands/tool/run.rs +++ b/crates/uv/src/commands/tool/run.rs @@ -37,6 +37,7 @@ use crate::commands::pip::loggers::{ }; use crate::commands::project::{resolve_names, EnvironmentSpecification, ProjectError}; use crate::commands::reporters::PythonDownloadReporter; +use crate::commands::run::run_to_completion; use crate::commands::tool::common::{matching_packages, refine_interpreter}; use crate::commands::tool::Target; use crate::commands::ExitStatus; @@ -188,7 +189,7 @@ pub(crate) async fn run( invocation_source, ); - let mut handle = match process.spawn() { + let handle = match process.spawn() { Ok(handle) => Ok(handle), Err(err) if err.kind() == std::io::ErrorKind::NotFound => { match get_entrypoints(&from.name, &site_packages) { @@ -239,60 +240,7 @@ pub(crate) async fn run( } .with_context(|| format!("Failed to spawn: `{executable}`"))?; - // Ignore signals in the parent process, deferring them to the child. This is safe as long as - // the command is the last thing that runs in this process; otherwise, we'd need to restore the - // signal handlers after the command completes. - let _handler = tokio::spawn(async { while tokio::signal::ctrl_c().await.is_ok() {} }); - - // Exit based on the result of the command. - #[cfg(unix)] - let status = { - use tokio::select; - use tokio::signal::unix::{signal, SignalKind}; - - let mut term_signal = signal(SignalKind::terminate())?; - loop { - select! { - result = handle.wait() => { - break result; - }, - - // `SIGTERM` - _ = term_signal.recv() => { - let _ = terminate_process(&mut handle); - } - }; - } - }?; - - #[cfg(not(unix))] - let status = handle.wait().await?; - - if let Some(code) = status.code() { - debug!("Command exited with code: {code}"); - if let Ok(code) = u8::try_from(code) { - Ok(ExitStatus::External(code)) - } else { - #[allow(clippy::exit)] - std::process::exit(code); - } - } else { - #[cfg(unix)] - { - use std::os::unix::process::ExitStatusExt; - debug!("Command exited with signal: {:?}", status.signal()); - } - Ok(ExitStatus::Failure) - } -} - -#[cfg(unix)] -fn terminate_process(child: &mut tokio::process::Child) -> anyhow::Result<()> { - use nix::sys::signal::{self, Signal}; - use nix::unistd::Pid; - - let pid = child.id().context("Failed to get child process ID")?; - signal::kill(Pid::from_raw(pid.try_into()?), Signal::SIGTERM).context("Failed to send SIGTERM") + run_to_completion(handle).await } /// Return the entry points for the specified package. diff --git a/crates/uv/tests/it/run.rs b/crates/uv/tests/it/run.rs index 63e4ca7bc..a8fa635eb 100644 --- a/crates/uv/tests/it/run.rs +++ b/crates/uv/tests/it/run.rs @@ -3266,7 +3266,7 @@ fn run_gui_script_explicit_windows() -> Result<()> { if not executable.startswith("pythonw"): print(f"Error: Expected pythonw.exe but got: {executable}", file=sys.stderr) sys.exit(1) - + print(f"Using executable: {executable}", file=sys.stderr) "#})?; @@ -3776,3 +3776,19 @@ fn run_with_group_conflict() -> Result<()> { Ok(()) } + +/// Test that a signal n makes the process exit with code 128+n. +#[cfg(unix)] +#[test] +fn exit_status_signal() -> Result<()> { + let context = TestContext::new("3.12"); + + let script = context.temp_dir.child("segfault.py"); + script.write_str(indoc! {r" + import os + os.kill(os.getpid(), 11) + "})?; + let status = context.run().arg(script.path()).status()?; + assert_eq!(status.code().expect("a status code"), 139); + Ok(()) +}