Follow-up from #692: a Shutdown that races Start*Context before the callsBefore load can run OnShutdown hooks twice

[FumingPower3925]

Follow-up from PR #692 (#673), left open under the maintainer's two-round review cap (2026-09-27). The round-2 independent review of #692 approved with no blocker or major. Its minors were handled before the merge: docs#73 rewrites graceful-shutdown.md, #692 carries the breaking label, and CodeRabbit reviewed 5a05c2e with no actionable comments. One nit is left, and it is tracked here.

A narrow window can still run the OnShutdown hooks twice.

• In listenUntilCancelled (server.go:858 at 5a05c2e), callsBefore := s.shutdownCalls.Load() runs after the caller's prepare has published the engine.
• A direct Server.Shutdown that races Start*Context between doPrepare and that load is counted as "before". It runs the hooks, because the engine is already set.
• If the caller then cancels ctx, the watcher sees shutdownCalls == callsBefore and calls Shutdown again, so every hook runs a second time.
• Moving the load before prepare is worse: a Shutdown that comes before Start would then suppress the watcher's Shutdown and leak the settle re-opener (adaptive dispatch never re-times a settled route: a store-backed handler that turns slow runs inline on the engine worker forever (#493 item 4, measured) #592). The current placement is defensible.

What to do: either close the window (for example, take the snapshot under the same publication step that sets the engine), or add one sentence to the comment at server.go:865-869 that names the window and why the load sits where it does. The #703 lane edits the same function (it reuses #692's listen/watcher join), so it can pick this up.

Found by: the lane E round-2 review of #692 (nit), evidence root evidence/celeris-673-679-653-424/lane-20260926/673/.

[FumingPower3925]

Closing: this no longer holds on main (dccb839). The window was real at #692's review head, and #692 closed it before it merged.

Why. At 5a05c2e, listenUntilCancelled took callsBefore := s.shutdownCalls.Load() after prepare, and the watcher shut down only if shutdownCalls had not moved since. A direct Shutdown between those two points counted as "before", so a later cancel ran Shutdown, and every hook, a second time. Commit a20af40 ("one claim decides who shuts down on a cancel") replaced the counter before the merge (fee0d1c): a direct Shutdown sets directShutdown under lifecycleMu before it runs anything, and the watcher shuts down only if, under the same lock, neither directShutdown nor its own claim is set. There is no snapshot left, so there is no window: whichever of the two claims first runs the hooks, and the other does not.

Measured. TestShutdownBeforeTheWatcherLoadsRunsHooksOnce forces exactly the #728 order: the engine is published (as doPrepare does), a direct Shutdown latches before the Start*Context path begins, Listen is held in its teardown after its context is cancelled so the watcher is still waiting, then the caller cancels; the hook count is read after the Start path, which waits for its watcher, has returned. -race -count=20 per tree, one container, linux/arm64, CI shape:

tree	what it is	runs where the hook ran twice
5a05c2e	#692's reviewed head (the callsBefore snapshot)	20/20
f17c04a	the last #692 commit with the snapshot	20/20
a20af40	the #692 commit that replaced it with the claim	0/20
dccb839	main (server.go identical to a20af40's: same sha256)	0/20
f7685c3	the #703 PR branch (the test is unchanged at its head 76c205c)	0/20

(728/verify-20260927T183220Z.log: 40 FAIL lines reading "the OnShutdown hook ran 2 times, want 1", all in the first two arms; no SKIP line.)

Guard. The test is committed in the #703 PR (#746), which changes the same path (the direct Shutdown now waits for the drain), so the order stays pinned.

Evidence: evidence/lanes-20260927/LIFECYCLE/728/ in the maintainer's probatorium evidence root (verify.sh and its log; the test and its two engine-install helpers are in overlay/).

[FumingPower3925]

The guard is now on main: #746 merged as dfd044f, with TestShutdownBeforeTheWatcherLoadsRunsHooksOnce (shutdown_race_hooks_once_test.go) in the root package.

This issue stays closed as completed. #692 fixed it in its final form: a20af40 replaced the callsBefore snapshot with the directShutdown claim before #692 merged as fee0d1c. The evidence is 728/verify.sh and 728/verify-20260927T183220Z.log under evidence/lanes-20260927/LIFECYCLE/. The test forces the #728 order and runs -race -count=20 per tree, in one container. The hook ran twice in 20/20 runs at 5a05c2e and 20/20 at f17c04a: 40 FAIL lines, all reading "the OnShutdown hook ran 2 times, want 1". It ran once in 20/20 runs at each of a20af40, main dccb839 and #746's branch f7685c3. server.go has the same sha256 at a20af40 and at dccb839.

After the merge, CI on #746's merged head 859d843 (run 36373856404) had the root package green. It ran with this test.