fix: daemon exit on RPC shutdown with --enable-gc - #11447
Open
lidel wants to merge 1 commit into
Open
Conversation
maybeRunGC ran PeriodicGC on req.Context, which the RPC shutdown command never cancels (it only closes the node), so gcErrc never closed and daemonFunc blocked in merge() forever. Cancel the GC context on node close as well, keeping req.Context so a signal still aborts an in-flight sweep before the OnStop hooks close the datastore. Closes #11424
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A daemon started with
--enable-gcansweredipfs shutdown(POST /api/v0/shutdown) by closing the node and then never exited: the periodic GC loop waited on the command's request context, which only a signal cancels, sodaemonFuncnever finished draining its error channels. Supervisors that stop Kubo over the RPC API waited on the process forever, andInternal.ShutdownTimeoutcould not help because its watchdog arms only after that drain. Present since #1495.Fix
req.Contextand cancel it whennode.Context()ends too, so the loop stops on node close; a signal still aborts an in-flight sweep before the OnStop hooks close the datastore (rationale inmaybeRunGC)TestDaemonRPCShutdownstops a daemon over RPC, with and without--enable-gc, and waits on the process; the harnessStopDaemonsends SIGTERM and would have hidden thisOut of scope: on the RPC path a running sweep still overlaps
app.Stopuntil the node context ends, and the shutdown watchdog still does not cover the RPC path.ipfs shutdown(RPC), only on SIGINT/SIGTERM #11424 cc @Rinse12