fix: memleak in getTableList #33818

Tony2h · 2025-12-04T11:08:42Z

Description

Please briefly describe the code changes in this pull request.

Jira: https://jira.taosdata.com:18080/browse/TD-

Checklist

Please check the items in the checklist if applicable.

Is the user manual updated?
Are the test cases passed and automated?
Is there no significant decrease in test coverage?

gemini-code-assist · 2025-12-04T11:09:04Z

Summary of Changes

Hello @Tony2h, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily resolves a memory leak within the getTableList function, which occurs during specific cache population scenarios. It ensures that dynamically allocated memory for cache payloads is correctly deallocated when operations fail or yield no results. Additionally, the changes enhance the system's diagnostic capabilities by introducing more detailed logging for cache-related activities and refine error handling for duplicate cache entries.

Highlights

Memory Leak Fix: Addressed a memory leak in the getTableList function by ensuring that dynamically allocated payload memory is explicitly freed in error paths or when no data is retrieved for cache population.
Error Handling Refinement: Corrected the return code in metaUidFilterCachePut from TSDB_CODE_SUCCESS to TSDB_CODE_DUP_KEY when a duplicate key is encountered, ensuring proper error propagation.
Enhanced Logging and Observability: Improved debug and info logging across metaCache.c and executil.c to provide more detailed insights into cache operations, including origin keys and UID list sizes, aiding in diagnostics.
Test Case Updates: Modified existing test cases to enable a specific metaDebugFlag and updated log pattern matching to align with the newly introduced detailed log messages, ensuring test coverage for the changes.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

Copilot

Pull request overview

This PR fixes a memory leak in the getTableList function that occurred when attempting to add duplicate entries to the normal tag filter cache. The leak happened because metaUidFilterCachePut returned TSDB_CODE_DUP_KEY without freeing the allocated payload, and the caller didn't handle this case properly.

Key changes:

Fixed memory leak by properly freeing pPayload when cache insertion fails, including the duplicate key scenario
Updated metaUidFilterCachePut to return TSDB_CODE_DUP_KEY instead of TSDB_CODE_SUCCESS when duplicate is detected, allowing caller to handle it appropriately
Enhanced log messages with additional context (idstr, origin keys) for better debugging

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
source/libs/executor/src/executil.c	Added error handling to free pPayload when putCachedTableList fails, treating DUP_KEY as success after cleanup; improved log messages with additional debug context
source/dnode/vnode/src/meta/metaCache.c	Changed return code from SUCCESS to DUP_KEY when duplicate entry is detected, enabling proper cleanup by caller; added debug logging for cache eviction
test/cases/18-StreamProcessing/99-Others/test_stream_tag_cache.py	Updated test to enable metaDebugFlag and adjusted log message patterns to match new log format in C code

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

Copilot · 2025-12-04T11:15:52Z

source/libs/executor/src/executil.c

+        if (tmp == NULL) {
+          taosMemFreeClear(pPayload);
+        }
        QUERY_CHECK_NULL(tmp, code, lino, _end, terrno);


The logic here is problematic. After checking if tmp == NULL and freeing pPayload, the code still executes QUERY_CHECK_NULL(tmp, ...) which will unconditionally jump to _end label when tmp is NULL. This creates unreachable code and a confusing control flow.

If tmp is NULL, the code should either:

Free pPayload and return/goto immediately without the second check, or

Only use QUERY_CHECK_NULL without the manual NULL check and free

Suggestion: Remove the manual NULL check and free, and let QUERY_CHECK_NULL handle it by jumping to _end, then ensure cleanup happens at the _end label.

Copilot · 2025-12-04T11:15:53Z

source/libs/executor/src/executil.c

