[FumingPower3925]

Flakes lane, package H: RegisterConnWait can make a second RegisterConn call for a ctx that was done before the call. A real race in the code (#1129), found as a CI failure of #1137. Refs #1104

The bug

RegisterConnWait waits out the io_uring engine's "fd already registered" refusal: after the first refusal it pauses (100 us, doubling) and calls loop.RegisterConn again. The pause is

t := time.NewTimer(pause)
select {
case <-ctx.Done(): return ...refusal wrapped with ctx.Err()
case <-t.C:
}
// second RegisterConn call

When the goroutine is not run for the whole pause, both channels are ready when the select runs, and Go picks one at random. So a ctx that was done before the call is followed by another RegisterConn about half of the time in that case: an fd registered with the engine for a caller that gave up. The documented contract is that a done ctx stops the wait.

Found in #1137's CI, required check "Portable fallback (macos-latest)", https://github.com/goceleris/celeris/actions/runs/40485540133 (job 122126282588): TestRegisterConnWait887/ctx_done_returns_the_refusal failed with register_wait_887_test.go:78: RegisterConn calls = 2, want 1. It is not a test flake: the test is right and the code is wrong.

The fix (commit 2)

After the select, whichever case fired, look at ctx.Err() and return the same wrapped refusal (%w (gave up waiting: %w) with the ctx error) before calling RegisterConn again. The budget path needs no such priority: it is checked after a call, and the call at the deadline is the last try by design. The doc of RegisterConnWait says what holds: a ctx that is done when a pause ends stops the wait before the next RegisterConn call, and a ctx that was already done on entry still gets the first call (the wait is only for a refusal of it, and the drivers close the fd on a failed handshake). A ctx cancelled in the few nanoseconds between the check and the call is followed by that one call; that is inherent.

The test (commit 1, fails on its own)

A deterministic version of the race needs the timer to be ready when the select runs, which the scheduler decides. registerConnWait is RegisterConnWait with the first pause as a parameter (RegisterConnWait passes the 100 us, behaviour unchanged); the test passes 0, so the timer is ready at the select, and checks 2000 waits with a ctx cancelled before the call that no wait made a second call. Two more cases (commit 3): a ctx cancelled DURING the wait (the scripted loop's first refusal cancels it, pause 0; 2000 waits, exactly one call each, errors.Is holds for both the refusal and context.Canceled), and a ctx whose deadline has passed (context.DeadlineExceeded, 200 waits).

golang:1.27.3, --cpus 2, -race -count=20:

	result
commit 1 (seam and test, no fix)	20 of 20 runs FAIL, 994 to 1025 of 2000 waits made the second call per run (about half, as a random pick has it)
commit 2 (the fix)	20 of 20 pass; TestRegisterConnWait887 (5 cases) and TestIsNumberBusy887 pass 20 of 20 too; the whole driver/internal/eventloop package passes once with -race
commit 3's two new tests on commit 1 (no fix)	both FAIL 20 of 20
commit 3 (all 6 register-wait tests)	20 of 20 pass, -race -count=20; the whole package passes once with -race

The same pattern elsewhere in driver/ and internal/

I grepped the non-test code for select cases on a timer or ticker channel (<-t.C, <-ticker.C, <-time.After) and read each one for a side effect that follows a done context or close channel that was ready too:

• driver/redis/pubsub.go:556 (reconnect backoff vs closeCh) and driver/redis/sentinel.go:264 (subscribeLoop): same shape, already safe: ps.closed.Load() / s.closed.Load() is checked after the select / at the top of the loop before anything is dialled.
• driver/redis/sentinel.go:476-481 (handleSwitchMaster, retry backoff vs closeCh): same shape and NOT guarded: if closeCh is closed and the timer is ready too, the loop dials the master again after Close (at most twice more, maxRetries is 3). It also has a second defect with no select in it: after the loop, s.client = newClient (line 487) has no closed check, so a Close that lands while dialMaster runs (it uses its own timeout, not closeCh) leaves a live client installed on a closed SentinelClient that nothing closes. A select priority alone would not fix that. driver/redis is in this module; I left it out of this PR for scope: the fix is a closed check under s.mu before the install, a different change in a different package. Read, not reproduced here; it is fixed in driver/redis: SentinelClient and ClusterClient no longer leak or panic when closed during a failover or a refresh #1141.
• driver/memcached/cluster.go:480 (probe ticker vs ctx.Done()): a probe after a done ctx runs with context.WithTimeout(ctx, ...), so it fails at once; no side effect.
• driver/redis/cluster.go:465 (60 s refresh ticker vs closeCh): the select race itself has no side effect: after Close, closeNodes has set c.nodes = nil, anyNode() returns nil and refreshTopology returns "no live nodes". A separate latent defect, read and not reproduced: if Do(CLUSTER SLOTS) finished before Close and Close runs closeNodes before refreshTopology takes c.mu, the write c.nodes[addr] = n (cluster.go ~381, ~408) goes into a nil map and panics. Also fixed in driver/redis: SentinelClient and ClusterClient no longer leak or panic when closed during a failover or a refresh #1141 (the refresh and the redirect writer).
• internal/engine/std/engine.go:355 (h2 stream poll vs ctx.Done()): the step after the select is closeH2Streams, a compare-and-swap that is the shutdown's own action; no harm if it runs after the budget.
• internal/adaptive/engine.go:629 and :811 (bind poll loops), internal/deferlinger/deferlinger.go:341, driver/postgres/conn.go:1273, 3067, 3141 (30 s timeouts that fail the connection, with doneCh as the other case), internal/recvtheft (test hooks): no side effect after a ready timer that a ready second case should have prevented.

So: one more site of the same shape with a real defect (sentinel.go:477), whose fix is different, listed and not changed.

Release note

None user-visible beyond: a driver dial on the io_uring engine whose ctx is done no longer makes a further registration attempt.

[coderabbitai]

Caution

Review failed

The pull request is closed.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!