From: Kefu Chai <tchaikov@gmail.com>
Date: Sun, 24 Mar 2024 23:43:26 +0000 (+0800)
Subject: test/common: avoid leakage of CephContext
X-Git-Tag: testing/wip-batrick-testing-20240411.154038~132^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=e316c2e481d4afd446b5baf477487b7505966414;p=ceph-ci.git

test/common: avoid leakage of CephContext

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
    #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
    #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>
---

diff --git a/src/test/common/test_context.cc b/src/test/common/test_context.cc
index 2c846a9ae72..889d000da85 100644
--- a/src/test/common/test_context.cc
+++ b/src/test/common/test_context.cc
@@ -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";
 
@@ -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";
 
diff --git a/src/test/common/test_util.cc b/src/test/common/test_util.cc
index 91ac771f804..8eb989d3047 100644
--- a/src/test/common/test_util.cc
+++ b/src/test/common/test_util.cc
@@ -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