Skip to content

fix(memory): keep allocator ownership across Buffer.Reset#947

Merged
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:fix/buffer-reset-ownership
Jul 15, 2026
Merged

fix(memory): keep allocator ownership across Buffer.Reset#947
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:fix/buffer-reset-ownership

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

Buffer.Reset replaced an allocator-owned slice directly. That leaked the old allocation and later caused Release to pass the caller supplied slice to the allocator.

What changes are included in this PR?

When a buffer has an allocator, copy replacement data into allocator-owned storage and resize that storage without transferring ownership of the caller supplied slice. For shrinking resets, copying before resize also keeps resets from an overlapping slice safe if the allocator moves the allocation. Non-owning buffers keep the existing zero-copy behavior.

Are these changes tested?

Yes. Tests cover shrinking and growing from unrelated slices, source-slice independence, immediate release through Reset(nil), final allocator accounting, and an overlapping reset with an allocator that always moves and invalidates the old allocation. Package, race-enabled, and lint checks pass.

Are there any user-facing changes?

Resetting an allocator-backed buffer now copies the supplied data so ownership, growth, shrinking, overlapping resets, and release behavior remain valid.

@fallintoplace
fallintoplace requested a review from zeroshade as a code owner July 15, 2026 03:40
@fallintoplace
fallintoplace force-pushed the fix/buffer-reset-ownership branch from 5245076 to ef84976 Compare July 15, 2026 03:51
@fallintoplace
fallintoplace force-pushed the fix/buffer-reset-ownership branch from ef84976 to 67027be Compare July 15, 2026 04:19

@zeroshade zeroshade left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM — verified the fix and its test coverage. Thanks!

@zeroshade
zeroshade merged commit 52e43fe into apache:main Jul 15, 2026
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