diff --git a/coderd/wsconncache/wsconncache_test.go b/coderd/wsconncache/wsconncache_test.go index 26ce44463e70c..635f1f5f0e4bf 100644 --- a/coderd/wsconncache/wsconncache_test.go +++ b/coderd/wsconncache/wsconncache_test.go @@ -30,118 +30,131 @@ import ( "github.com/coder/coder/tailnet/tailnettest" ) -func TestMain(m *testing.M) { - goleak.VerifyTestMain(m) +// t.Parallel() has been disabled on purpose to identify the TestCase, which leaks the goroutine. +// It can't be achieved with goleak.VerifyTestMain(). +// +// See: https://github.com/coder/coder/issues/5302 +func TestCacheSame(t *testing.T) { //nolint:paralleltest + t.Cleanup(func() { + goleak.VerifyNone(t) + }) + + cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*codersdk.AgentConn, error) { + return setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0), nil + }, time.Microsecond) + defer func() { + _ = cache.Close() + }() + + conn1, _, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil) + require.NoError(t, err) + conn2, _, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil) + require.NoError(t, err) + require.True(t, conn1 == conn2) } -func TestCache(t *testing.T) { - t.Parallel() - t.Run("Same", func(t *testing.T) { - t.Parallel() - cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*codersdk.AgentConn, error) { - return setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0), nil - }, 0) - defer func() { - _ = cache.Close() - }() - conn1, _, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil) - require.NoError(t, err) - conn2, _, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil) - require.NoError(t, err) - require.True(t, conn1 == conn2) - }) - t.Run("Expire", func(t *testing.T) { - t.Parallel() - called := atomic.NewInt32(0) - cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*codersdk.AgentConn, error) { - called.Add(1) - return setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0), nil - }, time.Microsecond) - defer func() { - _ = cache.Close() - }() - conn, release, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil) - require.NoError(t, err) - release() - <-conn.Closed() - conn, release, err = cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil) - require.NoError(t, err) - release() - <-conn.Closed() - require.Equal(t, int32(2), called.Load()) +func TestCacheExpire(t *testing.T) { //nolint:paralleltest + t.Cleanup(func() { + goleak.VerifyNone(t) }) - t.Run("NoExpireWhenLocked", func(t *testing.T) { - t.Parallel() - cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*codersdk.AgentConn, error) { - return setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0), nil - }, time.Microsecond) - defer func() { - _ = cache.Close() - }() - conn, release, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil) - require.NoError(t, err) - time.Sleep(time.Millisecond) - release() - <-conn.Closed() + + called := atomic.NewInt32(0) + cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*codersdk.AgentConn, error) { + called.Add(1) + return setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0), nil + }, time.Microsecond) + defer func() { + _ = cache.Close() + }() + conn, release, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil) + require.NoError(t, err) + release() + <-conn.Closed() + conn, release, err = cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil) + require.NoError(t, err) + release() + <-conn.Closed() + require.Equal(t, int32(2), called.Load()) +} + +func TestCacheNoExpireWhenLocked(t *testing.T) { //nolint:paralleltest + t.Cleanup(func() { + goleak.VerifyNone(t) }) - t.Run("HTTPTransport", func(t *testing.T) { - t.Parallel() - random, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - defer func() { - _ = random.Close() - }() - tcpAddr, valid := random.Addr().(*net.TCPAddr) - require.True(t, valid) - - server := &http.Server{ - ReadHeaderTimeout: time.Minute, - Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - }), - } - defer func() { - _ = server.Close() - }() - go server.Serve(random) - cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*codersdk.AgentConn, error) { - return setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0), nil - }, time.Microsecond) - defer func() { - _ = cache.Close() - }() + cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*codersdk.AgentConn, error) { + return setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0), nil + }, time.Microsecond) + defer func() { + _ = cache.Close() + }() + conn, release, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil) + require.NoError(t, err) + time.Sleep(time.Millisecond) + release() + <-conn.Closed() +} - var wg sync.WaitGroup - // Perform many requests in parallel to simulate - // simultaneous HTTP requests. - for i := 0; i < 50; i++ { - wg.Add(1) - go func() { - defer wg.Done() - proxy := httputil.NewSingleHostReverseProxy(&url.URL{ - Scheme: "http", - Host: fmt.Sprintf("127.0.0.1:%d", tcpAddr.Port), - Path: "/", - }) - req := httptest.NewRequest(http.MethodGet, "/", nil) - conn, release, err := cache.Acquire(req, uuid.Nil) - if !assert.NoError(t, err) { - return - } - defer release() - transport := conn.HTTPTransport() - defer transport.CloseIdleConnections() - proxy.Transport = transport - res := httptest.NewRecorder() - proxy.ServeHTTP(res, req) - resp := res.Result() - defer resp.Body.Close() - assert.Equal(t, http.StatusOK, resp.StatusCode) - }() - } - wg.Wait() +func TestCacheHTTPTransport(t *testing.T) { //nolint:paralleltest + t.Cleanup(func() { + goleak.VerifyNone(t) }) + + random, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer func() { + _ = random.Close() + }() + tcpAddr, valid := random.Addr().(*net.TCPAddr) + require.True(t, valid) + + server := &http.Server{ + ReadHeaderTimeout: time.Minute, + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }), + } + defer func() { + _ = server.Close() + }() + go server.Serve(random) + + cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*codersdk.AgentConn, error) { + return setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0), nil + }, time.Microsecond) + defer func() { + _ = cache.Close() + }() + + var wg sync.WaitGroup + // Perform many requests in parallel to simulate + // simultaneous HTTP requests. + for i := 0; i < 50; i++ { + wg.Add(1) + go func() { + defer wg.Done() + proxy := httputil.NewSingleHostReverseProxy(&url.URL{ + Scheme: "http", + Host: fmt.Sprintf("127.0.0.1:%d", tcpAddr.Port), + Path: "/", + }) + req := httptest.NewRequest(http.MethodGet, "/", nil) + conn, release, err := cache.Acquire(req, uuid.Nil) + if !assert.NoError(t, err) { + return + } + defer release() + transport := conn.HTTPTransport() + defer transport.CloseIdleConnections() + proxy.Transport = transport + res := httptest.NewRecorder() + proxy.ServeHTTP(res, req) + resp := res.Result() + defer resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + }() + } + wg.Wait() } func setupAgent(t *testing.T, metadata codersdk.WorkspaceAgentMetadata, ptyTimeout time.Duration) *codersdk.AgentConn {