-      qInfo("suid:%" PRIu64 ", %s add uid list to normal tag filter cache, "
-        "uidListSize:%d", pScanNode->suid, idstr,
-        (int32_t)taosArrayGetSize(pUidList));
+      qInfo("%s, suid:%" PRIu64 " add uid list to normal tag filter cache, "


Missing comma after suid formatting in log message. The format string should be "suid:%" PRIu64 ", add uid list to normal tag filter cache" to match the consistent formatting pattern used in line 2230.

Suggested change

qInfo("%s, suid:%" PRIu64 " add uid list to normal tag filter cache, "

qInfo("%s, suid:%" PRIu64 ", add uid list to normal tag filter cache, "

gemini-code-assist

Code Review

This pull request fixes a memory leak in the getTableList function by ensuring that allocated memory for pPayload is freed in all error paths. The changes also involve improving log messages for better debugging and adjusting the return value for duplicate keys in the cache. While the memory leak fix is correct, there's an opportunity to refactor the error handling for better code clarity and to remove redundant checks. Overall, the changes are beneficial and address the reported issue.

gemini-code-assist · 2025-12-04T11:16:28Z

source/libs/executor/src/executil.c

+        if (tmp == NULL) {
+          taosMemFreeClear(pPayload);
+        }
        QUERY_CHECK_NULL(tmp, code, lino, _end, terrno);


While adding if (tmp == NULL) to free pPayload correctly fixes a potential memory leak, the check is redundant since QUERY_CHECK_NULL on the next line performs the same null check. This can be simplified by handling the NULL case in a single block and then jumping to the error handling label.

if (taosArrayGet(pUidList, 0) == NULL) { taosMemFreeClear(pPayload); code = terrno; lino = __LINE__; goto _end; } void* tmp = taosArrayGet(pUidList, 0);

…leList function

… memory allocation

…debug message

dapan1121 · 2025-12-05T03:42:02Z

source/libs/executor/src/executil.c

                                                    pPayload, size, 1);
-      QUERY_CHECK_CODE(code, lino, _error);
+      if (TSDB_CODE_DUP_KEY == code) {
+        code = TSDB_CODE_SUCCESS;  // ignore duplicate key error


DUP_KEY时没有释放payload

…etting pPayload and handling duplicate key errors

enh: improve logging for stable and normal tag filter cache operations

3ff9900

Copilot AI review requested due to automatic review settings December 4, 2025 11:08

Tony2h requested review from a team, dapan1121, guanshengliang and hzcheng as code owners December 4, 2025 11:08

taosdata-bot bot added the team engine label Dec 4, 2025

Copilot started reviewing on behalf of Tony2h December 4, 2025 11:09 View session

Copilot finished reviewing on behalf of Tony2h December 4, 2025 11:11

Copilot AI reviewed Dec 4, 2025

View reviewed changes

gemini-code-assist bot reviewed Dec 4, 2025

View reviewed changes

Tony Zhang added 3 commits December 4, 2025 19:34

fix(executil): improve error handling and memory management in getTab…

412a6b6

…leList function

fix(executil): cast numOfTables to int32_t for consistent logging and…

6e5a311

… memory allocation

fix(executil): enhance logging by including identifier in table list …

1f13152

…debug message

dapan1121 reviewed Dec 5, 2025

View reviewed changes

fix(executil): improve error handling in getTableList function by res…

e472241

…etting pPayload and handling duplicate key errors

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: memleak in getTableList #33818

fix: memleak in getTableList #33818

Tony2h commented Dec 4, 2025

Uh oh!

gemini-code-assist bot commented Dec 4, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Tony2h Dec 5, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Tony2h Dec 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Dec 4, 2025

Uh oh!

Tony2h Dec 5, 2025

Uh oh!

dapan1121 Dec 5, 2025

Uh oh!

Tony2h Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	qInfo("%s, suid:%" PRIu64 " add uid list to normal tag filter cache, "
	qInfo("%s, suid:%" PRIu64 ", add uid list to normal tag filter cache, "

fix: memleak in getTableList #33818

Are you sure you want to change the base?

fix: memleak in getTableList #33818

Conversation

Tony2h commented Dec 4, 2025

Description

Checklist

Uh oh!

gemini-code-assist bot commented Dec 4, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Tony2h Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Tony2h Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Tony2h Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

dapan1121 Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Tony2h Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants