Summary
scan.Run sends on the outbound �vents channel from two places that are not guarded against a consumer that stops draining the channel (e.g. a future consumer that returns early on cancellation instead of draining to close). Today this is latent and harmless because every existing consumer drains to close, but it is a trap for anyone who later adds a consumer that does not.
This was discovered while scoping a select { case events <- ...: case <-ctx.Done(): } guard for the result send: writing the failing test that the guard would need exposed that guarding only the result send is an incomplete fix.
The two unguarded sends
-
EventResult in processJob (internal/scan/runner.go):
go atomic.AddInt64(found, 1) events <- Event{Kind: EventResult, Domain: j.domain, Records: records}
-
EventDone at the end of Run (internal/scan/runner.go):
go events <- Event{ Kind: EventDone, Processed: atomic.LoadInt64(&processed), Total: atomic.LoadInt64(&total), Found: atomic.LoadInt64(&found), }
By contrast, the completed send and the progress-ticker send are both wrapped in select { ... case <-ctx.Done(): }, which is the asymmetry that makes these two stand out.
Why guarding only EventResult is insufficient
If only the EventResult send is guarded, a non-draining consumer that cancels would no longer stall workers on the result send, but wg.Wait() then returns, the ticker stops, and Run blocks on the unguarded EventDone send instead. The stall is relocated, not removed. A complete fix must guard both sends.
Why this is not a trivial change
Guarding the EventDone send changes delivery semantics on cancellation: today EventDone is emitted even on user abort (the runner drains and emits it with partial counts), and the TUI relies on that to render the done/aborted status line and to finalize structured output via finalizeOutput(true). A guard that drops EventDone on ctx.Done() would route user aborts through the channel-closed path instead, which is a real behavioral change to the concurrency core, not insurance.
Recommended resolution (when a non-draining consumer exists)
- Guard both the
EventResult and EventDone sends together.
- Decide and document the cancellation semantics for
EventDone (drop-on-cancel vs always-emit) and update the TUI accordingly.
- Add a runner test that exercises a consumer which cancels and stops reading: it must hang/time out without the guard and pass with it. Without that test the change is unverifiable on exactly the file we least want to touch on faith.
Current status
Deferred intentionally. runner.go is left untouched until a consumer that stops draining actually exists. Tracked here so the reasoning survives independent of docs/ROADMAP.md and is searchable when someone adds such a consumer.
Summary
scan.Run sends on the outbound �vents channel from two places that are not guarded against a consumer that stops draining the channel (e.g. a future consumer that returns early on cancellation instead of draining to close). Today this is latent and harmless because every existing consumer drains to close, but it is a trap for anyone who later adds a consumer that does not.
This was discovered while scoping a
select { case events <- ...: case <-ctx.Done(): }guard for the result send: writing the failing test that the guard would need exposed that guarding only the result send is an incomplete fix.The two unguarded sends
EventResultinprocessJob(internal/scan/runner.go):go atomic.AddInt64(found, 1) events <- Event{Kind: EventResult, Domain: j.domain, Records: records}EventDoneat the end ofRun(internal/scan/runner.go):go events <- Event{ Kind: EventDone, Processed: atomic.LoadInt64(&processed), Total: atomic.LoadInt64(&total), Found: atomic.LoadInt64(&found), }By contrast, the
completedsend and the progress-ticker send are both wrapped inselect { ... case <-ctx.Done(): }, which is the asymmetry that makes these two stand out.Why guarding only EventResult is insufficient
If only the
EventResultsend is guarded, a non-draining consumer that cancels would no longer stall workers on the result send, butwg.Wait()then returns, the ticker stops, andRunblocks on the unguardedEventDonesend instead. The stall is relocated, not removed. A complete fix must guard both sends.Why this is not a trivial change
Guarding the
EventDonesend changes delivery semantics on cancellation: todayEventDoneis emitted even on user abort (the runner drains and emits it with partial counts), and the TUI relies on that to render the done/aborted status line and to finalize structured output viafinalizeOutput(true). A guard that dropsEventDoneonctx.Done()would route user aborts through the channel-closed path instead, which is a real behavioral change to the concurrency core, not insurance.Recommended resolution (when a non-draining consumer exists)
EventResultandEventDonesends together.EventDone(drop-on-cancel vs always-emit) and update the TUI accordingly.Current status
Deferred intentionally.
runner.gois left untouched until a consumer that stops draining actually exists. Tracked here so the reasoning survives independent ofdocs/ROADMAP.mdand is searchable when someone adds such a consumer.