Context
Raised during review of #119 (by Copilot and in review comment R2 on monitor.go). Pre-existing behavior, unchanged by that PR.
Monitor.Close() is a quiescence point for the refresh loop and its in-flight state saves (established in #119), but ProcessReceivedMessage spawns two untracked goroutines per accepted heartbeat message:
go m.addHeartbeatMessageToMap(...) — calls storer.SavePubkeyData and storer.SaveKeysgo m.computeAllHeartbeatMessages() — its (now inline) transition save also runs on this untracked goroutine
Neither is WaitGroup-tracked nor stopCh-gated, so a heartbeat arriving just before shutdown can still write to the storer concurrently with — or after — Close() returns. Impact is low (a benign extra state write during a shutdown window; storer errors are logged and swallowed), and it cannot affect the deflaked tests, which drive the monitor synchronously.
Task
Make Close() a true quiescence point for all persistence paths:
- Track or gate the message-driven goroutines so
Close() awaits them. Note the naive fix is incorrect: wg.Add(1) from an untracked spawn can race wg.Wait() (Add-when-counter-zero-during-Wait is WaitGroup misuse). A guarded spawn helper (e.g. mutex/state check that refuses new work once closing, then Add before spawn) or a stopCh check before each spawn plus join is needed.
- Remove the corresponding caveat from the
Close() doc comment once done.
- Cover with a shutdown test: heartbeat processed concurrently with
Close() must not write to the storer after Close() returns.
Context
Raised during review of #119 (by Copilot and in review comment R2 on
monitor.go). Pre-existing behavior, unchanged by that PR.Monitor.Close()is a quiescence point for the refresh loop and its in-flight state saves (established in #119), butProcessReceivedMessagespawns two untracked goroutines per accepted heartbeat message:go m.addHeartbeatMessageToMap(...)— callsstorer.SavePubkeyDataandstorer.SaveKeysgo m.computeAllHeartbeatMessages()— its (now inline) transition save also runs on this untracked goroutineNeither is WaitGroup-tracked nor
stopCh-gated, so a heartbeat arriving just before shutdown can still write to the storer concurrently with — or after —Close()returns. Impact is low (a benign extra state write during a shutdown window; storer errors are logged and swallowed), and it cannot affect the deflaked tests, which drive the monitor synchronously.Task
Make
Close()a true quiescence point for all persistence paths:Close()awaits them. Note the naive fix is incorrect:wg.Add(1)from an untracked spawn can racewg.Wait()(Add-when-counter-zero-during-Wait is WaitGroup misuse). A guarded spawn helper (e.g. mutex/state check that refuses new work once closing, then Add before spawn) or astopChcheck before each spawn plus join is needed.Close()doc comment once done.Close()must not write to the storer afterClose()returns.