Skip to content

Commit

Permalink
test/common: avoid leakage of CephContext
Browse files Browse the repository at this point in the history
before this change, in test_util.cc, we increment the refcount of
when constructing it. but at that moment, nobody really owns it.
also, `CephContext` 's refcount is set to 1 in its constructor.
so, we should not do this. otherwise, the created `CephContext`
is leaked as LeakSanitizer rightly points out:
```
Indirect leak of 10880000 byte(s) in 1 object(s) allocated from:
    #0 0x5632320d27ed in operator new(unsigned long) (/home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/unittest_util+0x1917ed) (BuildId: ff1df1455bd07b651ad580584a17ea204afeb36e)
    #1 0x7ff9d535b189 in __gnu_cxx::new_allocator<ceph::logging::ConcreteEntry>::allocate(unsigned long, void const*) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/ext/new_allocator.h:127:27
    #2 0x7ff9d535a563 in std::allocator<ceph::logging::ConcreteEntry>::allocate(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/allocator.h:185:32
    #3 0x7ff9d535a563 in boost::circular_buffer<ceph::logging::ConcreteEntry, std::allocator<ceph::logging::ConcreteEntry> >::allocate(unsigned long) /opt/ceph/include/boost/circular_buffer/base.hpp:2396:39
    #4 0x7ff9d535a2c0 in boost::circular_buffer<ceph::logging::ConcreteEntry, std::allocator<ceph::logging::ConcreteEntry> >::initialize_buffer(unsigned long) /opt/ceph/include/boost/circular_buffer/base.hpp:2494:18
    #5 0x7ff9d5354192 in boost::circular_buffer<ceph::logging::ConcreteEntry, std::allocator<ceph::logging::ConcreteEntry> >::circular_buffer(unsigned long, std::allocator<ceph::logging::ConcreteEntry> const&) /opt/ceph/include/boost/circular_buffer/base.hpp:1039:9
    #6 0x7ff9d53471e4 in ceph::logging::Log::Log(ceph::logging::SubsystemMap const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/log/Log.cc:53:5
    #7 0x7ff9d461d96d in ceph::common::CephContext::CephContext(unsigned int, ceph::common::CephContext::create_options const&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/common/ceph_context.cc:729:16
    #8 0x7ff9d461c93b in ceph::common::CephContext::CephContext(unsigned int, code_environment_t, int) /home/jenkins-build/build/workspace/ceph-pull-requests/src/common/ceph_context.cc:697:5
    #9 0x5632320d52e0 in util_collect_sys_info_Test::TestBody() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/common/test_util.cc:34:27
    #10 0x563232205c16 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2605:10
    ceph#11 0x5632321c2742 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2641:14
    ceph#12 0x5632321736dc in testing::Test::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2680:5
```
in this change, instead of using a raw pointer, let's
use `boost::intrusive_ptr<CephContext>` to manage the lifecyle
of `CephContext`, this also address the leakage reported by
LeakSanitizer.

the same applies to common/test_context.cc

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
  • Loading branch information
tchaikov committed Mar 25, 2024
1 parent 5611b7a commit 201386b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 7 deletions.
5 changes: 2 additions & 3 deletions src/test/common/test_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ using namespace std;

TEST(CephContext, do_command)
{
CephContext *cct = (new CephContext(CEPH_ENTITY_TYPE_CLIENT))->get();
boost::intrusive_ptr<CephContext> cct{new CephContext(CEPH_ENTITY_TYPE_CLIENT), false};

cct->_conf->cluster = "ceph";

Expand Down Expand Up @@ -89,12 +89,11 @@ TEST(CephContext, do_command)
string s(out.c_str(), out.length());
EXPECT_EQ("<config_diff_get><diff><key><default></default><override>" + value + "</override><final>value</final></key><rbd_default_features><default>61</default><final>61</final></rbd_default_features><rbd_qos_exclude_ops><default>0</default><final>0</final></rbd_qos_exclude_ops></diff></config_diff_get>", s);
}
cct->put();
}

TEST(CephContext, experimental_features)
{
CephContext *cct = (new CephContext(CEPH_ENTITY_TYPE_CLIENT))->get();
boost::intrusive_ptr<CephContext> cct{new CephContext(CEPH_ENTITY_TYPE_CLIENT), false};

cct->_conf->cluster = "ceph";

Expand Down
8 changes: 4 additions & 4 deletions src/test/common/test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ TEST(util, collect_sys_info)

map<string, string> sys_info;

CephContext *cct = (new CephContext(CEPH_ENTITY_TYPE_CLIENT))->get();
collect_sys_info(&sys_info, cct);
boost::intrusive_ptr<CephContext> cct{new CephContext(CEPH_ENTITY_TYPE_CLIENT), false};

collect_sys_info(&sys_info, cct.get());

ASSERT_TRUE(sys_info.find("distro") != sys_info.end());
ASSERT_TRUE(sys_info.find("distro_description") != sys_info.end());

cct->put();
}

#endif

0 comments on commit 201386b

Please sign in to comment.