Skip to content

fix(net/netcheck): prevent goroutine leak in measureHTTPLatency#124

Merged
mafredri merged 1 commit into
mainfrom
mafredri/fix-netcheck-goroutine-leak
May 29, 2026
Merged

fix(net/netcheck): prevent goroutine leak in measureHTTPLatency#124
mafredri merged 1 commit into
mainfrom
mafredri/fix-netcheck-goroutine-leak

Conversation

@mafredri
Copy link
Copy Markdown
Member

@mafredri mafredri commented May 29, 2026

measureHTTPLatency creates an http.Transport per DERP region probe for
a single HTTP request. With keep-alives enabled (the default), the
transport spawns persistent readLoop/writeLoop goroutines that outlive
the function call. These are detected as leaks by goleak in callers
like support/TestRun.

Set DisableKeepAlives: true on the transport since only one request is
made per probe. This prevents the transport from spawning persistent
connection goroutines entirely.

Fixes coder/coder PLAT-289
Refs coder/coder#25838

🤖 This PR was created with the help of Coder Agents, and will be reviewed by a human. 🏂🏻

measureHTTPLatency creates an http.Transport per DERP region probe for
a single HTTP request. With keep-alives enabled (the default), the
transport spawns persistent readLoop/writeLoop goroutines that outlive
the function call. These are detected as leaks by goleak in callers
like support/TestRun.

Set DisableKeepAlives: true on the transport since only one request is
made per probe. This prevents the transport from spawning persistent
connection goroutines entirely.
@mafredri mafredri marked this pull request as ready for review May 29, 2026 10:31
@mafredri mafredri requested a review from a team as a code owner May 29, 2026 10:31
@mafredri mafredri requested a review from Copilot May 29, 2026 10:31
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a goroutine leak in netcheck HTTP latency probing by disabling keep-alive behavior for the one-off HTTP transport used per DERP region probe.

Changes:

  • Sets DisableKeepAlives: true on the per-probe http.Transport.
  • Keeps the existing custom dial path for HTTP vs HTTPS DERP probes unchanged.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me 👍

@mafredri mafredri changed the title fix(netcheck): prevent goroutine leak in measureHTTPLatency fix(net/netcheck): prevent goroutine leak in measureHTTPLatency May 29, 2026
@mafredri mafredri merged commit b7c5fc6 into main May 29, 2026
16 of 27 checks passed
@mafredri mafredri deleted the mafredri/fix-netcheck-goroutine-leak branch May 29, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants