test(storage): fix flaky minimemcached nil-pointer panic#4301
Conversation
The minimemcached test server spawns a goroutine that calls Accept() on its listener, while Close() nils that listener. On very short tests the cleanup's Close() could run before the goroutine entered Accept(), dereferencing a nil listener inside the dependency and crashing the test binary with a panic that no test can recover (it runs in a foreign goroutine). Block in the test helper until the server answers a probe connection, guaranteeing the accept loop is parked in Accept() before the test proceeds. Close() then unblocks it cleanly instead of racing startup. Assisted by AI
|
Coverage Impact ⬆️ Merging this pull request will increase total coverage on 🛟 Help
|
Reference daangn/minimemcached#14 (fix in PR #13) so the workaround can be removed once an upstream release ships the fix. Assisted by AI
stevenvegt
left a comment
stevenvegt
left a comment
There was a problem hiding this comment.
Remove this workaround once a minimemcached release includes the upstream fix.
How do we know this?
We don't, I'm subscribed to the issue, so I'll get a notification. And we link to it in the source code, + we get dependency updates through dependabot (so check changelog). |
2 participants
Problem
The
storagetest job intermittently fails with a panic, crashing the whole test binary:Root cause
minimemcached.Runsynchronously creates the listener, then spawns a goroutine that loops onm.l.Accept().Close()(called fromt.Cleanup) sets that listener tonil. On very short tests the cleanup'sClose()can run before the goroutine reachesAccept(), som.l.Accept()dereferences a nil listener. The panic happens in a dependency-spawned goroutine, so no test can recover it — the binary dies.This is an upstream bug; v1.2.0 is the latest release, so there's no version to bump to.
Fix
Block in the test helper until the server actually answers a probe connection (
version\r\n). That guaranteesserve()is parked insideAccept()before the test proceeds, so the cleanupClose()unblocks it vialistener.Close()cleanly instead of racing goroutine startup.Verification
go test ./storage/ -run Memcached -count=10 -racepasses.