Fix child process kill error with npx based servers #850
+9
−1
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
When running npx based servers in stdio mode, the npx process spawns a sub-process. During exit, the main process does not propagate the SIGTERM to the child process which leads to a failed kill signal and the stdin & stdout of the root process getting mangled.
The current change creates a new process group when launching the fork for the server and ensures that a kill signal is sent for the process group is the process does not shutdown cleanly. This would ensure that the process group (which includes all the child process) would be killed successfully to avoid any potential issues.
How Has This Been Tested?
Yes, a simple script like this:
The above script will currently block the stdio of the parent server when run using as:
python test.py
After the change the npx root & the node child process die succesfully allowing the control to flow back to the parent process ensuring a clean shutdown.
Breaking Changes
No, this should not be a breaking change since the process group flag is ignored in non-POSIX systems, and the cleanup via process group functionality is only applied for POSIX style launches.
Types of changes
Checklist
Additional context
Please refer to the issue here (slightly different context):
#547