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

fix(command-dev): ensure no orphaned child process on Windows #3278

Merged
merged 1 commit into from
Aug 31, 2021
Merged

fix(command-dev): ensure no orphaned child process on Windows #3278

merged 1 commit into from
Aug 31, 2021

Conversation

Xenonym
Copy link
Contributor

@Xenonym Xenonym commented Aug 31, 2021

- Summary

Fixes #3251.

On Windows, the dev command does not properly terminate the framework process on exit, and leaves them running even though netlify-cli has exited.

This is because execa defaults to windowsHide: true, which seems to interfere with terminating child processes: sindresorhus/execa#433.

Other projects have also encountered this issue with execa's defaults and Windows: FredKSchott/snowpack#1022, lerna/lerna#2946.

Let's set windowsHide: false so that the framework process will terminate properly on exit.

- Test plan

Run netlify dev with this PR and attempt to exit on Windows (e.g. via Ctrl-C). No framework processes should be left running after exit.

- A picture of a cute animal (not mandatory but encouraged)
🦓

`execa` defaults to `windowsHide: true`, which prevents child processes
from terminating child processes on exit [1].

Let's set `windowsHide: false` to terminate child processes properly on
exit.

[1]: sindresorhus/execa#433
@erezrokah erezrokah requested a review from ehmicky August 31, 2021 13:52
@ehmicky ehmicky added the type: bug code to address defects in shipped code label Aug 31, 2021
@ehmicky
Copy link
Contributor

ehmicky commented Aug 31, 2021

Thanks a lot for looking into this @Xenonym.

This is a rather tricky one. Both true and false values for windowsHide have downsides. The two following comments are pointing to many issues discussing this in more details (here and there).

The CTRL-C issue has actually been mentioned in the following core Node.js issue (nodejs/node#29837).

Overall, I think using windowsHide: false might be the lesser evil, so let's give this a go 👍

@ehmicky ehmicky merged commit ef396b2 into netlify:main Aug 31, 2021
@Xenonym Xenonym deleted the fix/dev-windows-orphan-process branch August 31, 2021 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

netlify dev leaves behind zombie processes on Windows
2 participants