Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix child processes killing function #510

Merged
merged 2 commits into from
Sep 5, 2022

Conversation

kristopher-pellizzi
Copy link
Contributor

If a process exits (either normally or due to an error) before the call to process.kill('SIGINT') inside function killTask, then the signal cannot be delivered to the process, and the handler registered on 'exit' event for the process won't be executed, thus leaving the promise unresolved, and therefore the extension gets stuck.

To solve this, check the value returned by process.kill('SIGINT'):

  • If that's true, then the signal is delivered, and the 'exit' event handler should be executed;
  • If that 's false, then the signal cannot be delivered. Then, check the exitCode of the process. If that is not null, then the process already exited, therefore there is nothing more to do than resolve the returned promise.

Note that theoretically, in case the call to process.kill('SIGINT') returns false, then the exitCode of the process should be necessarily not null.
So, the conditional check for the value of exitCode should be quite redundant, but it allows to easily fix function's behavior in case the above assumption is false.

This should fix issue #360 and related issues.

Packages @types and typescript updated to the last available version.

If SIGINT cannot be delivered to a process and its exitCode is not null (the process already exited), then do not wait for an 'exit' event, simply resolve the promise.
@qjebbs qjebbs merged commit 42157a5 into qjebbs:master Sep 5, 2022
@qjebbs
Copy link
Owner

qjebbs commented Sep 5, 2022

Thanks for your work!

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.

2 participants