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

feat: propagate signals to launched subprocesses so that they can properly clean up resources #63

Closed
wants to merge 1 commit into from

Conversation

r-vdp
Copy link
Member

@r-vdp r-vdp commented Nov 18, 2024

Please let me know if there is a better way to do this, I haven't written Go in a long time.

There also seems to be some bug, but I'm not sure where the issue is coming from. If the underlying process doesn't exit, for instance after receiving SIGHUP, and we sent another signal, then cmd.Process.Pid ends up decremented by one, and so we try to send the signal to the wrong process. I don't know if I'm holding something the wrong way here, or if this is a Go runtime bug. This is what I see when sending SIGHUP twice:

2024/11/18 15:36:12 deploy ref refs/heads/main
2024/11/18 15:36:12 origin XXXX
Installing signal handlers...
Done
Launched child, child pid: 218206
Caught OTHER signal!
Passing signal to child process
2024/11/18 15:36:40 error sending signal to child process (218205): os: process already finished

Which is really strange, because there is no process with PID 218205, and the actual spawned process has PID 218206.

@r-vdp
Copy link
Member Author

r-vdp commented Nov 21, 2024

Superseded by #64.

@r-vdp r-vdp closed this Nov 21, 2024
@zimbatm zimbatm deleted the propagate_signals branch November 21, 2024 13:45
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.

1 participant