Skip to content

39133-custom-linter-for-time-after-leaks-in-loops.md

Latest commit

 

History

History
42 lines (27 loc) · 3.55 KB

39133-custom-linter-for-time-after-leaks-in-loops.md

File metadata and controls

42 lines (27 loc) · 3.55 KB
Raw

ADR-39133: Ship a Custom timeafterleak Linter for time.After in Loop Selects

Date: 2026-06-13 Status: Draft

Context

time.After(d) allocates a fresh *time.Timer (and its backing runtime timer) on every call; the timer is not reclaimed until it fires. When a time.After call is the channel-receive of a select case inside a for/range loop, every iteration where another case fires first leaves a timer alive until its full duration elapses, leaking goroutine/heap resources in hot loops. This is a documented Go gotcha but is not flagged by any commonly enabled linter: staticcheck SA6001 only covers time.Tick, not time.After in select cases. A code scan of this repository found three production occurrences of the pattern, so the project needs an automated guard that fits its existing in-repo linter suite (pkg/linters/* registered in cmd/linters/main.go).

Decision

We will add a bespoke go/analysis analyzer, timeafterleak, to the project's custom linter suite rather than relying on an external linter. The analyzer uses inspector.Cursor parent-chain traversal to flag a time.After call only when it is the Comm receive expression of a select CommClause enclosed by a for or range loop, stopping at FuncLit/FuncDecl boundaries and verifying the time package identity via type information rather than a syntactic name match. It honors //nolint:timeafterleak suppression and skips test files, and is registered in the multichecker in cmd/linters/main.go.

Alternatives Considered

Alternative 1: Rely on staticcheck / golangci-lint defaults

The project already runs standard linters, so extending their configuration would avoid new code. Rejected because no default rule (including SA6001) detects time.After inside select cases; the specific leak pattern would go uncaught.

Alternative 2: Document the gotcha and rely on code review

A contributor guideline plus manual review requires no tooling. Rejected because manual review is unreliable for an easily overlooked pattern — three instances already reached production — and provides no enforceable, repeatable guard.

Alternative 3: Use a syntactic (string-match) detector instead of type-checked detection

A simpler analyzer could match the identifier text time.After directly. Rejected because it would produce false positives for shadowed identifiers or unrelated packages; type-based identity checking is more precise at modest extra complexity.

Consequences

Positive

  • Catches a real, otherwise-undetected resource-leak pattern automatically in CI.
  • Follows the established in-repo linter convention, so it composes with the existing cmd/linters multichecker and shared internal helpers.
  • Precise: type-checked time identity and Comm-position scoping minimize false positives; FuncLit boundary handling avoids flagging per-iteration goroutine closures.

Negative

  • Adds a custom analyzer that the team must maintain, including keeping pace with go/ast/golang.org/x/tools API changes.
  • Narrow scope: only flags the loop-select Comm position, so other time.After misuse patterns remain uncaught and may create a false sense of full coverage.

Neutral

  • Suppression is available via //nolint:timeafterleak for intentional cases.
  • Test files are excluded from analysis, matching the suite's existing conventions.

This is a DRAFT ADR generated by the Design Decision Gate workflow. The PR author must review, complete, and finalize this document before the PR can merge.