tool
/
starrocks
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[UT] Fix memory leak in test LocalTabletsChannelTest::test_add_chunk_not_exist_tablet (#63218) 

## Why I'm doing
- ASAN reported a memory leak in BE unit test `LocalTabletsChannelTest::test_add_chunk_not_exist_tablet`
  - Direct leak from `SecondaryReplicasWaiter::_send_replica_status_request()` allocating `ReusableClosure<PLoadReplicaStatusResult>`
```
=================================================================
==752488==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 792 byte(s) in 1 object(s) allocated from:
    #0 0x1a830f48 in operator new(unsigned long) (/root/celerdata/be/ut_build_ASAN/test/starrocks_test+0x1a830f48)
    #1 0x30153219 in starrocks::SecondaryReplicasWaiter::_send_replica_status_request(int) /root/celerdata/be/src/runtime/local_tablets_channel.cpp:1342
    #2 0x30152d52 in starrocks::SecondaryReplicasWaiter::_try_check_replica_status_on_primary(int) /root/celerdata/be/src/runtime/local_tablets_channel.cpp:1327
    #3 0x301520e3 in starrocks::SecondaryReplicasWaiter::wait() /root/celerdata/be/src/runtime/local_tablets_channel.cpp:1285
    #4 0x3013603a in starrocks::LocalTabletsChannel::add_chunk(starrocks::Chunk*, starrocks::PTabletWriterAddChunkRequest const&, starrocks::PTabletWriterAddBatchResult*, bool*) /root/celerdata/be/src/runtime/local_tablets_channel.cpp:380
    #5 0x1eefff41 in starrocks::LocalTabletsChannelTest_test_add_chunk_not_exist_tablet_Test::TestBody() /root/celerdata/be/test/runtime/local_tablets_channel_test.cpp:255
    #6 0x37d95703 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) googletest-release-1.10.0/googletest/src/gtest.cc:2433
    #7 0x37d95703 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) googletest-release-1.10.0/googletest/src/gtest.cc:2469
    #8 0x37d8856d in testing::Test::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2508
    #9 0x37d8856d in testing::Test::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2498
    #10 0x37d886cd in testing::TestInfo::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2684
    #11 0x37d886cd in testing::TestInfo::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2657
    #12 0x37d887b6 in testing::TestSuite::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2816
    #13 0x37d887b6 in testing::TestSuite::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2795
    #14 0x37d88d13 in testing::internal::UnitTestImpl::RunAllTests() googletest-release-1.10.0/googletest/src/gtest.cc:5338
    #15 0x37d88f23 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) googletest-release-1.10.0/googletest/src/gtest.cc:2433
    #16 0x37d88f23 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) googletest-release-1.10.0/googletest/src/gtest.cc:2469
    #17 0x37d88f23 in testing::UnitTest::Run() googletest-release-1.10.0/googletest/src/gtest.cc:4925
    #18 0x1a893ea2 in RUN_ALL_TESTS() /var/local/thirdparty/installed/include/gtest/gtest.h:2473
    #19 0x1a87d8a4 in starrocks::init_test_env(int, char**) /root/celerdata/be/src/testutil/init_test_env.h:115
    #20 0x1a87e332 in main /root/celerdata/be/test/test_main.cpp:18
    #21 0x7f5813e29d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)

```
  
- Root cause:
  - The send path does `closure->ref()` twice (one for “self-hold”, one for “RPC-hold”).
  - The test does not register a sync-point callback for `LocalTabletsChannel::rpc::get_load_replica_status` to simulate the RPC, so no one calls `closure->Run()`, and the “RPC-hold” ref is never released.
  - The waiter’s release path only dropped one ref, leaving one outstanding reference and causing the leak.

## What I'm doing
- Introduce and use a sync-point primitive `TEST_SYNC_POINT_CALLBACK_OR_DEFAULT` with a default action, so tests without a registered callback still release the “RPC-hold” reference.
- At `LocalTabletsChannel::rpc::get_load_replica_status`, switch to a callback-with-default that calls `closure->Run()` when no test callback is installed. This balances the double `ref()` even in no-callback tests, while keeping production behavior unchanged.


Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
Signed-off-by: Kevin Cai <kevin.cai@celerdata.com>
This commit is contained in:
PengFei Li 2025-09-25 09:26:01 +08:00 committed by GitHub
parent 7e2c7cf94b
commit dfa190b48f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 10 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
be/test/runtime/local_tablets_channel_test.cpp
View File

@ -234,9 +234,17 @@ using RpcLoadDisagnosePair = std::pair<PLoadDiagnoseRequest*, ReusableClosure<PL
TEST_F(LocalTabletsChannelTest, test_add_chunk_not_exist_tablet) {
_create_tablets(1);
// open as a secondary replica of 3 replicas
ReplicaInfo replica_info{_tablets[0]->tablet_id(), _nodes};
_open_channel(_nodes[1].node_id(), {replica_info});
PTabletWriterOpenRequest request;
_create_open_request(_nodes[1].node_id(), {replica_info}, &request);
// turn off _is_replicated_storage to avoid launching secondary waiter
request.set_is_replicated_storage(false);
std::shared_ptr<OlapTableSchemaParam> schema_param(new OlapTableSchemaParam());
ASSERT_OK(schema_param->init(request.schema()));
PTabletWriterOpenResult response;
ASSERT_OK(_tablets_channel->open(request, &response, schema_param, false));
PTabletWriterAddChunkRequest add_chunk_request;
add_chunk_request.mutable_id()->CopyFrom(_load_id);