You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(tunnel): join tunnel goroutine on handler exit - #1013
pkg/tunnel — pipebridge.goawaitPair (the goroutine-join path of RunPair).
Issue found
awaitPair waits on two goroutines: the tunnel forwarder and the user handler. The
two completion branches were asymmetric:
When the tunnel finished first, the branch cancelled the context, closed the
stdin/stdout pipe write ends, and then joined the handler goroutine
(handlerErr = <-handlerChan) so awaitPair only returned once both sides were done.
When the handler finished first, the branch only did a non-blocking select { case tunnelErr = <-tunnelChan: default: }. It did not cancel the
context, did not close the pipe write ends, and did not join the tunnel
goroutine. awaitPair returned immediately while the tunnel goroutine kept running.
Because the tunnel forwarder blocks reading from its stdin pipe, and the stdin write
end is never closed on the handler-first path, the tunnel goroutine blocks forever on
that read — it is orphaned/leaked. This is a goroutine leak plus a file-descriptor leak
(the pipe write ends are left open). The tunnel goroutine is never joined, so any
error it returns is also silently dropped.
Change
Make the handler-first branch symmetric with the tunnel-first branch: on handler
completion, cancel the context, close the stdout/stdin pipe write ends, and join
the tunnel goroutine (tunnelErr = <-tunnelChan) before returning. This ensures both
goroutines are reaped and their errors are surfaced through ClassifyTunnelErrors,
matching the existing tunnel-first behavior.
This is a focused correctness fix (no behavioral change for the already-correct
tunnel-first path), keeping RunPair symmetric regardless of which side completes first.
Verification
task cli:format — clean.
task cli:lint:ci — 0 new issues.
task cli:test — pkg/tunnel passes (ok ... coverage: 37.3%). The only failing
package is pkg/git (TestRepoClone*), a known pre-existing failure on origin/main unrelated to this change (my change does not touch pkg/git).
New regression test TestRunPairJoinsTunnelWhenHandlerFinishesFirst:
On the old code it hangs (the orphaned tunnel goroutine never exits), failing
after the 2s test timeout with tunnel goroutine was orphaned when handler finished first.
On the fixed code it passes (the tunnel goroutine exits promptly once the pipe is
closed and is joined before RunPair returns).
No proto files changed, so task cli:build:grpc was not required.
This PR was created by an AI agent as part of an automated daily package review job.
awaitPair returned without joining the tunnel goroutine when the handler completed first, leaving the tunnel blocked on its stdin pipe read and orphaning both the goroutine and the pipe write ends. Mirror the tunnel-first branch: cancel the context, close the pipe write ends, then join the tunnel goroutine so its error is surfaced and no goroutine is leaked. Add TestRunPairJoinsTunnelWhenHandlerFinishesFirst as a regression guard.
AI Reviewer: run a review on demand. To trigger the first review automatically, go to your organization or repository integration settings. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Package reviewed
pkg/tunnel—pipebridge.goawaitPair(the goroutine-join path ofRunPair).Issue found
awaitPairwaits on two goroutines: the tunnel forwarder and the user handler. Thetwo completion branches were asymmetric:
stdin/stdout pipe write ends, and then joined the handler goroutine
(
handlerErr = <-handlerChan) soawaitPaironly returned once both sides were done.select { case tunnelErr = <-tunnelChan: default: }. It did not cancel thecontext, did not close the pipe write ends, and did not join the tunnel
goroutine.
awaitPairreturned immediately while the tunnel goroutine kept running.Because the tunnel forwarder blocks reading from its stdin pipe, and the stdin write
end is never closed on the handler-first path, the tunnel goroutine blocks forever on
that read — it is orphaned/leaked. This is a goroutine leak plus a file-descriptor leak
(the pipe write ends are left open). The tunnel goroutine is never joined, so any
error it returns is also silently dropped.
Change
Make the handler-first branch symmetric with the tunnel-first branch: on handler
completion, cancel the context, close the stdout/stdin pipe write ends, and join
the tunnel goroutine (
tunnelErr = <-tunnelChan) before returning. This ensures bothgoroutines are reaped and their errors are surfaced through
ClassifyTunnelErrors,matching the existing tunnel-first behavior.
This is a focused correctness fix (no behavioral change for the already-correct
tunnel-first path), keeping
RunPairsymmetric regardless of which side completes first.Verification
task cli:format— clean.task cli:lint:ci— 0 new issues.task cli:test—pkg/tunnelpasses (ok ... coverage: 37.3%). The only failingpackage is
pkg/git(TestRepoClone*), a known pre-existing failure onorigin/mainunrelated to this change (my change does not touchpkg/git).TestRunPairJoinsTunnelWhenHandlerFinishesFirst:after the 2s test timeout with
tunnel goroutine was orphaned when handler finished first.closed and is joined before
RunPairreturns).task cli:build:grpcwas not required.This PR was created by an AI agent as part of an automated daily package review job.