Skip to content

fix(web): reap the shutdown goroutine when Start fails to bind#74

Merged
xunholy merged 1 commit into
mainfrom
fix/web-shutdown-goroutine-leak
Jun 21, 2026
Merged

fix(web): reap the shutdown goroutine when Start fails to bind#74
xunholy merged 1 commit into
mainfrom
fix/web-shutdown-goroutine-leak

Conversation

@xunholy

@xunholy xunholy commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Bug

Server.Start spawns a goroutine that blocks on the caller's context until shutdown, then calls srv.Shutdown. It waited on the caller's ctx directly — but Start has an early-return path: when srv.ListenAndServe returns a non-ErrServerClosed error (the common case being a bind failure, address already in use), Start returns that error without cancelling ctx. The shutdown goroutine is then stuck on <-ctx.Done() forever (until the caller eventually cancels its context — which on an error return it may never do). Every failed bind leaks one goroutine.

Fix

Derive a cancellable context (runCtx) from the caller's ctx with a deferred cancel, and have the goroutine wait on runCtx:

runCtx, cancelRun := context.WithCancel(ctx)
defer cancelRun()
go func() { <-runCtx.Done(); /* srv.Shutdown */ }()

Now the goroutine is reaped on every return path:

  • bind-error path — the deferred cancelRun wakes it (srv.Shutdown on a server that never served is a no-op)
  • normal path — the caller's ctx cancellation still propagates through runCtx exactly as before, so graceful shutdown is unchanged

Verification

Regression test mirrors the runtime.NumGoroutine before/after pattern already used in internal/agent/streaming_test.go: occupy a loopback port, call Start on it so ListenAndServe fails to bind, and assert — with the caller's ctx left uncancelled — that the goroutine count returns to baseline.

Red→green proven: fails on pre-fix code (2 → 3 goroutines, one never reaped), passes after the fix (reaped in ~0.07s).

  • gofmt / go vet clean
  • golangci-lint run ./...0 issues
  • go test -race ./internal/web/ok

Server.Start spawns a goroutine that blocks on the caller's context until
shutdown, then calls srv.Shutdown. But it waited on the caller's ctx directly,
and Start has an early-return path: when srv.ListenAndServe returns a
non-ErrServerClosed error — the common case being a bind failure, "address
already in use" — Start returns that error WITHOUT cancelling ctx. The shutdown
goroutine is then stuck on <-ctx.Done() forever (until the caller eventually
cancels its context, which on an error return it may never do). Every failed
bind leaks one goroutine.

Fixed by deriving a cancellable context (runCtx) from the caller's ctx with a
deferred cancel, and having the goroutine wait on runCtx. Now the goroutine is
guaranteed to be reaped on every Start return path: on the bind-error path the
deferred cancel wakes it (srv.Shutdown on a server that never served is a
no-op), and on the normal path the caller's ctx cancellation still propagates
through runCtx exactly as before, so graceful shutdown is unchanged.

Regression test (mirrors the runtime.NumGoroutine before/after pattern already
used in internal/agent/streaming_test.go): occupy a loopback port, call Start
on it so ListenAndServe fails to bind, and assert — with the caller's ctx left
uncancelled — that the goroutine count returns to baseline. Verified red→green:
the test fails on the pre-fix code (2 -> 3 goroutines, one never reaped) and
passes after the fix (reaped in ~0.07s). gofmt / go vet / golangci-lint
(0 issues) / go test -race ./internal/web/ all green.
@xunholy
xunholy merged commit 9d849dd into main Jun 21, 2026
10 of 11 checks passed
@xunholy
xunholy deleted the fix/web-shutdown-goroutine-leak branch June 21, 2026 08:26
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.

1 participant