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

Fix misuse of sync.Pool #75

Closed
wants to merge 1 commit into from
Closed

Conversation

darhwa
Copy link
Contributor

@darhwa darhwa commented Jun 20, 2020

1. What does this change do, exactly?

The buffers got from bufferPool are never put back. They will still get GCed sometime later, but that's not the purpose of using sync.Pool

2. Please link to the relevant issues.

N/A

3. Which documentation changes (if any) need to be made because of this PR?

None.

4. Checklist

  • I have written tests and verified that they fail without my change
  • I made pull request as minimal and simple as possible. If change is not small or additional dependencies are required, I opened an issue to propose and discuss the design first
  • I have squashed any insignificant commits
  • This change has comments for package types, values, functions, and non-obvious lines of code

@mholt
Copy link
Member

mholt commented Jun 20, 2020

Thanks for the PR!

Can you actually make this against the caddy2 branch, here: #74 ? Because the v1 / master version is no longer being maintained at this point, I guess.

@darhwa
Copy link
Contributor Author

darhwa commented Jun 20, 2020

@mholt

Thanks for the prompt reply!

I've created a new one (#76) against caddy2. Think this can be closed.

@darhwa darhwa closed this Jun 20, 2020
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.

2 participants