Repository navigation
Stop saves pending trust changes and no longer waits on a hung RPC - #48
Merged
Merged
Conversation
Stop closed done before waiting for the background RPCs, never waited for the drain goroutine, and never saved on its own. drainSaves picks between a pending dirty signal and done at random, so a queued save was dropped about half the time, and a change made by an RPC that finished while Stop waited for it (backfillPeerKey binding or dropping a record) was never saved. Eight relayed approvals followed by Stop left trust.json incomplete in 48 to 58 of 200 runs. Stop now waits for the RPCs first, then closes done, waits for drainSaves to exit (drainDone), and saves once more itself. The RPC wait is bounded: the first Stop sets a deadline of now + stopRPCWait (2 s), because a registry RPC can sit in its 30 s read deadline and the daemon's whole shutdown budget is shorter. Later calls share that deadline, so the daemon's second Stop does not wait again, but every call saves, so the second one picks up what a late RPC changed. saveTrust now holds saveMu, so the drain goroutine and concurrent Stop calls never stage their writes in trust.json.tmp at the same time. The Stop signature is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stop's final save was unconditional, so a plain start and stop rewrote trust.json. A trust.json that failed to parse at startup was replaced with the empty state, where before it survived until the next real trust change. markDirty now also sets unsaved, an atomic.Bool meaning "changed since the last successful save". A save clears it under saveMu and mu.RLock, before taking its snapshot, and sets it again if the write fails. Stop saves only if it is set, and checks it under saveMu, so a concurrent Stop that skips has waited out the save in progress and sees whether it failed. drainSaves and direct saveTrust calls are unchanged. New tests: an unparseable trust.json is byte-for-byte unchanged by a start and two Stops; with no change, Stop leaves an existing file's mtime alone and does not create a missing one; a failed save leaves the change unsaved and Stop writes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
1 participant
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
Manager.Stopcould lose trust changes and could block shutdown.drainSaveswas untracked, and Stop closeddonewithout waiting for it or doing a final save.drainSavespicks randomly betweendirtyanddone, so a pending save was dropped about half the time.wg.Wait()had no limit. A registry RPC can take up to its 30 s read deadline, against the daemon's 8 s shutdown budget.trust.json.tmp.Fix
doneonce, waits fordrainSavesto exit, and saves.saveMu).Stop()'s signature is unchanged.Tests
.tmpis left and the file is complete.The first two fail on main.
go test -race ./...passes, and the new tests pass 10 of 10 runs.🤖 Generated with Claude Code