fix: forward CTRL+C signal to deno_task_shell#4243
fix: forward CTRL+C signal to deno_task_shell#4243baszalmstra merged 10 commits intoprefix-dev:mainfrom
deno_task_shell#4243Conversation
deno_task_shell
|
Can we get this tested on all platforms? This was an issue I ran into last time: #3837 (comment) |
|
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 I think we also have to deal with |
|
Oke so I did a bit of a deepdive. Pixi doesnt listen to either SIGKILL or SIGSTOP because we want the OS to handle that for us (e.g. suspend or kill the process). However, when SIGSTOP is send to the pixi process it will be suspended but the child process will not. I see no way around this except for running the child processes in the same process group, but I dont know if thats possible with deno_task_shell. Using ctrl+z in the way you described is also weird @ruben-arts . What you are effectively doing is sending the process to the background, it remains alive and will keep consuming resources. A better approach would be to use ctrl+\ to terminate the process. I would be in favour of leaving this limitation as is for now, and merging this PR. I think it solves way more problems than it introduces. |
|
I also did a deep dive - and SIGSTOP and SIGTSTP (CTRL+Z) are different. I guess what we could do is we could send SIGTSTP to the child process, unset the listener, re-send SIGTSTP to the pixi process. Then, on SIGCONT do the reverse. I don't really know whether that's worth it though. Seems like a pretty heavy complication. |
|
@ruben-arts the test is fixed and ran on Unix. @baszalmstra idk if we could test this easily on Windows? IMO we can merge this. |
|
Thanks! <3 |
|
It looks like this is broken again. |
|
@jonashaag but it worked for some time? |
|
Not sure! I tried to remove my hacks just now and it looks like I'll have to keep them for now |
I cleaned up my previous work and this should fix #3789
I'll look into forwarding more signals (like SIGHUP etc.)