Add setpgid for all called commands#1494
Merged
Merged
Conversation
eikenb
force-pushed
the
child-set-process-group
branch
from
41460d9 to
13b5204
Compare
eikenb
force-pushed
the
child-set-process-group
branch
from
13b5204 to
306ce90
Compare
Need to make sure signals are sent to all processes as sub-shells interfere with signal propagation. Using Setpgid to create a process group for the command and all it's subprocesses fixes this by allowing child to send the signals to the pgid, which thus gets sent to all processes in that group. This is required for upcoming change to use sub-shells for all executed commands to eliminate need for shell parsing. With this change it is always set to true and the pgid is always used to send signals to the process(es). In the future it could be made configurable if needed.
Small issue with subshells sending text to STDERR about the signal received, breaking the text check (result of sending signals to process group). Changed them to only check STDOUT, where the test text is sent. Previously used a sleep time of 500 milliseconds when 50 worked fine. This sped up the child tests a lot. Everything else are cleaning up lots of useless checks, hardcoded times, and replace subshell test calls use of bash with standard /bin/sh.
eikenb
force-pushed
the
child-set-process-group
branch
from
306ce90 to
7d17648
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Need to make sure signals are sent to all processes as sub-shells interfere with signal propagation. Using Setpgid to create a process group for the command and all it's subprocesses fixes this by allowing child to send the signals to the pgid, which thus gets sent to all processes in that group.
This is required for upcoming change to use sub-shells for all executed commands to eliminate need for shell parsing.
With this change it is always set to true and the pgid is always used to send signals to the process(es).
In the future it could be made configurable if needed.
Might be easier to review using the commits. The second commit is test cleanups and fixes where the first one contains the new code and tests.