Abort commands not running when max processes < N #460
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.
Problem
In a few issues, this situation came up where commands that haven't started yet will run anyways when they shouldn't even start.
Best examples of this are SIGINT and
--kill-others
combined with--max-processes
that's lower than N (where N = number of commands itself).For someone who pressed Ctrl + C, it could be annoying that one press isn't sufficient and they have to press it up to N times (see #452).
For
--kill-others
/--kill-others-on-fail
, it, well... just doesn't work (see #433).Solution
With SIGINT, it's quite clear that commands awaiting their turn should be aborted.
For
--kill-others
, there was a possible interaction with--restart-tries
: should it be respected at all?But the flag is documented as "restart processes that died", which isn't the case of a command that never started. Therefore I made the decision of also aborting these commands.
Now, aborting the commands is done here by using an
AbortController
.It's quite simple: flow controllers make use of it, and concurrently stops further spawning of processes if the signal is already aborted.
Note
AbortController
is built into Node 15+, which is a requirement for concurrently 9.0.0 (#448), therefore this PR is a breaking change.Fixes #433
Fixes #452