[FumingPower3925]

driver/postgres: TestSyncFallThroughAfterTheCapKeepsOrderAndTheError1104 raced on its own feeder, found on the macOS runner of #1151. A TEST race, not a product one. Refs #1147 Refs #1104

The failure

CI run 40619726636, job 122590552041, required check "Portable fallback (macos-latest)", head 9a96459e of #1151 (green on its rerun):

WARNING: DATA RACE
Write at 0x00c0001a8000 by goroutine 49840:
  protocol.(*Reader).Feed()   message.go:100
  pgConn.onRecv()             conn.go:1299
  TestSyncFallThrough...1104.func3.1()   sync_fallthrough_1104_test.go:80      <- the test's feeder goroutine
Previous read at 0x00c0001a8000 by goroutine 49839:
  protocol.(*Reader).Feed()
  pgConn.onRecv()
  partialSync.WriteAndPoll()             sync_fallthrough_1104_test.go:38      <- the poll, still delivering its part
  pgConn.pollSync() / doExtendedQuery()
...
sync_fallthrough_1104_test.go:86: query: postgres/protocol: unexpected DataRow in extended query (phase=0)
--- FAIL: TestSyncFallThroughAfterTheCapKeepsOrderAndTheError1104/doExtendedQuery

Cause

The fake event loop of the test (a goroutine) feeds the rest of the reply "once the caller has fallen through", which it decided by pending[0].syncMode == false. But a request is put on pending BEFORE the query sets syncMode to true (enqueue, then req.syncMode.Store(true)), so for a moment the request is queued with the flag still false: the feeder saw "fell through" before the poll had even started, and fed the rest while the poll's own onRecv was still running. protocol.Reader is single-threaded (the real worker guarantees it), so that is a data race on the reader's buffer inside the test, and when the rest got in first the extended-query parser saw a DataRow before ParseComplete and BindComplete: "unexpected DataRow in extended query (phase=0)". Not the product path: in the product the poll and the event loop's dispatch never overlap on a conn (the loop leaves the fd to the poll while it runs), which is also why the product fix of #1148 holds; the race exists only in the test's fake. The previous fix of this test (a timer, then the flag) had the same hole.

Fix

partialSync closes a polled channel after it delivered its part; the feeder waits for it (20 s guard), and only then for the request to have left the sync mode (the caller clears it right after the poll returns). No product change.

Evidence

Main (3d217a6), the unchanged test:

	result
native darwin/arm64, -race -count=100	11 of 100 FAIL (10 simpleQuery with a DATA RACE, 1 doExtendedQuery with the exact CI message unexpected DataRow in extended query (phase=0))
docker --cpus 2, six busy loops, -race -count=100	2 of the first 40 iterations FAIL (simpleQuery, DATA RACE; 7 race reports)

This PR:

	result
native darwin/arm64, -race -count=100	100 of 100 pass, both query kinds
docker --cpus 2, six busy loops, -race -count=50	50 of 50 pass
docker --cpus 2, -race -count=50, all six 1104 tests	50 of 50 pass each
mutant: the wait for the poll removed (the old gate)	10 of 100 FAIL, 8 with unexpected DataRow in extended query (phase=0)
mutant: the #1148 guard put back inside if syncMode	5 of 5 FAIL: the test still guards the product fix

golangci-lint (CI pin v2.14): 0 issues.

Release note

None (tests only).

[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!