From 0643f42190069596102053b2a5417626c5b9aa5d Mon Sep 17 00:00:00 2001 From: "Y.Horie" Date: Sat, 13 Jun 2026 12:49:39 +0900 Subject: [PATCH] ci: re-enable gocritic deferInLoop check (#2288) deferInLoop was disabled in .golangci.yml, so CI could not catch defer statements added inside for loops. Such defers run only at function exit, not per iteration, leaking file descriptors, connections, or locks under load. Re-enable the check and fix the two existing occurrences in test code instead of excluding test files: - inmemory listener test: deferred conn.Close() inside the accept loop; move the per-connection handling into a closure so each connection is closed when done. - TestClientManyServers: every server must stay up until the test ends, so hoist the defer out of the loop and stop all servers in a single deferred loop. Fixes #2233 Signed-off-by: Y.Horie --- .golangci.yml | 1 - client_test.go | 10 ++++++- fasthttputil/inmemory_listener_test.go | 38 ++++++++++++++------------ 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index fa5d0b5..7eb9120 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -47,7 +47,6 @@ linters: settings: gocritic: disabled-checks: - - deferInLoop - importShadow - sloppyReassign - unnamedResult diff --git a/client_test.go b/client_test.go index a359f79..476698e 100644 --- a/client_test.go +++ b/client_test.go @@ -2943,12 +2943,20 @@ func TestClientHTTPSConcurrent(t *testing.T) { func TestClientManyServers(t *testing.T) { t.Parallel() + servers := make([]*testEchoServer, 0, 10) addrs := make([]string, 0, 10) for range 10 { s := startEchoServer(t, "tcp", "127.0.0.1:") - defer s.Stop() + servers = append(servers, s) addrs = append(addrs, s.Addr()) } + // All servers must stay up until the test ends, so stop them with a single + // deferred loop rather than deferring inside the loop above. + defer func() { + for _, s := range servers { + s.Stop() + } + }() var wg sync.WaitGroup for i := range 4 { diff --git a/fasthttputil/inmemory_listener_test.go b/fasthttputil/inmemory_listener_test.go index 943b329..5ab1127 100644 --- a/fasthttputil/inmemory_listener_test.go +++ b/fasthttputil/inmemory_listener_test.go @@ -57,24 +57,26 @@ func TestInmemoryListener(t *testing.T) { close(serverCh) return } - defer conn.Close() - buf := make([]byte, 30) - n, err := conn.Read(buf) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - buf = buf[:n] - if !bytes.HasPrefix(buf, []byte("request_")) { - t.Errorf("unexpected request prefix %q. Expecting %q", buf, "request_") - } - resp := fmt.Sprintf("response_%s", buf[len("request_"):]) - n, err = conn.Write([]byte(resp)) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if n != len(resp) { - t.Errorf("unexpected number of bytes written: %d. Expecting %d", n, len(resp)) - } + func() { + defer conn.Close() + buf := make([]byte, 30) + n, err := conn.Read(buf) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + buf = buf[:n] + if !bytes.HasPrefix(buf, []byte("request_")) { + t.Errorf("unexpected request prefix %q. Expecting %q", buf, "request_") + } + resp := fmt.Sprintf("response_%s", buf[len("request_"):]) + n, err = conn.Write([]byte(resp)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if n != len(resp) { + t.Errorf("unexpected number of bytes written: %d. Expecting %d", n, len(resp)) + } + }() } }()