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

Remove try/catch inside kill() #273

Merged
merged 1 commit into from
Jun 3, 2019
Merged

Conversation

ehmicky
Copy link
Collaborator

@ehmicky ehmicky commented Jun 3, 2019

There are only two ways childProcess.kill() might throw:

  1. invalid signal argument
  2. signal not supported by current OS (see here and there)

Both conditions cannot happen with kill('SIGKILL'), since that signal is supported on all OS.
When the child process has already exited, kill() returns false.

So the try/catch wrapping kill('SIGKILL') is not needed.

@zokker13

@ehmicky ehmicky merged commit 6fb284b into master Jun 3, 2019
@ehmicky ehmicky deleted the feature/remove-try-catch-kill branch June 3, 2019 14:17
@zokker13
Copy link
Contributor

zokker13 commented Jun 3, 2019

I thought about that the kill might fail if you start the process-to-be with sudo so the process has another owner. But it turns out that the kill just returns false which is fine. :)

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.

3 participants