Fix #72: fan-out drain/end race #81

Merged
crueber merged 1 commit from fix/issue-72 into main 2026-09-05 00:35:39 +00:00
Owner

Closes #72 (corroborated dropped-work race in internal/notify/tasks.go — silent notification loss).

Two-sided close, both in internal/notify/tasks.go:

  • Leader ends ONLY via new endIfQuiescent: re-checks e.seqs and removes the task atomically (table lock nests entry lock — the only nested acquisition; order always table→entry; no lock held across store/network calls). Refusal means seqs pending → drain again.
  • Joiner re-checks current() after attaching in enqueueFanout and re-enqueues onto the live entry on a miss (seq re-attached there; orphaned copy never drains twice; any double drain is idempotent via deterministic ids + Create-412).

Neither half alone suffices (documented in the Decisions entry): a re-checking end still loses an attach between check and removal; a re-verifying joiner still loses to a bare end.

Regression tests (internal/notify/tasks_test.go):

  • TestFanoutTerminalDrainKeepsLateSeq: deterministic script of the exact issue interleaving (terminal drain empty → late join+attach → end must refuse → leftover drains → quiescent end). FAILS pre-fix (build failure: pins the new contract), green -race -count=20 post-fix.
  • TestFanoutConcurrentEnqueueLosesNothing: 60 seqs x 6 concurrent enqueues through the real enqueueFanout/drainFanout paths; all 60 notifications must land.

Corroboration: throwaway old-API script on pre-fix code showed stillRunning=false with orphanedSeqs=[7] — the exact loss.

Verification: gofmt clean, go vet clean, full package go test -race green, coverage 97.2% (gate 95%; all touched funcs 100%). Doc Decisions entry appended in docs/features/06_notifications.md (law 12); package Concurrency note in notify.go updated.

Closes #72 (corroborated dropped-work race in internal/notify/tasks.go — silent notification loss). Two-sided close, both in internal/notify/tasks.go: - Leader ends ONLY via new endIfQuiescent: re-checks e.seqs and removes the task atomically (table lock nests entry lock — the only nested acquisition; order always table→entry; no lock held across store/network calls). Refusal means seqs pending → drain again. - Joiner re-checks current() after attaching in enqueueFanout and re-enqueues onto the live entry on a miss (seq re-attached there; orphaned copy never drains twice; any double drain is idempotent via deterministic ids + Create-412). Neither half alone suffices (documented in the Decisions entry): a re-checking end still loses an attach between check and removal; a re-verifying joiner still loses to a bare end. Regression tests (internal/notify/tasks_test.go): - TestFanoutTerminalDrainKeepsLateSeq: deterministic script of the exact issue interleaving (terminal drain empty → late join+attach → end must refuse → leftover drains → quiescent end). FAILS pre-fix (build failure: pins the new contract), green -race -count=20 post-fix. - TestFanoutConcurrentEnqueueLosesNothing: 60 seqs x 6 concurrent enqueues through the real enqueueFanout/drainFanout paths; all 60 notifications must land. Corroboration: throwaway old-API script on pre-fix code showed stillRunning=false with orphanedSeqs=[7] — the exact loss. Verification: gofmt clean, go vet clean, full package go test -race green, coverage 97.2% (gate 95%; all touched funcs 100%). Doc Decisions entry appended in docs/features/06_notifications.md (law 12); package Concurrency note in notify.go updated.
drainFanout ended via bare end() without re-checking e.seqs, so a seq
attached by a concurrent enqueueFanout between the terminal drain and
the removal orphaned on a detached entry while the joiner started no
worker (silent notification loss). Two-sided close: leader ends only
via endIfQuiescent (drain-then-end atomic under table->entry lock
order, refuses while seqs pending); joiner re-checks current() after
attaching and re-enqueues on a miss. Regression tests:
TestFanoutTerminalDrainKeepsLateSeq (deterministic interleaving pin)
+ TestFanoutConcurrentEnqueueLosesNothing (real-path hammer).
Doc Decisions entry in 06 (law 12).
Sign in to join this conversation.
No description provided.