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(duplication): fix the dangling pointer after blob object is moved #2088

Merged
merged 24 commits into from
Aug 15, 2024

Conversation

empiredan
Copy link
Contributor

@empiredan empiredan commented Aug 3, 2024

#2089

Refactor blob object to avoid the dangling pointer leading to heap-use-after-free
error which happened in #2089 after blob object is moved. Also refactor and add
some tests for blob and mutation_batch.

@github-actions github-actions bot added the cpp label Aug 3, 2024
@empiredan empiredan changed the title fix(test): fix heap-use-after-free error by mutation_batch::add_mutation_if_valid fix(duplication): fix heap-use-after-free error by mutation_batch::add_mutation_if_valid Aug 3, 2024
@empiredan empiredan added the type/bug-fix This PR fixes a bug. label Aug 3, 2024
bb = std::move(update.data);
} else {
bb = blob::create_from_bytes(update.data.data(), update.data.length());
}
Copy link
Member

Choose a reason for hiding this comment

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

While it's possible the _holder is nullptr and _data has a valid data pointer, so it's needed to process the latter case.

@empiredan empiredan marked this pull request as ready for review August 12, 2024 07:25
@empiredan empiredan changed the title fix(duplication): fix heap-use-after-free error by mutation_batch::add_mutation_if_valid fix(duplication): fix the dangling pointer after blob object is moved Aug 12, 2024
@acelyc111
Copy link
Member

Good work! Could you please rebase the latest master branch to check if there are any clang-tidy warnings or errors.

You don't have to fix all the warnings/errors, some rules can be disabled after discussion.

src/utils/blob.h Outdated Show resolved Hide resolved
acelyc111
acelyc111 previously approved these changes Aug 14, 2024
@empiredan empiredan merged commit e68c2b9 into apache:master Aug 15, 2024
81 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants