Skip to content

Commit c34710d

Browse files
committed
Enable container-level goroutine-leak checks for the HTTP suite
Disable connection keep-alive so the HTTP transport's per-connection goroutines are torn down promptly instead of lingering in idle pools past a spec: - Client: set Request.Close on the unary client (used only in tests) so net/http closes the connection after reading each response. - Server: set fiber DisableKeepalive on the unary test servers so they close connections after each response. Stream/websocket servers keep keep-alive, as disabling it severs the upgraded connection. - Close the response bodies of the Eventually health-poll loops (pollHealth) so those polling connections are released rather than pinned in net/http's pool. Exempt the fasthttp worker-pool janitor in the shared leak checker: it sleeps for MaxIdleWorkerDuration (10s) between cleanup passes and only observes its stop signal after the in-progress sleep returns, so it outlives its server far longer than the assertion's polling window. This matches the existing updateServerDate exemption; a genuinely leaked server is still caught by its other goroutines. With these, the unary BeforeSuite and stream BeforeAll now run ShouldNotLeakGoroutines. Per-spec checking stays disabled: several specs stand up their own per-spec server and exchange websocket streams whose connections cannot be guaranteed to drain within the per-spec window.
1 parent 4419d0a commit c34710d

7 files changed

Lines changed: 46 additions & 35 deletions

File tree

freighter/go/http/http_suite_test.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package http_test
1111

