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

Improve child process management #663

Merged
merged 1 commit into from
Feb 3, 2022
Merged

Conversation

gsoltis
Copy link
Contributor

@gsoltis gsoltis commented Feb 1, 2022

  • When we receive an external signal, send SIGINT to our child processes
  • When a child process exits w/ a non-zero exit code, and --continue is not set, send SIGINT to all of our child processes

Practically, this means that hitting Ctrl+C on the terminal will shut down all of the child processes via SIGINT. Additionally, if one task fails, other tasks will not be left lingering, and will be sent a SIGINT.

Some considerations: if turbo is invoked via yarn or npm, it will not be the root of the process tree, and as such Ctrl+C will hand control back to the user before turbo has exited, meaning that the user will likely get stdout/stderr after the prompt has reappeared. I don't think there's anything we can do about this.

Running turbo directly should avoid this, with the exception of a child process failing to exit in the 10-second grace period.

Also note that as a result of this change, each child process is given its own process group. This is done so that we can send SIGINT to an entire child process tree without also sending it to ourselves. By default, all child processes would be in the same process tree as turbo.

Remaining work:

  • testing
  • resolve some questions on structure between process.Manager and the signal handling code
  • document code modified from hashicorp/consul-template

Fixes #444
Fixes #470

@vercel
Copy link

vercel bot commented Feb 1, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/vercel/turbo-site/4KHFLDKEMT4ekNKoA644Ae1mRfww
✅ Preview: https://turbo-site-git-gsoltis-handlesignals.vercel.sh

@jaredpalmer
Copy link
Contributor

Makes sense so far to me

cli/cmd/turbo/main.go Show resolved Hide resolved
@jaredpalmer jaredpalmer merged commit 8e8a4c6 into main Feb 3, 2022
@jaredpalmer jaredpalmer deleted the gsoltis/handle_signals branch February 3, 2022 21:21
@scorsi
Copy link

scorsi commented Feb 3, 2022

In which Turbo version this PR will be available ? :)

@jaredpalmer
Copy link
Contributor

jaredpalmer commented Feb 3, 2022 via email

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.

Kill other processes when another fails turborepo does not propagate the SIGINT signal event.
3 participants