Skip to content
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

handle resource leaks #399

Merged
merged 2 commits into from
Feb 26, 2022
Merged

Conversation

dustin-decker
Copy link
Contributor

Description

This MR cleans up resource leaks of response bodies.

There are a few categories of changes:

  • closing resp.Body when the response is not returned and the interface{} provided to client.Do() is nil
  • instructing the caller to close resp.Body when a response is returned and the interface{} provided to client.Do() is nil

@benjivesterby
Copy link
Contributor

@dustin-decker can you please rebase on current master branch so I can merge this?

@benjivesterby
Copy link
Contributor

@dustin-decker Looks like you'll have to handle the conflicts here. I'm going to push a release without this change for now.

@dustin-decker
Copy link
Contributor Author

Github says there are no conflicts with the base branch after you merged master into it.

@andygrunwald
Copy link
Owner

This is looking good! Thanks @dustin-decker

@andygrunwald andygrunwald merged commit 1e16435 into andygrunwald:master Feb 26, 2022
@benjivesterby
Copy link
Contributor

This is looking good! Thanks @dustin-decker

The tests were broken so I just pushed a change to fix them. The merge on github wasn't complete like I thought.

@dustin-decker dustin-decker deleted the resource-leaks branch March 1, 2022 04:27
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