fix(eic-tunnel-pool): capture poolJanitorInterval at pool construction
Some checks failed
CI / Canvas Deploy Reminder (pull_request) Has been skipped
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 2m11s
CI / Canvas (Next.js) (pull_request) Successful in 14s
CI / Python Lint & Test (pull_request) Successful in 8s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 13s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 26s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 10s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Failing after 6m9s
Harness Replays / Harness Replays (pull_request) Failing after 1m17s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 18s
branch-protection drift check / Branch protection drift (pull_request) Successful in 16s
CodeQL / Analyze (${{ matrix.language }}) (go) (pull_request) Successful in 8s
CodeQL / Analyze (${{ matrix.language }}) (javascript-typescript) (pull_request) Successful in 8s
Check merge_group trigger on required workflows / Required workflows have merge_group trigger (pull_request) Successful in 14s
CodeQL / Analyze (${{ matrix.language }}) (python) (pull_request) Successful in 9s
pr-guards / disable-auto-merge-on-push (pull_request) Failing after 9s
CI / Detect changes (pull_request) Successful in 23s
E2E API Smoke Test / detect-changes (pull_request) Successful in 22s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 23s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 23s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 20s
Harness Replays / detect-changes (pull_request) Successful in 23s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 20s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 23s
Ops Scripts Tests / Ops scripts (unittest) (pull_request) Successful in 1m0s
CI / Platform (Go) (pull_request) Successful in 12m26s

Closes the chronic -race flake on TestPooledWithEICTunnel_PanicPoisonsEntry
and the handlers package as a whole (CI / Platform (Go) was intermittent
on staging, ~50% red on workspace-server-touching commits since 2026-04).

The race: tests swap the package-level poolJanitorInterval via t.Cleanup
(eic_tunnel_pool_test.go:61) AFTER an earlier test caused the global pool's
janitor goroutine to start. The janitor loops on time.NewTicker(poolJanitorInterval)
on every tick — so the cleanup write races the goroutine read for the rest
of the process. Caught locally + on PR #84's CI run on Gitea.

Fix: capture the interval as a field on eicTunnelPool at newEICTunnelPool().
The janitor now reads p.janitorInterval, which never changes after construction.
Tests that override poolJanitorInterval before freshPool() still get the new
value (they set the package var before construction). The global pool's
janitor — created lazily once via sync.Once on first getEICTunnelPool() —
is now immune to t.Cleanup-driven swaps from later tests.

Surfaced while verifying #84 (SaaS plugin install via EIC SSH); folded
into this PR per the "fix root not symptom" rule rather than merging
around a chronic-red CI signal.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent 16868c4ec1
commit b664691051

View File

@ -108,6 +108,18 @@ type eicTunnelPool struct {
// First acquirer takes the slot; later ones wait on the channel. // First acquirer takes the slot; later ones wait on the channel.
pendingSetups map[string]chan struct{} pendingSetups map[string]chan struct{}
stopJanitor chan struct{} stopJanitor chan struct{}
// janitorInterval is captured at pool construction from the
// package-level poolJanitorInterval var. Captured (not re-read on
// every tick) so a test that swaps the package var via t.Cleanup
// after a global pool's janitor is already running can't race
// with that goroutine's ticker read. The global pool is created
// lazily once per process via sync.Once; before this capture
// landed, every test that touched poolJanitorInterval after the
// global pool's first-touch raced the janitor (caught by -race
// on staging tip 249dbc6a — TestPooledWithEICTunnel_PanicPoisonsEntry).
// Tests still get the new value on a freshPool() because they
// set the package var BEFORE calling newEICTunnelPool().
janitorInterval time.Duration
} }
var ( var (
@ -127,11 +139,16 @@ func getEICTunnelPool() *eicTunnelPool {
// newEICTunnelPool constructs an empty pool. Exported so tests can // newEICTunnelPool constructs an empty pool. Exported so tests can
// build isolated pools without sharing the singleton. // build isolated pools without sharing the singleton.
//
// Captures poolJanitorInterval at construction time so the janitor
// goroutine doesn't race with t.Cleanup-driven swaps of the package
// var. See the janitorInterval field comment for the failure mode.
func newEICTunnelPool() *eicTunnelPool { func newEICTunnelPool() *eicTunnelPool {
return &eicTunnelPool{ return &eicTunnelPool{
entries: map[string]*pooledTunnel{}, entries: map[string]*pooledTunnel{},
pendingSetups: map[string]chan struct{}{}, pendingSetups: map[string]chan struct{}{},
stopJanitor: make(chan struct{}), stopJanitor: make(chan struct{}),
janitorInterval: poolJanitorInterval,
} }
} }
@ -290,8 +307,11 @@ func (p *eicTunnelPool) evictLRUIfFullLocked(skipInstance string) {
// janitor periodically scans for entries that are idle AND expired, // janitor periodically scans for entries that are idle AND expired,
// closing their tunnels. Runs forever (per pool lifetime); cancelled // closing their tunnels. Runs forever (per pool lifetime); cancelled
// by close(p.stopJanitor) for tests that build short-lived pools. // by close(p.stopJanitor) for tests that build short-lived pools.
//
// Reads p.janitorInterval (captured at construction) instead of the
// package-level poolJanitorInterval — see janitorInterval field comment.
func (p *eicTunnelPool) janitor() { func (p *eicTunnelPool) janitor() {
t := time.NewTicker(poolJanitorInterval) t := time.NewTicker(p.janitorInterval)
defer t.Stop() defer t.Stop()
for { for {
select { select {