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

feat: force kill daemons after timeout #441

Merged
merged 6 commits into from
Jan 29, 2020
Merged

feat: force kill daemons after timeout #441

merged 6 commits into from
Jan 29, 2020

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented Jan 22, 2020

This PR re-adds a feature that was removed in 1.0.0 whereby daemons are force killed with a SIGKILL when they do not stop within a timeout.

It adds 2 new options to the Daemon class - forceKill (boolean - default true) and forceKillTimeout (number - default 10000).

resolves #438

Alan Shaw added 2 commits January 22, 2020 12:29
This PR re-adds a feature that was removed in 1.0.0 whereby daemons are force killed with a SIGKILL when they do not stop within a timeout.

It adds 2 new options to the `Daemon` class - `forceKill` (boolean - default `true`) and `forceKillTimeout` (number - default 10000).
@achingbrain
Copy link
Member

The new options need documenting here: https://github.com/ipfs/js-ipfsd-ctl#stop

src/ipfsd-daemon.js Show resolved Hide resolved
src/ipfsd-daemon.js Outdated Show resolved Hide resolved
src/ipfsd-daemon.js Show resolved Hide resolved
src/ipfsd-daemon.js Show resolved Hide resolved
@Stebalien
Copy link
Member

Answering the question in ipfs/interop#96 (comment), just send a SIGKILL. There's no real need to wait the 10s.

If someone wants to try to debug this:

  • Wait for the test to hang.
  • Send a SIGQUIT to go-ipfs.
  • Look at the stacktrace (go-ipfs will send it to stderr on SIGQUIT).
    • Or file an issue with that stack trace.

@hugomrdias
Copy link
Member

@Stebalien thanks, so maybe we should reduce the timeout, wait 5s for api.stop() to finish after that SIGKILL immediately.

@Stebalien
Copy link
Member

SGTM!

@achingbrain
Copy link
Member

@hugomrdias I've pushed changes to this branch that address the PR comments (I think) - can it go in?

src/ipfsd-daemon.js Outdated Show resolved Hide resolved
src/ipfsd-daemon.js Outdated Show resolved Hide resolved
@hugomrdias hugomrdias merged commit 182e532 into master Jan 29, 2020
@hugomrdias hugomrdias deleted the feat/force-kill branch January 29, 2020 11:41
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.

Daemon is no longer force killed
4 participants