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

Shut down all children on SIGTERM #39

Open
etianen opened this issue Oct 19, 2015 · 4 comments
Open

Shut down all children on SIGTERM #39

etianen opened this issue Oct 19, 2015 · 4 comments

Comments

@etianen
Copy link

etianen commented Oct 19, 2015

I see that parallelshell responds to the SIGINT signal by cleanly shutting down all it's children.

However, when stopped with the SIGTERM signal it simply exits, leaving all it's spawned children running.

Is this intentional behaviour?

@keithamus
Copy link
Collaborator

Hey @etianen thanks for the issue. It sounds like a bug. PRs welcome to fix it 😉

@etianen
Copy link
Author

etianen commented Oct 19, 2015

Out of interest, why does parellelshell choose to terminate child processes
with SIGINT rather than SIGTERM?

I think the correct behaviour here is to listen for both SIGINT and
SIGTERM, then stop the child processes with the same signal. I'm not
totally certain, however. I think you could also make a case for
responding to both SIGINT and SIGTERM, but always sending SIGTERM to child
processes (it's the traditional kill signal).

On Mon, 19 Oct 2015 at 09:54 Keith Cirkel notifications@github.com wrote:

Hey @etianen https://github.com/etianen thanks for the issue. It sounds
like a bug. PRs welcome to fix it [image: 😉]


Reply to this email directly or view it on GitHub
#39 (comment)
.

@keithamus
Copy link
Collaborator

SIGINT is used when a terminal sends Ctrl+C. We close apps down because that's what you'd expect from sending Ctrl+C. I guess we should probably send all signals to the respective processes though, as you suggest.

FYI, if you submit a PR to this code, you should know that mysticatea/npm-run-all#10 exists and that parallelshell may be deprecated soon, in favour of consolidating it with other libs (see mysticatea/npm-run-all#10 for more). While I'm happy to release new versions in the interim - this is the end goal of parallelshell (to be subsumed/consumed into npm-run-all).

@etianen
Copy link
Author

etianen commented Oct 19, 2015

Hmm, I see, thanks for the heads up!

I'm not sure I'll bother with the PR, in that case. I've been using
parallelshell as a quick process manager hack, but I can make do with bash
builtins in the meanwhile. :)

On Mon, 19 Oct 2015 at 10:11 Keith Cirkel notifications@github.com wrote:

SIGINT is used when a terminal sends Ctrl+C. We close apps down because
that's what you'd expect from sending Ctrl+C. I guess we should probably
send all signals to the respective processes though, as you suggest.

FYI, if you submit a PR to this code, you should know that
mysticatea/npm-run-all#10
mysticatea/npm-run-all#10 exists and that
parallelshell may be deprecated soon, in favour of consolidating it with
other libs (see mysticatea/npm-run-all#10
mysticatea/npm-run-all#10 for more). While
I'm happy to release new versions in the interim - this is the end goal of
parallelshell (to be subsumed/consumed into npm-run-all).


Reply to this email directly or view it on GitHub
#39 (comment)
.

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

No branches or pull requests

3 participants