Skip to content

Commit d656c6c

Browse files
committed
Add leaklint analyzer and enforce goroutine-leak checks
- Add x/go/analyzers/leaklint: a Go analyzer enforcing that every Ginkgo suite registers ShouldNotLeakGoroutinesPerSpec() at package scope and that every BeforeSuite/BeforeAll calls ShouldNotLeakGoroutines() as its first statement. - Make leak_test.go black-box (package testutil_test); add fasthttp updateServerDate to default leak filters (upstream won't-fix #2257). - Fix Fake.AfterFunc goroutine leak: Stop releases the goroutine and After is buffered so Advance's send never blocks. - Apply the per-spec and setup leak checks across x/go, alamos, aspen, and freighter/integration; suppress the un-teardownable OTel daemons in the alamos suite via //nolint:leaklint.
1 parent 76f67c2 commit d656c6c

80 files changed

Lines changed: 645 additions & 58 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

alamos/go/alamos_suite_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,15 @@ func TestAlamos(t *testing.T) {
2525
RunSpecs(t, "Alamos Suite")
2626
}
2727

28+
// The suite-wide instrumentation starts process-global OpenTelemetry SDK daemons (batch
29+
// span processor, periodic reader, log exporter) that alamos exposes no teardown for, so
30+
// a BeforeSuite leak check is not applicable here.
31+
//
32+
//nolint:leaklint
2833
var _ = BeforeSuite(func() {
2934
devIns = Instrumentation("alamos-test", InstrumentationConfig{
3035
Trace: new(true),
3136
})
3237
})
38+
39+
var _ = ShouldNotLeakGoroutinesPerSpec()

aspen/internal/cluster/pledge/pledge_suite_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func TestMembership(t *testing.T) {
2626
}
2727

2828
var _ = BeforeSuite(func() {
29+
ShouldNotLeakGoroutines()
2930
ins = Instrumentation("pledge", InstrumentationConfig{Log: new(false)})
3031
})
3132

freighter/integration/http/http_suite_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@ import (
1414

1515
. "github.com/onsi/ginkgo/v2"
1616
. "github.com/onsi/gomega"
17+
. "github.com/synnaxlabs/x/testutil"
1718
)
1819

1920
func TestHTTP(t *testing.T) {
2021
RegisterFailHandler(Fail)
2122
RunSpecs(t, "Integration HTTP Suite")
2223
}
24+
25+
var _ = ShouldNotLeakGoroutinesPerSpec()

x/go/address/address_suite_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@ import (
1414

1515
. "github.com/onsi/ginkgo/v2"
1616
. "github.com/onsi/gomega"
17+
. "github.com/synnaxlabs/x/testutil"
1718
)
1819

1920
func TestAddress(t *testing.T) {
2021
RegisterFailHandler(Fail)
2122
RunSpecs(t, "Address Suite")
2223
}
24+
25+
var _ = ShouldNotLeakGoroutinesPerSpec()
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
// Copyright 2026 Synnax Labs, Inc.
2+
//
3+
// Use of this software is governed by the Business Source License included in the file
4+
// licenses/BSL.txt.
5+
//
6+
// As of the Change Date specified in that file, in accordance with the Business Source
7+
// License, use of this software will be governed by the Apache License, Version 2.0,
8+
// included in the file licenses/APL.txt.
9+
10+
package leaklint
11+
12+
import (
13+
"go/ast"
14+
"go/token"
15+
16+
"golang.org/x/tools/go/analysis"
17+
)
18+
19+
const (
20+
// leakCheck is the per-scope goroutine-leak assertion that BeforeSuite and BeforeAll
21+
// nodes must call first.
22+
leakCheck = "ShouldNotLeakGoroutines"
23+
// perSpecCheck is the suite-wide per-spec goroutine-leak assertion that every suite
24+
// must register at package scope.
25+
perSpecCheck = "ShouldNotLeakGoroutinesPerSpec"
26+
// runSpecs is the Ginkgo entry point whose presence marks a package as a test suite.
27+
runSpecs = "RunSpecs"
28+
)
29+
30+
// setupNodes are the Ginkgo lifecycle nodes whose fixtures persist across specs and so
31+
// must verify their own teardown with leakCheck. Per-spec nodes (BeforeEach,
32+
// JustBeforeEach) are intentionally excluded: they are covered by perSpecCheck.
33+
var setupNodes = map[string]struct{}{
34+
"BeforeSuite": {},
35+
"BeforeAll": {},
36+
}
37+
38+
var Analyzer = &analysis.Analyzer{
39+
Name: "leaklint",
40+
Doc: `enforces goroutine-leak checks in Ginkgo suites.
41+
42+
This analyzer enforces two conventions from github.com/synnaxlabs/x/testutil:
43+
44+
1. Every package that calls RunSpecs (a Ginkgo suite) must register the per-spec
45+
leak check at package scope, e.g. ` + "`var _ = ShouldNotLeakGoroutinesPerSpec()`" + `.
46+
47+
2. Every BeforeSuite and BeforeAll node must call ShouldNotLeakGoroutines() as its
48+
first statement, so the goroutine snapshot is taken before the node creates its
49+
fixtures and the matching AfterSuite/AfterAll teardown is verified to release them.
50+
51+
A node that legitimately needs to opt out (e.g. a suite that deliberately leaves a
52+
process-global daemon running) can be exempted with a //nolint:leaklint comment.`,
53+
Run: run,
54+
}
55+
56+
func run(pass *analysis.Pass) (any, error) {
57+
for _, file := range pass.Files {
58+
ast.Inspect(file, func(n ast.Node) bool {
59+
call, ok := n.(*ast.CallExpr)
60+
if !ok {
61+
return true
62+
}
63+
if _, isSetup := setupNodes[calleeName(call)]; !isSetup {
64+
return true
65+
}
66+
body := setupBody(call)
67+
if body == nil {
68+
// The node was given a named function rather than a literal; its body is
69+
// not visible here, so there is nothing to check.
70+
return true
71+
}
72+
if !firstStmtIsLeakCheck(body) {
73+
pass.Report(analysis.Diagnostic{
74+
Pos: call.Pos(),
75+
End: call.End(),
76+
Message: calleeName(call) + " must call " + leakCheck +
77+
"() as its first statement so the goroutine snapshot is taken " +
78+
"before fixtures are created",
79+
})
80+
}
81+
return true
82+
})
83+
}
84+
checkSuiteHasPerSpec(pass)
85+
return nil, nil
86+
}
87+
88+
// checkSuiteHasPerSpec reports every RunSpecs call in a package that does not also
89+
// register perSpecCheck at package scope.
90+
func checkSuiteHasPerSpec(pass *analysis.Pass) {
91+
if hasTopLevelPerSpec(pass) {
92+
return
93+
}
94+
for _, file := range pass.Files {
95+
ast.Inspect(file, func(n ast.Node) bool {
96+
call, ok := n.(*ast.CallExpr)
97+
if !ok || calleeName(call) != runSpecs {
98+
return true
99+
}
100+
pass.Report(analysis.Diagnostic{
101+
Pos: call.Pos(),
102+
End: call.End(),
103+
Message: "test suite calls " + runSpecs + " but does not register " +
104+
perSpecCheck + "() at package scope; add `var _ = " +
105+
perSpecCheck + "()`",
106+
})
107+
return true
108+
})
109+
}
110+
}
111+
112+
// hasTopLevelPerSpec reports whether any file in the package contains a package-scope
113+
// call to perSpecCheck, e.g. `var _ = ShouldNotLeakGoroutinesPerSpec()`.
114+
func hasTopLevelPerSpec(pass *analysis.Pass) bool {
115+
for _, file := range pass.Files {
116+
for _, decl := range file.Decls {
117+
gen, ok := decl.(*ast.GenDecl)
118+
if !ok || gen.Tok != token.VAR {
119+
continue
120+
}
121+
found := false
122+
ast.Inspect(gen, func(n ast.Node) bool {
123+
if call, ok := n.(*ast.CallExpr); ok && calleeName(call) == perSpecCheck {
124+
found = true
125+
return false
126+
}
127+
return true
128+
})
129+
if found {
130+
return true
131+
}
132+
}
133+
}
134+
return false
135+
}
136+
137+
// firstStmtIsLeakCheck reports whether the first statement of body is a bare call to
138+
// leakCheck.
139+
func firstStmtIsLeakCheck(body *ast.BlockStmt) bool {
140+
if len(body.List) == 0 {
141+
return false
142+
}
143+
exprStmt, ok := body.List[0].(*ast.ExprStmt)
144+
if !ok {
145+
return false
146+
}
147+
call, ok := exprStmt.X.(*ast.CallExpr)
148+
if !ok {
149+
return false
150+
}
151+
return calleeName(call) == leakCheck
152+
}
153+
154+
// setupBody returns the body of the first function-literal argument of a
155+
// BeforeSuite/BeforeAll call, or nil if the call has no literal argument (e.g. it was
156+
// handed a named function, whose body cannot be inspected here).
157+
func setupBody(call *ast.CallExpr) *ast.BlockStmt {
158+
for _, arg := range call.Args {
159+
if lit, ok := arg.(*ast.FuncLit); ok {
160+
return lit.Body
161+
}
162+
}
163+
return nil
164+
}
165+
166+
// calleeName returns the called function's identifier for both unqualified F(...) (the
167+
// dot-imported case) and qualified pkg.F(...) calls. It returns "" for any other callee
168+
// expression.
169+
func calleeName(call *ast.CallExpr) string {
170+
switch fn := call.Fun.(type) {
171+
case *ast.Ident:
172+
return fn.Name
173+
case *ast.SelectorExpr:
174+
return fn.Sel.Name
175+
}
176+
return ""
177+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright 2026 Synnax Labs, Inc.
2+
//
3+
// Use of this software is governed by the Business Source License included in the file
4+
// licenses/BSL.txt.
5+
//
6+
// As of the Change Date specified in that file, in accordance with the Business Source
7+
// License, use of this software will be governed by the Apache License, Version 2.0,
8+
// included in the file licenses/APL.txt.
9+
10+
package leaklint_test
11+
12+
import (
13+
. "github.com/onsi/ginkgo/v2"
14+
"github.com/synnaxlabs/x/analyzers/leaklint"
15+
"golang.org/x/tools/go/analysis/analysistest"
16+
)
17+
18+
var _ = Describe("Analyzer", func() {
19+
It("Should accept a well-formed suite", func() {
20+
analysistest.Run(GinkgoT(), analysistest.TestData(), leaklint.Analyzer, "good")
21+
})
22+
23+
It("Should flag a suite that does not register the per-spec leak check", func() {
24+
analysistest.Run(GinkgoT(), analysistest.TestData(), leaklint.Analyzer, "missingperspec")
25+
})
26+
27+
It("Should flag BeforeSuite/BeforeAll nodes missing the leak check", func() {
28+
analysistest.Run(GinkgoT(), analysistest.TestData(), leaklint.Analyzer, "badsetup")
29+
})
30+
})
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2026 Synnax Labs, Inc.
2+
//
3+
// Use of this software is governed by the Business Source License included in the file
4+
// licenses/BSL.txt.
5+
//
6+
// As of the Change Date specified in that file, in accordance with the Business Source
7+
// License, use of this software will be governed by the Apache License, Version 2.0,
8+
// included in the file licenses/APL.txt.
9+
10+
package main
11+
12+
import (
13+
"github.com/synnaxlabs/x/analyzers/leaklint"
14+
"golang.org/x/tools/go/analysis/singlechecker"
15+
)
16+
17+
func main() { singlechecker.Main(leaklint.Analyzer) }
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright 2026 Synnax Labs, Inc.
2+
//
3+
// Use of this software is governed by the Business Source License included in the file
4+
// licenses/BSL.txt.
5+
//
6+
// As of the Change Date specified in that file, in accordance with the Business Source
7+
// License, use of this software will be governed by the Apache License, Version 2.0,
8+
// included in the file licenses/APL.txt.
9+
10+
package leaklint_test
11+
12+
import (
13+
"testing"
14+
15+
. "github.com/onsi/ginkgo/v2"
16+
. "github.com/onsi/gomega"
17+
. "github.com/synnaxlabs/x/testutil"
18+
)
19+
20+
func TestLeaklint(t *testing.T) {
21+
RegisterFailHandler(Fail)
22+
RunSpecs(t, "Analyzers LeakLint Suite")
23+
}
24+
25+
var _ = ShouldNotLeakGoroutinesPerSpec()

x/go/analyzers/leaklint/plugin.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright 2026 Synnax Labs, Inc.
2+
//
3+
// Use of this software is governed by the Business Source License included in the file
4+
// licenses/BSL.txt.
5+
//
6+
// As of the Change Date specified in that file, in accordance with the Business Source
7+
// License, use of this software will be governed by the Apache License, Version 2.0,
8+
// included in the file licenses/APL.txt.
9+
10+
package leaklint
11+
12+
import (
13+
"github.com/golangci/plugin-module-register/register"
14+
"golang.org/x/tools/go/analysis"
15+
)
16+
17+
func init() { register.Plugin("leaklint", newPlugin) }
18+
19+
func newPlugin(any) (register.LinterPlugin, error) { return &plugin{}, nil }
20+
21+
type plugin struct{}
22+
23+
func (p *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) {
24+
return []*analysis.Analyzer{Analyzer}, nil
25+
}
26+
27+
func (p *plugin) GetLoadMode() string { return register.LoadModeSyntax }
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright 2026 Synnax Labs, Inc.
2+
//
3+
// Use of this software is governed by the Business Source License included in the file
4+
// licenses/BSL.txt.
5+
//
6+
// As of the Change Date specified in that file, in accordance with the Business Source
7+
// License, use of this software will be governed by the Apache License, Version 2.0,
8+
// included in the file licenses/APL.txt.
9+
10+
package badsetup
11+
12+
func BeforeSuite(...any) bool { return true }
13+
func BeforeAll(...any) bool { return true }
14+
func BeforeEach(...any) bool { return true }
15+
func ShouldNotLeakGoroutines(...any) {}
16+
func ShouldNotLeakGoroutinesPerSpec(...any) bool { return true }
17+
func openFixture() {}
18+
19+
var _ = ShouldNotLeakGoroutinesPerSpec()
20+
21+
// Missing the leak check entirely.
22+
var _ = BeforeSuite(func() { // want "BeforeSuite must call ShouldNotLeakGoroutines"
23+
openFixture()
24+
})
25+
26+
// Present, but not the first statement.
27+
var _ = BeforeAll(func() { // want "BeforeAll must call ShouldNotLeakGoroutines"
28+
openFixture()
29+
ShouldNotLeakGoroutines()
30+
})
31+
32+
// Correct: leak check is the first statement — no diagnostic.
33+
var _ = BeforeSuite(func() {
34+
ShouldNotLeakGoroutines()
35+
openFixture()
36+
})
37+
38+
// BeforeEach is a per-spec node, covered by ShouldNotLeakGoroutinesPerSpec — not flagged.
39+
var _ = BeforeEach(func() {
40+
openFixture()
41+
})

0 commit comments

Comments
 (0)