From 2af96de64406265bf63d189771c162eb7b2f477f Mon Sep 17 00:00:00 2001 From: konstin Date: Mon, 20 Jan 2025 15:59:11 +0100 Subject: [PATCH 1/3] Unify child handling in uv crate Removes some code duplication --- crates/uv/src/commands/mod.rs | 1 + crates/uv/src/commands/project/run.rs | 58 ++----------------------- crates/uv/src/commands/run.rs | 62 +++++++++++++++++++++++++++ crates/uv/src/commands/tool/run.rs | 58 ++----------------------- 4 files changed, 69 insertions(+), 110 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 a3fb800b7249..df7a76dc0211 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 777d5472c7e1..3fc1bb867a4b 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; @@ -1115,64 +1116,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 000000000000..60d2cbda443d --- /dev/null +++ b/crates/uv/src/commands/run.rs @@ -0,0 +1,62 @@ +use crate::commands::ExitStatus; +use anyhow::Context; +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()); + } + Ok(ExitStatus::Failure) + } +} + +#[cfg(unix)] +fn terminate_process(child: &mut 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") +} diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index 36fc154c7c3c..b18258e9d6f6 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. From d8fb3463df8d8091d020df33b635b396ba7880d5 Mon Sep 17 00:00:00 2001 From: konstin Date: Mon, 20 Jan 2025 16:12:49 +0100 Subject: [PATCH 2/3] Exit with signal n return 128+n Following https://tldp.org/LDP/abs/html/exitcodes.html, a fatal signal n gets the exit code 128+n (unix only). Fixes #10751 --- crates/uv/src/commands/run.rs | 9 +++++++++ crates/uv/tests/it/run.rs | 18 +++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/uv/src/commands/run.rs b/crates/uv/src/commands/run.rs index 60d2cbda443d..1d9e8088ebe2 100644 --- a/crates/uv/src/commands/run.rs +++ b/crates/uv/src/commands/run.rs @@ -47,6 +47,15 @@ pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result 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) "#})?; @@ -3750,3 +3750,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(()) +} From d53ac338346d5cd84dbc34ac5e263ed8048b0b80 Mon Sep 17 00:00:00 2001 From: konstin Date: Mon, 20 Jan 2025 16:36:25 +0100 Subject: [PATCH 3/3] Clippy: Windows --- crates/uv/src/commands/run.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/uv/src/commands/run.rs b/crates/uv/src/commands/run.rs index 1d9e8088ebe2..a06025c18d6a 100644 --- a/crates/uv/src/commands/run.rs +++ b/crates/uv/src/commands/run.rs @@ -1,5 +1,4 @@ use crate::commands::ExitStatus; -use anyhow::Context; use tokio::process::Child; use tracing::debug; @@ -63,6 +62,7 @@ pub(crate) async fn run_to_completion(mut handle: Child) -> anyhow::Result anyhow::Result<()> { + use anyhow::Context; use nix::sys::signal::{self, Signal}; use nix::unistd::Pid;