open-cli-tools / concurrently

Run commands concurrently. Like `npm run watch-js & npm run watch-less` but better.
https://www.npmjs.com/package/concurrently
MIT License
6.98k stars 227 forks source link

Abort commands not running when max processes < N #460

Closed gustavohenke closed 5 months ago

gustavohenke commented 7 months ago

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

coveralls commented 7 months ago

Coverage Status

coverage: 99.346% (+0.02%) from 99.324% when pulling 91d6b3ff820cfbca6d6ed5f34116c5f58fcb7266 on issues-433-and-452 into bae6fe58022f84430998c30070d951cd6429c47d on main.

gustavohenke commented 7 months ago

@paescuj keen to take a look?