Skip to content

fix: memory: fix Bytes move assignment ownership transfer#82

Merged
lucasfang merged 6 commits into
alibaba:mainfrom
xylaaaaa:fix-bytes-move-assign
Feb 4, 2026
Merged

fix: memory: fix Bytes move assignment ownership transfer#82
lucasfang merged 6 commits into
alibaba:mainfrom
xylaaaaa:fix-bytes-move-assign

Conversation

@xylaaaaa

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: close #xxx

Fix the move assignment of Bytes to avoid the ~Bytes() + memcpy pattern that can cause double-free/UB. The new implementation explicitly releases current memory, takes ownership from other, and then nulls out the source.

Tests

Not run.

API and Format

No API/storage/protocol changes (internal implementation only).

Documentation

No.

Copilot AI review requested due to automatic review settings January 26, 2026 12:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the move-assignment implementation of paimon::Bytes to avoid a destructor + memcpy pattern that could lead to double-free or undefined behavior and instead performs an explicit, safe ownership transfer.

Changes:

  • Replace Bytes::operator=(Bytes&&) implementation from this->~Bytes(); memcpy; placement-new to explicit release of current buffer via pool_->Free followed by field-wise move of pool_, data_, and size_.
  • Clear the moved-from Bytes object by nulling pool_ and data_ and zeroing size_ to make its destructor a no-op and avoid double-free.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/paimon/common/memory/bytes.cpp
@xylaaaaa
xylaaaaa force-pushed the fix-bytes-move-assign branch from 381cdff to 0df4771 Compare February 3, 2026 07:32

@lucasfang lucasfang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

+1

@lucasfang
lucasfang merged commit 00fd06d into alibaba:main Feb 4, 2026
8 checks passed
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.

4 participants