merger-gen: one generation sequence per merger, not per entry - #290
Merged
Conversation
Merger.gen answered "a timer that has begun firing cannot be stopped" for a settler reset and not for the same race across Flush. It counted per ENTRY and a fresh entry started at zero, while a coalescer entry is never reset — so its stranded callback always held zero too, and the two collided: a key's timer begins firing (stop returns false); Flush detaches and emits that entry; the same key is Added again at generation zero; the stranded callback matches the new entry and fires it a window early. The number now comes from a monotonic sequence on the Merger, so no two armed timers can ever share one and a stranded callback can match nothing but the timer it was armed for. That subsumes the settler case rather than replacing it — the reset path takes its number from the same sequence. The consequence was mild: one notification arrived early, which the bus contract already makes safe, since consumers re-read authoritative state instead of treating an event as a log. Fixed because the mechanism that prevents it was already there, one level too narrow. Both races are pinned, driven by a hand-run timer rather than a clock — the interleaving is the subject, and a sleep would test the scheduler instead. Verified the new case fails on the previous code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The GAP recorded here a round ago is closed, so the entry states the rule: the sequence lives on the Merger, so no two armed timers share a number and a stranded callback can match nothing but the timer it was armed for. The account of the collision stays, because it is what explains why the counter is not on the entry — the shape a reader would otherwise reintroduce. Co-Authored-By: Claude Opus 5 (1M context) <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
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.
Fixes the defect recorded in
docs/modules/config.mdby the 2026-08-10 tidypass over
internal/event.Merger.genanswered "a timer that has begun firing cannot be stopped" for asettler reset and not for the same race across
Flush. It counted per entryand a fresh entry started at zero, while a coalescer entry is never reset — so
its stranded callback always held zero too, and the two collided: a key's timer
begins firing,
Flushdetaches and emits that entry, the same key isAddedagain at generation zero, and the stranded callback matches the new entry and
fires it a window early.
The number now comes from a monotonic sequence on the
Merger, so a strandedcallback can match nothing but the timer it was armed for. That subsumes
the settler case rather than replacing it — the reset path draws from the same
sequence.
The consequence was mild and the commit says so: one notification arrived
early, which the bus contract already makes safe. It is fixed because the
mechanism that prevents it was already there, applied one level too narrowly.
hand-run timer rather than a clock — the interleaving is the subject,
and a sleep would test the scheduler instead. Verified the new case
fails on the previous code.
docs/modules/config.md: the GAP entry becomes the invariant, keepingthe account of the collision because it is what explains why the counter
is not on the entry.
One note on the test, since it cost a cycle: a fake timer that keeps only the
latest callback hands you the second entry's timer, not the stranded one.
The fake collects callbacks in arming order and the test calls
armed[0].Verification:
make cigreen.