Skip to content

node: avoid double close resp.Body #31710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 2, 2025

Conversation

gzliudan
Copy link
Contributor

The functions rpcRequest and batchRpcRequest call baseRpcRequest. And resp.Body will be closed in the function baseRpcRequest later by t.Cleanup:

func baseRpcRequest(t *testing.T, url, bodyStr string, extraHeaders ...string) *http.Response {
        // ......
	t.Cleanup(func() { resp.Body.Close() })
	return resp
}

@gzliudan gzliudan requested a review from fjl as a code owner April 25, 2025 05:18
Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM

@lightclient
Copy link
Member

Looks like Martin recommended using the t.Cleanup function to generally close the resp.Body across many uses, but there were a few from the original node refactor in #21105.

@lightclient lightclient merged commit 86a4924 into ethereum:master May 2, 2025
3 of 4 checks passed
@lightclient lightclient added this to the 1.15.11 milestone May 2, 2025
@gzliudan gzliudan deleted the close-resp-body-once branch May 2, 2025 15:42
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