Skip to content

Assert no enrichment pass is active instead of polling the caller's defer - #533

Merged
zzet merged 1 commit into
mainfrom
fix/wait-for-the-context-enrich-pass-to-stop
Aug 10, 2026
Merged

Assert no enrichment pass is active instead of polling the caller's defer#533
zzet merged 1 commit into
mainfrom
fix/wait-for-the-context-enrich-pass-to-stop

Conversation

@zzet

@zzet zzet commented Aug 10, 2026

Copy link
Copy Markdown
Owner

What broke

TestManager_CloseCancelsContextProviderAndWaitsForPass failed on main (macOS runner, run 31365875121):

--- FAIL: TestManager_CloseCancelsContextProviderAndWaitsForPass (0.00s)
    enrich_deadline_test.go:421: Close returned before the complete manager-owned pass stopped

Why

The test polled enrichDone without blocking. That channel is closed by a defer in the test's own goroutine, which runs after runEnrichOne's deferred endPass releases the activePasses gate Close waits on. Nothing orders that goroutine's remaining instructions against Close returning in the test goroutine, so a runner that had not yet rescheduled it hit default and failed.

This is a test-timing defect, not a product bug: runEnrichOne drains its provider goroutine and writes its terminal status before endPass, so Close's guarantee — no admitted pass can still mutate the graph — holds. TestManager_CloseStopsLegacyProviderBeforeReturning had the identical defect and was fixed in df855b9; this sibling was missed.

Reproduced locally at 2/1500 (-race, full GOMAXPROCS, cores saturated), same line and message. It does not reproduce at GOMAXPROCS=1 — the pass goroutine keeps its P after Done() and finishes unwinding.

The fix

Assert the invariant Close actually provides: no pass may be running or draining the instant Close returns. The terminal setEnrichStatus happens-before endPass, so EnrichmentActive() is ordered by Close's own wait. The enrichDone handshake stays, as a bounded wait, so a pass that genuinely never stops still fails.

The replacement is a stronger guard, not just a quieter one. With m.activePasses.Wait() removed from Close:

assertion caught the regression
old non-blocking enrichDone poll 11/25
new EnrichmentActive() assertion 19/25

Verification

  • 2000× -run TestManager_Close -race under CPU saturation — clean (old code failed at 1500)
  • go test -race ./internal/semantic/... — all 5 packages green
  • golangci-lint run ./internal/semantic/... — no issues

…r's defer

TestManager_CloseCancelsContextProviderAndWaitsForPass polled enrichDone
without blocking, but that channel is closed by a deferred close in the
test's own goroutine, which runs after the pass releases the active-pass
gate Close waits on. Nothing orders that goroutine against Close's return,
so a loaded runner that had not yet rescheduled it failed the assertion —
the macOS CI failure, reproduced locally at 2/1500 under CPU saturation.

Assert the invariant Close actually provides instead: the pass writes its
terminal enrichment status before releasing the gate, so no pass may be
running or draining the instant Close returns. That check is ordered by
Close's own wait, and it catches a Close that stops waiting more often
than the poll it replaces (19/25 vs 11/25 with the wait removed). The
enrichDone handshake stays as a bounded wait, so a pass that genuinely
never stops still fails the test.
@zzet
zzet merged commit fe48f16 into main Aug 10, 2026
10 checks passed
@zzet
zzet deleted the fix/wait-for-the-context-enrich-pass-to-stop branch August 15, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant