Skip to content

Close the response body in the wait command's poll loop - #2700

Draft
rootkiller6788 wants to merge 2 commits into
GoogleCloudPlatform:mainfrom
rootkiller6788:fix-wait-response-body-leak
Draft

rootkiller6788 wants to merge 2 commits into
GoogleCloudPlatform:mainfrom
rootkiller6788:fix-wait-response-body-leak

Conversation

@rootkiller6788

Copy link
Copy Markdown

The wait subcommand polls the other proxy's /startup endpoint once a second until it gets a 200, and it never closed the response body. Go only returns a connection to the pool if the body is read and closed, so every poll opened a fresh connection to the proxy instead of reusing one. With the default 30s --max that's up to 30 connections, and the command is meant to be run in tight sidecar/init-container loops.

The fix reads the body into io.Discard and closes it before checking the status code, so both the ready and the still-starting paths release it. Behaviour is otherwise unchanged: same retry cadence, same timeout.

The test installs a RoundTripper on http.DefaultClient that keeps a handle on the bodies it hands out, then runs wait against a 200 and a 503 so both exit paths are covered. It fails on current main with response body was left open and passes with the fix.

The wait subcommand hits the startup endpoint once a second until it
gets a 200 and never closed the response body. Go can't hand the
connection back to the pool unless the body is read and closed, so
each poll burned a new connection to the proxy.

Read the body into io.Discard and close it before checking the status
code, so both the ready and the still-starting paths release it.
Adds a test that swaps in a RoundTripper which keeps a handle on the
response bodies it returns, then runs wait against both a 200 and a
503 so both exit paths are covered. Fails without the fix.
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