1212
import (
13+
"net/http"
1314
"testing"
1415

1516
. "github.com/onsi/ginkgo/v2"
@@ -18,10 +19,22 @@ import (
1819

1920
func TestHTTP(t *testing.T) {
2021
RegisterFailHandler(Fail)
21-
// freighter's HTTP transport keeps net/http idle keep-alive connections and
22-
// fasthttp server-worker goroutines alive past a spec's end; they are not
23-
// guaranteed to drain within the per-spec window, so per-spec goroutine-leak
24-
// checking is not applicable to this suite.
22+
// Per-spec leak checking is not registered: several specs stand up their own
23+
// per-spec fiber server and exchange websocket streams, whose connections cannot be
24+
// guaranteed to drain within the per-spec window. The BeforeSuite and BeforeAll
25+
// container-level checks still verify that the suite-wide servers are fully torn
26+
// down at teardown.
2527
//nolint:leak
2628
RunSpecs(t, "HTTP Suite")
2729
}
30+
31+
// pollHealth issues a GET to url and closes the response body, returning any error. The
32+
// Eventually health-check loops use it so their polling connections are released rather
33+
// than left pinned in net/http's idle pool.
34+
func pollHealth(url string) error {
35+
res, err := http.Get(url)
36+
if err != nil {
37+
return err
38+
}
39+
return res.Body.Close()
40+
}

freighter/go/http/recovery_wire_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ package http_test
1111

1212
import (
1313
"context"
14-
"net/http"
1514
"time"
1615

1716
"github.com/gofiber/fiber/v3"
@@ -51,8 +50,7 @@ var _ = Describe("Recovery (wire)", func() {
5150
}()
5251
DeferCleanup(func() { Expect(app.Shutdown()).To(Succeed()) })
5352
Eventually(func(g Gomega) {
54-
_, err := http.Get("http://" + addr.String() + "/health")
55-
g.Expect(err).To(Succeed())
53+
g.Expect(pollHealth("http://" + addr.String() + "/health")).To(Succeed())
5654
}).WithPolling(time.Millisecond).Should(Succeed())
5755

5856
client := MustSucceed(fhttp.NewUnaryClient[test.Request, test.Response]())

freighter/go/http/router_test.go

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ package http_test
1111

1212
import (
1313
"context"
14-
"net/http"
1514
"time"
1615

1716
"github.com/gofiber/fiber/v3"
@@ -109,7 +108,7 @@ var _ = Describe("Router", func() {
109108
Describe("BindTo", func() {
110109
It("should register a unary route on the bound fiber app", func(specCtx SpecContext) {
111110
addr := address.Newf("localhost:%d", MustSucceed(net.FindOpenPort()))
112-
app := fiber.New(fiber.Config{})
111+
app := fiber.New(fiber.Config{DisableKeepalive: true})
113112
router := MustSucceed(fhttp.NewRouter())
114113
server := fhttp.NewUnaryServer[test.Request, test.Response](router, "/echo")
115114
server.BindHandler(func(_ context.Context, req test.Request) (test.Response, error) {
@@ -125,8 +124,7 @@ var _ = Describe("Router", func() {
125124
DeferCleanup(func() { Expect(app.Shutdown()).To(Succeed()) })
126125

127126
Eventually(func(g Gomega) {
128-
_, err := http.Get("http://" + addr.String() + "/echo")
129-
g.Expect(err).To(Succeed())
127+
g.Expect(pollHealth("http://" + addr.String() + "/echo")).To(Succeed())
130128
}).WithPolling(time.Millisecond).Should(Succeed())
131129

132130
client := MustSucceed(fhttp.NewUnaryClient[test.Request, test.Response](
@@ -166,8 +164,7 @@ var _ = Describe("Router", func() {
166164
}()
167165

168166
Eventually(func(g Gomega) {
169-
_, err := http.Get("http://" + addr.String() + "/anything")
170-
g.Expect(err).To(Succeed())
167+
g.Expect(pollHealth("http://" + addr.String() + "/anything")).To(Succeed())
171168
}).WithPolling(time.Millisecond).Should(Succeed())
172169

173170
client := MustSucceed(fhttp.NewStreamClient[test.Request, test.Response](
@@ -185,7 +182,7 @@ var _ = Describe("Router", func() {
185182
Describe("Use", func() {
186183
It("should install middleware on every server registered before the call", func(specCtx SpecContext) {
187184
addr := address.Newf("localhost:%d", MustSucceed(net.FindOpenPort()))
188-
app := fiber.New(fiber.Config{})
185+
app := fiber.New(fiber.Config{DisableKeepalive: true})
189186
router := MustSucceed(fhttp.NewRouter())
190187

191188
var calls int
@@ -210,8 +207,7 @@ var _ = Describe("Router", func() {
210207
}()
211208
DeferCleanup(func() { Expect(app.Shutdown()).To(Succeed()) })
212209
Eventually(func(g Gomega) {
213-
_, err := http.Get("http://" + addr.String() + "/anything")
214-
g.Expect(err).To(Succeed())
210+
g.Expect(pollHealth("http://" + addr.String() + "/anything")).To(Succeed())
215211
}).WithPolling(time.Millisecond).Should(Succeed())
216212

217213
client := MustSucceed(fhttp.NewUnaryClient[test.Request, test.Response](
@@ -226,7 +222,7 @@ var _ = Describe("Router", func() {
226222

227223
It("should not install middleware on servers registered after the call", func(specCtx SpecContext) {
228224
addr := address.Newf("localhost:%d", MustSucceed(net.FindOpenPort()))
229-
app := fiber.New(fiber.Config{})
225+
app := fiber.New(fiber.Config{DisableKeepalive: true})
230226
router := MustSucceed(fhttp.NewRouter())
231227

232228
var calls int
@@ -251,8 +247,7 @@ var _ = Describe("Router", func() {
251247
}()
252248
DeferCleanup(func() { Expect(app.Shutdown()).To(Succeed()) })
253249
Eventually(func(g Gomega) {
254-
_, err := http.Get("http://" + addr.String() + "/anything")
255-
g.Expect(err).To(Succeed())
250+
g.Expect(pollHealth("http://" + addr.String() + "/anything")).To(Succeed())
256251
}).WithPolling(time.Millisecond).Should(Succeed())
257252

258253
client := MustSucceed(fhttp.NewUnaryClient[test.Request, test.Response](
@@ -270,7 +265,7 @@ var _ = Describe("Router", func() {
270265

271266
It("should chain multiple middlewares in registration order", func(specCtx SpecContext) {
272267
addr := address.Newf("localhost:%d", MustSucceed(net.FindOpenPort()))
273-
app := fiber.New(fiber.Config{})
268+
app := fiber.New(fiber.Config{DisableKeepalive: true})
274269
router := MustSucceed(fhttp.NewRouter())
275270

276271
var order []string
@@ -305,8 +300,7 @@ var _ = Describe("Router", func() {
305300
}()
306301
DeferCleanup(func() { Expect(app.Shutdown()).To(Succeed()) })
307302
Eventually(func(g Gomega) {
308-
_, err := http.Get("http://" + addr.String() + "/anything")
309-
g.Expect(err).To(Succeed())
303+
g.Expect(pollHealth("http://" + addr.String() + "/anything")).To(Succeed())
310304
}).WithPolling(time.Millisecond).Should(Succeed())
311305

312306
client := MustSucceed(fhttp.NewUnaryClient[test.Request, test.Response](

freighter/go/http/stream_test.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,8 @@ var _ = Describe("Stream", Ordered, Serial, func() {
3535
app *fiber.App
3636
)
3737

38-
// freighter's HTTP transport keeps net/http idle keep-alive connections and
39-
// fasthttp server-worker goroutines alive past this container's lifetime, so a
40-
// leak check here is not applicable.
41-
//nolint:leak
4238
BeforeAll(func() {
39+
ShouldNotLeakGoroutines()
4340
addr = address.Newf("localhost:%d", MustSucceed(net.FindOpenPort()))
4441
app = fiber.New(fiber.Config{})
4542
router := MustSucceed(fhttp.NewRouter(fhttp.RouterConfig{
@@ -60,8 +57,7 @@ var _ = Describe("Stream", Ordered, Serial, func() {
6057
})).To(Succeed())
6158
}()
6259
Eventually(func(g Gomega) {
63-
_, err := http.Get("http://" + addr.String() + "/health")
64-
g.Expect(err).To(Succeed())
60+
g.Expect(pollHealth("http://" + addr.String() + "/health")).To(Succeed())
6561
}).WithPolling(1 * time.Millisecond).Should(Succeed())
6662
})
6763

freighter/go/http/unary_client.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ func (u *unaryClient[RQ, RS]) Send(
145145
return freighter.Context{}, err
146146
}
147147
setRequestCtx(httpReq, inCtx)
148+
// This client is only used in tests; disable connection keep-alive so the
149+
// underlying connection (and its net/http reader/writer goroutines) is torn
150+
// down when the response is read rather than lingering in the idle pool.
151+
httpReq.Close = true
148152
httpReq.Header.Set(fiber.HeaderContentType, u.encoder.ContentType())
149153
httpReq.Header.Set(fiber.HeaderAccept, u.acceptHeader)
150154

freighter/go/http/unary_test.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,10 @@ func (failingEncoder) EncodeStream(context.Context, io.Writer, any) error {
5757
return errFailingEncoderEncodeFail
5858
}
5959

60-
// The suite-wide fiber (fasthttp) server started here keeps server-worker goroutines
61-
// alive for the lifetime of the process, so a BeforeSuite leak check is not applicable.
62-
//
63-
//nolint:leak
6460
var _ = BeforeSuite(func() {
61+
ShouldNotLeakGoroutines()
6562
unaryAddr = address.Newf("localhost:%d", MustSucceed(net.FindOpenPort()))
66-
unaryApp = fiber.New(fiber.Config{})
63+
unaryApp = fiber.New(fiber.Config{DisableKeepalive: true})
6764
router := MustSucceed(fhttp.NewRouter())
6865
unaryApp.Get("/health", func(ctx fiber.Ctx) error {
6966
return ctx.SendStatus(fiber.StatusOK)
@@ -103,8 +100,7 @@ var _ = BeforeSuite(func() {
103100
})).To(Succeed())
104101
}()
105102
Eventually(func(g Gomega) {
106-
_, err := http.Get("http://" + unaryAddr.String() + "/health")
107-
g.Expect(err).To(Succeed())
103+
g.Expect(pollHealth("http://" + unaryAddr.String() + "/health")).To(Succeed())
108104
}).WithPolling(1 * time.Millisecond).Should(Succeed())
109105
})
110106

x/go/testutil/leak.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,16 @@ func assertNoLeakedGoroutines(snapshot []gleak.Goroutine) {
8787
args,
8888
gleak.IgnoringCreator("github.com/valyala/fasthttp.updateServerDate"),
8989
)
90+
// Every fasthttp server starts a worker-pool janitor that sleeps for
91+
// MaxIdleWorkerDuration (10s) between cleanup passes. On server shutdown it is
92+
// signaled to stop, but the signal is only observed after the in-progress sleep
93+
// returns, so it can outlive its server by up to 10s. That is far longer than the
94+
// leak assertion's polling window, so ignore it; a genuinely leaked server is still
95+
// caught by its other (acceptor/connection) goroutines.
96+
args = append(
97+
args,
98+
gleak.IgnoringCreator("github.com/valyala/fasthttp.(*workerPool).Start"),
99+
)
90100
assertion := gomega.Eventually(gleak.Goroutines)
91101
assertion.ShouldNot(gleak.HaveLeaked(args...))
92102
}

0 commit comments

Comments
 (0)