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: slices clone related items #473

Merged
merged 6 commits into from
Sep 30, 2024
Merged

fix: slices clone related items #473

merged 6 commits into from
Sep 30, 2024

Conversation

muktihari
Copy link
Owner

Currently we use append(s[:0:0], s...) to clone a slice from an array pool. That approach comes with drawback if the slice happen to have zero len, the returning slice will share the same backing array of the array pool, causing array pool can not be freed until the returning slice become unreachable. More detail is declared here: golang/go#68488 (comment)

This pattern was inspired from slices.Clone's CL that was first introduced in Go version v1.22.0. A new CL has been merged into Go's main branch to fix this issue but it seems without a backport request, so it might not be available until v1.24.0. If that's the case, people using v1.22.0 to v1.23.X are still being affected unless we do something about it. Since we typically clone a slice from an array pool, let's create our own clone function that better suite that case.

@muktihari muktihari added the bug Something isn't working label Sep 30, 2024
@muktihari muktihari added this to the 0.24.0 milestone Sep 30, 2024
@muktihari muktihari self-assigned this Sep 30, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.94%. Comparing base (85fb3b9) to head (f65c526).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #473   +/-   ##
=======================================
  Coverage   99.94%   99.94%           
=======================================
  Files          42       42           
  Lines        3689     3689           
=======================================
  Hits         3687     3687           
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@muktihari muktihari merged commit aa68a94 into master Sep 30, 2024
5 checks passed
@muktihari muktihari deleted the fix/slices-clone branch September 30, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants