[BugFix] Fix bRPC stub cache clean timer leak by kaijianding · Pull Request #75973 · StarRocks/starrocks

kaijianding · 2026-07-07T17:24:46Z

Why I'm doing:

there is mem leak caused by bRPC stub cache clean timer leak due to timer is unscheduled but not removed from mem.

std::shared_ptr<PInternalService_RecoverableStub> BrpcStubCache::get_stub(const butil::EndPoint& endpoint) {
    ...
    if (_timer->unschedule((*stub_pool)->_cleanup_task.get()) != TIMER_TASK_RUNNING) {
        timespec tm = butil::seconds_from_now(config::brpc_stub_expire_s);
        auto status = _timer->schedule((*stub_pool)->_cleanup_task.get(), tm);
        if (!status.ok()) {
            LOG(WARNING) << "Failed to schedule brpc cleanup task: " << endpoint;
        }
    }
}

every get_stub() will do _timer->unschedule() and _timer->schedule().
unlike what we expect, _timer->unschedule() will not remove internal objects, thus leak happens

What I'm doing:

extend timer deadline if the stub is still in use instead of unschedule()

Fixes #issue

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function
- This pr needs auto generate documentation
This is a backport pr

Bugfix cherry-pick branch check:

I have checked the version labels which the pr will be auto-backported to the target branch
- 4.1
- 4.0
- 3.5

CelerData-Reviewer · 2026-07-07T17:33:37Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 218a34af6a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

CelerData-Reviewer · 2026-07-08T01:08:46Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f88ceddd5a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

stdpain · 2026-07-08T02:43:11Z

I don't understand the cause of the memory leak. Could you explain it in more detail? This might not be a memory leak.

kaijianding · 2026-07-08T03:35:26Z

@stdpain the root cause of this leak is that unscheduled timer internal objects is not removed from brpc itself when timer.unschedule().

std::shared_ptr<PInternalService_RecoverableStub> BrpcStubCache::get_stub(const butil::EndPoint& endpoint) {
    ...
    if (_timer->unschedule((*stub_pool)->_cleanup_task.get()) != TIMER_TASK_RUNNING) {
        timespec tm = butil::seconds_from_now(config::brpc_stub_expire_s);
        auto status = _timer->schedule((*stub_pool)->_cleanup_task.get(), tm);
        if (!status.ok()) {
            LOG(WARNING) << "Failed to schedule brpc cleanup task: " << endpoint;
        }
    }
}

every get_stub() will do _timer->unschedule() and _timer->schedule().
unlike what we expect, _timer->unschedule() will not remove internal objects, thus leak happens

CelerData-Reviewer · 2026-07-08T05:14:17Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 62d84ff4a5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

stdpain · 2026-07-08T05:34:34Z

@stdpain the root cause of this leak is that unscheduled timer internal objects is not removed from brpc itself when timer.unschedule().
std::shared_ptr<PInternalService_RecoverableStub> BrpcStubCache::get_stub(const butil::EndPoint& endpoint) {
    ...
    if (_timer->unschedule((*stub_pool)->_cleanup_task.get()) != TIMER_TASK_RUNNING) {
        timespec tm = butil::seconds_from_now(config::brpc_stub_expire_s);
        auto status = _timer->schedule((*stub_pool)->_cleanup_task.get(), tm);
        if (!status.ok()) {
            LOG(WARNING) << "Failed to schedule brpc cleanup task: " << endpoint;
        }
    }
}
every get_stub() will do _timer->unschedule() and _timer->schedule(). unlike what we expect, _timer->unschedule() will not remove internal objects, thus leak happens

This doesn't actually cause a memory leak. Why would you think there's a leak here? Is it because you observed high memory usage from the schedule in the profile? brpc's rpc timeout is used in this way.

stdpain

I suspect the issue is caused by an unexpected blockage in the timer thread. This isn't the root cause; otherwise, regular RPC calls would also have the same problem.

kaijianding · 2026-07-08T09:06:36Z

This doesn't actually cause a memory leak. Why would you think there's a leak here? Is it because you observed high memory usage from the schedule in the profile? brpc's rpc timeout is used in this way.

staff from Mirrorship analyzed the heap dump for me. The heap dump indicates the timer objects leaks.

CelerData-Reviewer · 2026-07-08T15:03:19Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25d0bc3dbd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: kaijian.ding <kaijian.ding@gmail.com>

CelerData-Reviewer · 2026-07-13T14:42:00Z

@codex review

chatgpt-codex-connector · 2026-07-13T14:48:04Z

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: d01f6687b4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

CelerData-Reviewer · 2026-07-13T16:39:12Z

@codex review

chatgpt-codex-connector · 2026-07-13T16:43:58Z

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: f7a37db171

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Replace white-box _timer assertions with behavioral assertions via public API in the two singleton-reinitialize tests. Signed-off-by: kaijian.ding <kaijian.ding@gmail.com>

CelerData-Reviewer · 2026-07-14T03:01:35Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f3cd25612

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: kaijian.ding <kaijian.ding@gmail.com>

CelerData-Reviewer · 2026-07-14T06:12:49Z

@codex review

chatgpt-codex-connector · 2026-07-14T06:18:03Z

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 4b8ba5c04a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

github-actions · 2026-07-14T07:33:35Z

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2026-07-14T07:34:04Z

[FE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2026-07-14T07:50:13Z

[BE Incremental Coverage Report]

✅ pass : 101 / 113 (89.38%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	be/src/common/brpc/brpc_stub_cache.cpp	70	79	88.61%	[62, 137, 138, 277, 278, 279, 368, 369, 370]
🔵	be/src/common/brpc/brpc_stub_cache.h	31	34	91.18%	[91, 96, 98]

github-actions · 2026-07-14T12:05:19Z

@Mergifyio backport branch-4.1

github-actions · 2026-07-14T12:05:20Z

@Mergifyio backport branch-4.0

mergify · 2026-07-14T12:05:23Z

backport branch-4.1

✅ Backports have been created

Details

#76360 [BugFix] Fix bRPC stub cache clean timer leak (backport #75973) has been created for branch branch-4.1 but encountered conflicts

Cherry-pick of 2bbca67 has failed:

On branch mergify/bp/branch-4.1/pr-75973
Your branch is up to date with 'origin/branch-4.1'.

You are currently cherry-picking commit 2bbca67281.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   be/src/util/brpc_stub_cache.cpp
	both modified:   be/src/util/brpc_stub_cache.h
	both modified:   be/test/util/brpc_stub_cache_test.cpp

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

mergify · 2026-07-14T12:05:29Z

backport branch-4.0

✅ Backports have been created

Details

#76361 [BugFix] Fix bRPC stub cache clean timer leak (backport #75973) has been created for branch branch-4.0 but encountered conflicts

Cherry-pick of 2bbca67 has failed:

On branch mergify/bp/branch-4.0/pr-75973
Your branch is up to date with 'origin/branch-4.0'.

You are currently cherry-picking commit 2bbca67281.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   be/src/util/brpc_stub_cache.cpp
	both modified:   be/src/util/brpc_stub_cache.h
	both modified:   be/test/util/brpc_stub_cache_test.cpp

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

github-actions Bot added 4.0 3.5 4.1 labels Jul 7, 2026

mergify Bot assigned kaijianding Jul 7, 2026

github-actions Bot requested review from satanson and stdpain July 7, 2026 17:28

kaijianding force-pushed the fix_brpc_timer_leak branch from 203eb09 to 218a34a Compare July 7, 2026 17:30

chatgpt-codex-connector Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread be/src/common/brpc/brpc_stub_cache.h Outdated

kaijianding force-pushed the fix_brpc_timer_leak branch 3 times, most recently from 3d9d169 to f88cedd Compare July 8, 2026 01:07

chatgpt-codex-connector Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread be/src/common/brpc/brpc_stub_cache.h Outdated

kaijianding force-pushed the fix_brpc_timer_leak branch 2 times, most recently from 7648625 to 62d84ff Compare July 8, 2026 05:13

chatgpt-codex-connector Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread be/src/common/brpc/brpc_stub_cache.cpp

satanson previously approved these changes Jul 8, 2026

View reviewed changes

stdpain requested changes Jul 8, 2026

View reviewed changes

kaijianding dismissed satanson’s stale review via 25d0bc3 July 8, 2026 15:02

kaijianding force-pushed the fix_brpc_timer_leak branch from 62d84ff to 25d0bc3 Compare July 8, 2026 15:02

chatgpt-codex-connector Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread be/src/common/brpc/brpc_stub_cache.cpp Outdated

[BugFix] Fix bRPC stub cache clean timer leak

df737f5

Signed-off-by: kaijian.ding <kaijian.ding@gmail.com>

kaijianding force-pushed the fix_brpc_timer_leak branch from 25d0bc3 to df737f5 Compare July 8, 2026 15:24

kaijianding force-pushed the fix_brpc_timer_leak branch from d01f668 to 4b89d77 Compare July 13, 2026 14:43

kaijianding force-pushed the fix_brpc_timer_leak branch 2 times, most recently from 46726f6 to f7a37db Compare July 13, 2026 16:38

Remove BrpcStubCacheTest friend declaration from production header

9f3cd25

Replace white-box _timer assertions with behavioral assertions via public API in the two singleton-reinitialize tests. Signed-off-by: kaijian.ding <kaijian.ding@gmail.com>

kaijianding force-pushed the fix_brpc_timer_leak branch from f7a37db to 9f3cd25 Compare July 14, 2026 03:00

chatgpt-codex-connector Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread be/src/common/brpc/brpc_stub_cache.h Outdated

Comment thread be/src/common/brpc/brpc_stub_cache.cpp

kaijianding and others added 2 commits July 14, 2026 14:10

[BugFix] Fix UAF in brpc stub cache cleanup task on reinitialize

0d0722f

Signed-off-by: kaijian.ding <kaijian.ding@gmail.com>

style: auto-fix clang-format issues

4b8ba5c

stdpain approved these changes Jul 14, 2026

View reviewed changes

satanson approved these changes Jul 14, 2026

View reviewed changes

satanson merged commit 2bbca67 into StarRocks:main Jul 14, 2026
65 of 66 checks passed

github-actions Bot removed 4.1 4.0 labels Jul 14, 2026

This was referenced Jul 14, 2026

[BugFix] Fix bRPC stub cache clean timer leak (backport #75973) #76360

Closed

[BugFix] Fix bRPC stub cache clean timer leak (backport #75973) #76361

Closed

kaijianding deleted the fix_brpc_timer_leak branch July 15, 2026 05:48

Uh oh!

Conversation

kaijianding commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why I'm doing:

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

Uh oh!

CelerData-Reviewer commented Jul 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

CelerData-Reviewer commented Jul 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

stdpain commented Jul 8, 2026

Uh oh!

kaijianding commented Jul 8, 2026

Uh oh!

CelerData-Reviewer commented Jul 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

stdpain commented Jul 8, 2026

Uh oh!

stdpain left a comment

Choose a reason for hiding this comment

Uh oh!

kaijianding commented Jul 8, 2026

Uh oh!

CelerData-Reviewer commented Jul 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

CelerData-Reviewer commented Jul 13, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 13, 2026

Uh oh!

CelerData-Reviewer commented Jul 13, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 13, 2026

Uh oh!

CelerData-Reviewer commented Jul 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

CelerData-Reviewer commented Jul 14, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 14, 2026

Uh oh!

github-actions Bot commented Jul 14, 2026

[Java-Extensions Incremental Coverage Report]

Uh oh!

github-actions Bot commented Jul 14, 2026

[FE Incremental Coverage Report]

Uh oh!

github-actions Bot commented Jul 14, 2026

[BE Incremental Coverage Report]

file detail

Uh oh!

Uh oh!

kaijianding commented Jul 7, 2026 •

edited

Loading

mergify Bot commented Jul 14, 2026 •

edited

Loading

mergify Bot commented Jul 14, 2026 •

edited

Loading