Skip to content

fix: add null-check before buf_ allocation in ProcessQueryImpl#2165

Open
mukejane wants to merge 1 commit into
antgroup:mainfrom
mukejane:fix/processqueryimpl-memory-leak
Open

fix: add null-check before buf_ allocation in ProcessQueryImpl#2165
mukejane wants to merge 1 commit into
antgroup:mainfrom
mukejane:fix/processqueryimpl-memory-leak

Conversation

@mukejane

@mukejane mukejane commented Jun 8, 2026

Copy link
Copy Markdown

Fixes #2163

Root cause: ProcessQueryImpl unconditionally allocates computer.inner_computer_->buf_ without checking if it is already non-null. If SetQuery() is called more than once on the same Computer object, the previously allocated buffer is leaked.

Fix: Wrap the allocation in a null-check, matching the pattern already used in ScalarQuantizer:

// Before (leaks memory on repeated SetQuery calls):
try {
    computer.inner_computer_->buf_ =
        reinterpret_cast<uint8_t*>(this->allocator_->Allocate(this->query_code_size_));

// After (safe):
try {
    if (computer.inner_computer_->buf_ == nullptr) {
        computer.inner_computer_->buf_ =
            reinterpret_cast<uint8_t*>(this->allocator_->Allocate(this->query_code_size_));
    }

Also applies to the bad_alloc catch block — buf_ is now only set to nullptr when it was originally null, avoiding overwriting an existing allocation on rethrow.

@mergify

mergify Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Require kind label

Wonderful, this rule succeeded.
  • label~=^kind/

🟢 Require version label

Wonderful, this rule succeeded.
  • label~=^version/

🟢 Require linked issue for feature/bug PRs

Wonderful, this rule succeeded.
  • body~=(?im)(?:^|[\s\-\*])(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s+(?:#\d+|[\w.\-]+/[\w.\-]+#\d+|https?://github\.com/[\w.\-]+/[\w.\-]+/issues/\d+)

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates ProcessQueryImpl in transform_quantizer.h to only allocate memory for computer.inner_computer_->buf_ if it is currently null, avoiding unnecessary allocations. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Fixes antgroup#2163. Prevents memory leak when SetQuery() is called multiple times on the same Computer object. Matches the pattern used in ScalarQuantizer.

Signed-off-by: mukejane <mukejane@outlook.com>
@mukejane

mukejane commented Jun 8, 2026

Copy link
Copy Markdown
Author

Hi @antgroup/vsag — this PR fixes a null pointer bug in ProcessImplInner. Mergify is blocking merge because the "kind" label is missing. Could you please add kind/bug label so it can merge? Happy to help with anything else.

@wxyucs

wxyucs commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Hi @mukejane, thanks for your contribution! I added labels and triggered the CI Testing. Now, please wait for the testing.

@wxyucs wxyucs self-assigned this Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory leak in TransformQuantizer::ProcessQueryImpl on repeated SetQuery calls

2 participants