From f1a094233fdd5321ebee4a69007f13a1f5e6d97a Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Tue, 2 Jun 2026 10:38:27 +0200 Subject: [PATCH 1/2] test(storage): fix flaky minimemcached nil-pointer panic 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 --- storage/session_memcached_test.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/storage/session_memcached_test.go b/storage/session_memcached_test.go index b5ce5964ed..7535f66d7d 100644 --- a/storage/session_memcached_test.go +++ b/storage/session_memcached_test.go @@ -170,11 +170,25 @@ func memcachedTestServer(t *testing.T) *minimemcached.MiniMemcached { if err != nil { t.Fatal(err) } + // Wait until the server's accept loop is actually serving connections before returning. + // minimemcached.Run spawns a goroutine that calls Accept() on the listener; Close() nils that + // listener. On really short tests Close() (in the cleanup below) could run before the goroutine + // entered Accept(), causing a nil pointer dereference in the dependency. Probing the server until + // it answers guarantees the accept loop is parked in Accept(), so Close() unblocks it cleanly. + require.Eventually(t, func() bool { + conn, err := net.DialTimeout("tcp", fmt.Sprintf("localhost:%d", m.Port()), time.Second) + if err != nil { + return false + } + defer conn.Close() + if _, err = conn.Write([]byte("version\r\n")); err != nil { + return false + } + buf := make([]byte, 1) + _, err = conn.Read(buf) + return err == nil + }, 5*time.Second, 10*time.Millisecond, "memcached server did not start serving") t.Cleanup(func() { - // Note on DATA RACE - // minimemcached.Run creates a new go routine to listen for new connections. - // In certain cases the new go routine may be created after/simultaneous with this cleanup resulting in a data race / nil pointer dereference. - // Mostly happens on CI during really short tests. m.Close() }) return m From 86636a319c09d7f0eaff7d971c1680cd8045302d Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Tue, 2 Jun 2026 11:01:36 +0200 Subject: [PATCH 2/2] test(storage): link upstream issue for the minimemcached workaround Reference https://github.com/daangn/minimemcached/issues/14 (fix in PR #13) so the workaround can be removed once an upstream release ships the fix. Assisted by AI --- storage/session_memcached_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/storage/session_memcached_test.go b/storage/session_memcached_test.go index 7535f66d7d..342e9f188d 100644 --- a/storage/session_memcached_test.go +++ b/storage/session_memcached_test.go @@ -170,11 +170,13 @@ func memcachedTestServer(t *testing.T) *minimemcached.MiniMemcached { if err != nil { t.Fatal(err) } + // Workaround for https://github.com/daangn/minimemcached/issues/14 (fix proposed in PR #13). // Wait until the server's accept loop is actually serving connections before returning. // minimemcached.Run spawns a goroutine that calls Accept() on the listener; Close() nils that // listener. On really short tests Close() (in the cleanup below) could run before the goroutine // entered Accept(), causing a nil pointer dereference in the dependency. Probing the server until // it answers guarantees the accept loop is parked in Accept(), so Close() unblocks it cleanly. + // Remove this workaround once a minimemcached release includes the upstream fix. require.Eventually(t, func() bool { conn, err := net.DialTimeout("tcp", fmt.Sprintf("localhost:%d", m.Port()), time.Second) if err != nil {