-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
x-pack/filebeat/input/httpjson: Close connections properly #39790
Conversation
This pull request doesn't have a |
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
defer func() { | ||
if httpResp != nil && httpResp.Body != nil { | ||
httpResp.Body.Close() | ||
} | ||
}() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The callees here all follow the normal invariant that if err is nil, then the response is non-nil. So the condition can go away. If the response is non-nil, the body should be non-nil (this is documented as a property of http.Response
values returned by http.Client
and http.Transport
and if we are not following that, it is a bug).
defer func() { | |
if httpResp != nil && httpResp.Body != nil { | |
httpResp.Body.Close() | |
} | |
}() | |
defer httpResp.Body.Close() |
Actually, looking closer, I do not believe that this ever needs to happen; all responses come from httpClient.do
which passes all bodies through drainBody
which both completely drains (which is not done here) and closes it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right. Now that I look more closely, I see:
err = resp.Body.Close() |
getIdsFromResponses
, which we can get rid of since we are already draining the body and closing it with drainBody
?
I'll remove my changes as well.
Do we want a note in changelog dev? |
Proposed commit message
Fix potential connection leaks in case of errors or when the response has been read but the connection remains unclosed.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Related issues