Skip to content

event: close idle streams when the final listener is removed - #1223

Open
Rajvira10 wants to merge 1 commit into
fsouza:mainfrom
Rajvira10:fix-event-listener-connection-leak
Open

Rajvira10 wants to merge 1 commit into
fsouza:mainfrom
Rajvira10:fix-event-listener-connection-leak

Conversation

@Rajvira10

Copy link
Copy Markdown

Fixes #911.

Removing the final event listener only stopped the decoder's next iteration, leaving an idle connection blocked in Decode. Close the underlying socket to interrupt that read, and close the connection before draining the response body. Guard decoder errors with the monitor lock and session identity so shutdown cannot send to a closed error channel.

Keep each monitor and its retries attached to their original channels. If a connection finishes opening after its listener was removed, close it instead of installing its closer into a new session. Session-scoped shutdown prevents an old monitor from disabling a newly added listener.

The httptest regressions cover idle HTTP and TLS streams across listener re-addition, plus a delayed handshake that completes after a new session is active. All three cases fail against unchanged main and pass with this change. Event delivery behavior is otherwise unchanged.

Validation:

  • 100 repetitions of the event-listener tests with go test -race on Go 1.27.
  • Full go test -race -vet all ./... on Go 1.26.0 and Go 1.27.0.
  • staticcheck and golangci-lint v2.15.0.
  • No live Docker workloads or Docker integration tests.

Thanks to @shabicheng for the original report, @fsouza for #928, and @yyuuttaaoo for explaining the blocked decoder, body-close ordering, and shutdown-channel hazards in #911 and #930. This implementation uses the existing transport and does not copy #930's transport rewrite.

Implementation and regression tests were produced with OpenAI Codex.

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.

RemoveEventListener cause connection leak

1 participant