Skip to content

fix: ubuntu loop waiting for sub processes#1635

Merged
remy merged 1 commit intomasterfrom
fix/1633
Nov 22, 2019
Merged

fix: ubuntu loop waiting for sub processes#1635
remy merged 1 commit intomasterfrom
fix/1633

Conversation

@remy
Copy link
Copy Markdown
Owner

@remy remy commented Nov 21, 2019

Fixes #1633

Multiple parts but all localised to run.js:

  • Refactor the kill logic to be simpler
  • Consistently use pstree to determine sub process list
  • Pipe stderr through nodemon to squash sh -c error warning

Bug was caused by waiting on multiple sub processes and if they all
ended the logic would only subtract one from the count list (rather
than the total number).

I've refactored the code so that it doesn't use the kill -0 <pid> as
this was a little confusing to read (it's effectively a no op) and
switched to using pstree to test if any sub processes are still running.

The logic for killing the processes has also been refactored to
simplify. Before it would fork the logic based on whether ps existed
on the system.

Now it uses the same logic with the exception of the kill signal sent -
when ps isn't on the system, we have to send numeric signals (I can't
remember how I found that out, but I do remember it was a painful
process!).

The last part required due to a side effect of the refactor on kill:
when a kill signial is sent to sh -c the shell prints a warning.
Details on how to replicate: https://git.io/Je6d0

To squash this, I track if the process is about to be killed (by
flagging the sub process right before the kill function call) and if
there's an error whilst shutdown is in effect, the error is only printed
to nodemon's detailed output (using nodemon -V).

Fixes #1633

Multiple parts but all localised to run.js:

- Refactor the kill logic to be simpler
- Consistently use pstree to determine sub process list
- Pipe stderr through nodemon to squash `sh -c` error warning

Bug was caused by waiting on multiple sub processes and if they all
ended the logic would only subtract one from the count list (rather
than the total number).

I've refactored the code so that it doesn't use the `kill -0 <pid>` as
this was a little confusing to read (it's effectively a no op) and
switched to using pstree to test if any sub processes are still running.

The logic for killing the processes has also been refactored to
simplify. Before it would fork the logic based on whether `ps` existed
on the system.

Now it uses the same logic with the exception of the kill signal sent -
when `ps` isn't on the system, we have to send numeric signals (I can't
remember how I found that out, but I do remember it was a painful
process!).

The last part required due to a side effect of the refactor on kill:
when a kill signial is sent to `sh -c` the shell prints a warning.
Details on how to replicate: https://git.io/Je6d0

To squash this, I track if the process is about to be killed (by
flagging the sub process right before the kill function call) and if
there's an error whilst shutdown is in effect, the error is only printed
to nodemon's detailed output (using nodemon -V).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nodemon waits forever for subprocesses to end

1 participant