Skip to content
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

SIGHUP seems to not be propagated to inner child processes #1781

Closed
davoclavo opened this issue Dec 24, 2023 · 4 comments
Closed

SIGHUP seems to not be propagated to inner child processes #1781

davoclavo opened this issue Dec 24, 2023 · 4 comments

Comments

@davoclavo
Copy link

davoclavo commented Dec 24, 2023

Howdy! First of all, I am a recent just convert and I am loving it. Thanks for all the work into this amazing tool.

I have been using a mix of just and zellij to handle some of my workflows, and noticed that when I close a zellij pane running a just recipe it leaves the process alive.

Summary: It seems that a SIGHUP signal is being sent to just, but that signal is not being propagated to the inner child process, which causes the whole process tree to be left alive.

Here is a basic way to reproduce this issue using a simple bash script:

just --version

just 1.16.0

sighup.sh

#!/bin/bash

# Define a function to handle SIGHUP
handle_sighup() {
 echo "Received SIGHUP"
 exit 1
}

# Register the function as a signal handler
trap handle_sighup SIGHUP

# Main program loop
while true; do
 sleep 1
done

justfile

sighup:
  bash ./sighup.sh

Problematic behaviour:

# Run recipe and send it to the background
just sighup &

# Attempt to SIGHUP the `just` process
kill -1 $(pgrep -f 'just sighup')
# Problem here: Nothing happens!

# Attempt to SIGHUP the child process directly
kill -1 $(pgrep -f 'bash ./sighup.sh')
# Got the following expected stdout output
Received SIGHUP
[1]  + 21612 exit 130   just sighup

Expected behaviour:

# Run recipe and send it to the background
just sighup &

# Attempt to SIGHUP the just process
kill -1 $(pgrep -f 'just sighup')

# Get the following expected stdout output and process termination
Received SIGHUP
[1]  + 21612 exit 130   just sighup

Happy to contribute a fix for this, but wanted to check in first if this is something worth fixing, or is it an expected behaviour? Also, any pointers on where to look for would be appreciated.

@davoclavo davoclavo changed the title SIGHUP seems to not be propagated SIGHUP seems to not be propagated to inner child processes Dec 24, 2023
@davoclavo
Copy link
Author

This issue also might be related to these other issues:

#1558
#1560

@damccull
Copy link

I'm also experiencing this when using just to launch tasks inside of zellij. I would love to see a fix for this. For now I'm going to have to adjust my zellij layout to launch the commands directly instead of using just. Hopefully can migrate back soon.

@kuglee
Copy link

kuglee commented Oct 12, 2024

I've also ran into this and it's not ideal. I'd be great to have a fix/option for this.

@casey
Copy link
Owner

casey commented Nov 19, 2024

Closing in favor of #2473, where I tried to collect everything related to signal handling in one place. I'm not able to reproduce processes being left behind when just is run inside of zellij. If I run just with a recipe that calls sleep 100 inside a zellij tab, closing that tab doesn't leave behind any processes, which is expected, since even if just ignores SIGHUP, it should be sent to all child processes, so sleep 100 should get it, and just should terminate when it exits. A reproduction would be great!

Currently, just is ignoring SIGHUP sent to it directly. However, I'm not sure if this is a problem, since SIGHUP, when the terminal is closed, should be sent to all child processes, so the child should get it, terminate, and then just should terminate.

@casey casey closed this as completed Nov 19, 2024
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

No branches or pull requests

4 participants