Skip to content

fix: forward kill signals from pixi to deno task shell commands #3837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

wolfv
Copy link
Member

@wolfv wolfv commented May 26, 2025

We had some logic that ignored CTRL+C on the pixi level. However, my understanding is that it only made CTRL+C "transparent" so that it reached the right process underneath, but didn't properly set up forwarding for CTRL+C and other signals.

By using the KillSignal from deno_task_shell we can properly forward any signal received in pixi to the child processes.

Should fix #3789

TODO:

  • check that CTRL+C works on all platforms (currently only tested on macOS)
  • check that CTRL+C in selection dialog still looks good

Copy link
Contributor

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this works that would be great! Lets test it throughly!

Comment on lines +523 to +526
// fn reset_cursor() {
// let term = console::Term::stdout();
// let _ = term.show_cursor();
// }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt this still an issue?

@@ -0,0 +1,257 @@
// Copyright 2018-2025 the Deno authors. MIT license.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used?

@@ -371,6 +370,73 @@ enum TaskExecutionError {
UnsupportedPlatformError(#[from] UnsupportedPlatformError),
}

/// Runs a deno task future forwarding any signals received
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deno?

@@ -304,7 +303,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
}

// Handle CTRL-C ourselves again
ctrlc_should_exit_process.store(true, Ordering::Relaxed);
// ctrlc_should_exit_process.store(true, Ordering::Relaxed);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove commented code.

@ruben-arts
Copy link
Contributor

Using the example in the issue this doesn't allow me to correctly stop the pixi process anymore.

import signal
import time

def signal_handler(signum, frame):
    print(f"Signal handler called with signal {signum}")

signal.signal(signal.SIGINT, signal_handler)

while True:
    time.sleep(1)

This will run indefinitely without me being able to stop it. While the old pixi allows me to send ctrl+z to suspend it.

If I add exit(0) to the signal_handler it gives me this output:

➜ pixi run python test.py
^CSignal handler called with signal 2
Traceback (most recent call last):
 File "/Users/rubenarts/dev/pixi/test.py", line 11, in <module>
   time.sleep(1)
   ~~~~~~~~~~^^^
 File "/Users/rubenarts/dev/pixi/test.py", line 5, in signal_handler
   print(f"Signal handler called with signal {signum}")
   ~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/Users/rubenarts/dev/pixi/test.py", line 5, in signal_handler
   print(f"Signal handler called with signal {signum}")
   ~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: reentrant call inside <_io.BufferedWriter name='<stdout>'>

❯

While the latest release gives me:

➜  ~/.pixi/bin/pixi run python test.py
^CSignal handler called with signal 2
➜ 

@wolfv
Copy link
Member Author

wolfv commented May 26, 2025

Thanks for trying. Indeed, I also added exit(0) inside the signal handler to make it actually stop. I haven't tested on Linux yet, but looks like that might be necessary.

@wolfv
Copy link
Member Author

wolfv commented May 26, 2025

When I run this, I get:

╭─wolfv@pixi ~/Programs/pixi/examples/ctrlc ‹use-kill-signal●› 
╰─$ cargo r -- run python test.py                                                                           1 ↵
   Compiling pixi v0.47.0 (/Users/wolfv/Programs/pixi)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 14.50s
     Running `/Users/wolfv/Programs/pixi/target/debug/pixi run python test.py`
Sleeping...
Sleeping...
^CSignal handler called with signal 2

@ruben-arts
Copy link
Contributor

The problem for me is that I can't properly quit it when the signal is ignored by the running executable. Pixi just sends it through and doesn't stop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signals not forwarded to child processes
3 participants