search

darkguy2008 / parallelshell

Run multiple shell commands in parallel
501 stars 44 forks source link

Shut down all children on SIGTERM #39

Open etianen opened 9 years ago

etianen commented 9 years ago

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 commented 9 years ago

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

etianen commented 9 years ago

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: :wink:]

— Reply to this email directly or view it on GitHub https://github.com/keithamus/parallelshell/issues/39#issuecomment-149150092 .

keithamus commented 9 years ago

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 https://github.com/mysticatea/npm-run-all/issues/10 exists and that parallelshell may be deprecated soon, in favour of consolidating it with other libs (see https://github.com/mysticatea/npm-run-all/issues/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 commented 9 years ago

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 https://github.com/mysticatea/npm-run-all/issues/10 exists and that parallelshell may be deprecated soon, in favour of consolidating it with other libs (see mysticatea/npm-run-all#10 https://github.com/mysticatea/npm-run-all/issues/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 https://github.com/keithamus/parallelshell/issues/39#issuecomment-149153427 .