neorados: Fix Neorados CephContext leak and prevent future ones

The original fix just dropped the extraneous reference. Decided it was
better to make explicitly when a reference was being taken, so
`make_with_cct` no longer accepts `CephContext*` and instead requires
`boost::intrusive_ptr<CephContext>`.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
(cherry picked from commit 949e80d0a6e522322d0e00a65feb93f5e13c4655)
Resolves: rhbz#2412500
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

diff --git a/src/include/neorados/RADOS.hpp b/src/include/neorados/RADOS.hpp
index a3ff6f8abefb0b4fef82940089dc820440330c13..f9a50678608528376f8dc9a34b3cab505f81fb1d 100644 (file)
--- a/src/include/neorados/RADOS.hpp
+++ b/src/include/neorados/RADOS.hpp
@@ -35,6 +35,8 @@
 #include <boost/asio/executor_work_guard.hpp>
 #include <boost/asio/io_context.hpp>
 
+#include <boost/smart_ptr/intrusive_ptr.hpp>
+
 #include <boost/container/flat_map.hpp>
 #include <boost/container/flat_set.hpp>
 #include <boost/uuid/uuid.hpp>
@@ -1352,20 +1354,31 @@ public:
                BuildComp c);
   };
 
-
+  /// We take an `intrusive_ptr<CephContext>` to make it clear that we
+  /// will take ownership of the reference.
+  /// We take an `intrusive_ptr<CephContext>` to make it clear that we
+  /// will take ownership of the reference.
   template<boost::asio::completion_token_for<BuildSig> CompletionToken>
-  static auto make_with_cct(CephContext* cct,
+  static auto make_with_cct(boost::intrusive_ptr<CephContext> cct,
                            boost::asio::io_context& ioctx,
                            CompletionToken&& token) {
     auto consigned = boost::asio::consign(
       std::forward<CompletionToken>(token), boost::asio::make_work_guard(
        boost::asio::get_associated_executor(token, ioctx.get_executor())));
     return boost::asio::async_initiate<decltype(consigned), BuildSig>(
-      [cct, &ioctx](auto&& handler) {
-       make_with_cct_(cct, ioctx, std::move(handler));
+      [cct = std::move(cct), &ioctx](auto&& handler) mutable {
+       make_with_cct_(std::move(cct), ioctx, std::move(handler));
       }, consigned);
   }
 
+  // Prevent passing a `CephContext*`.
+
+  template<boost::asio::completion_token_for<BuildSig> CompletionToken>
+  static auto make_with_cct(CephContext* cct,
+                           boost::asio::io_context& ioctx,
+                           CompletionToken&& token) = delete;
+
+
   static RADOS make_with_librados(librados::Rados& rados);
 
   RADOS(const RADOS&);
@@ -1812,7 +1825,7 @@ private:
   friend Builder;
 
   RADOS(std::shared_ptr<detail::Client> impl);
-  static void make_with_cct_(CephContext* cct,
+  static void make_with_cct_(boost::intrusive_ptr<CephContext> cct,
                             boost::asio::io_context& ioctx,
                             BuildComp c);
 

diff --git a/src/neorados/RADOS.cc b/src/neorados/RADOS.cc
index 4dcba611907fb5e67e9295794665d50b475c6610..6e8d338f45f66b6e579c2af5e7ca2b03e4937732 100644 (file)
--- a/src/neorados/RADOS.cc
+++ b/src/neorados/RADOS.cc
@@ -839,7 +839,11 @@ void RADOS::Builder::build_(asio::io_context& ioctx,
   if (no_mon_conf)
     flags |= CINIT_FLAG_NO_MON_CONFIG;
 
-  CephContext *cct = common_preinit(ci, env, flags);
+  boost::intrusive_ptr<CephContext> cct
+  {
+    common_preinit(ci, env, flags),
+    false //< So the intrusive_ptr adopts the reference we already have
+  };
   if (cluster)
     cct->_conf->cluster = *cluster;
 
@@ -875,7 +879,7 @@ void RADOS::Builder::build_(asio::io_context& ioctx,
   }
 
   if (!no_mon_conf) {
-    MonClient mc_bootstrap(cct, ioctx);
+    MonClient mc_bootstrap(cct.get(), ioctx);
     // TODO This function should return an error code.
     auto err = mc_bootstrap.get_monmap_and_config();
     if (err < 0) {
@@ -888,22 +892,22 @@ void RADOS::Builder::build_(asio::io_context& ioctx,
   if (!cct->_log->is_started()) {
     cct->_log->start();
   }
-  common_init_finish(cct);
+  common_init_finish(cct.get());
 
-  RADOS::make_with_cct(cct, ioctx, std::move(c));
+  RADOS::make_with_cct(std::move(cct), ioctx, std::move(c));
 }
 
-void RADOS::make_with_cct_(CephContext* cct,
+void RADOS::make_with_cct_(boost::intrusive_ptr<CephContext> cct,
                           asio::io_context& ioctx,
                           BuildComp c) {
   try {
     auto r = std::make_shared<detail::NeoClient>(
-      std::make_unique<detail::RADOS>(ioctx, cct));
-    r->objecter->wait_for_osd_map(
-      [c = std::move(c), r = std::move(r)]() mutable {
-       asio::dispatch(asio::append(std::move(c), bs::error_code{},
-                                   RADOS{std::move(r)}));
-      });
+      std::make_unique<detail::RADOS>(ioctx, std::move(cct)));
+    r->objecter->wait_for_osd_map([c = std::move(c),
+                                   r = std::move(r)]() mutable {
+      asio::dispatch(
+          asio::append(std::move(c), bs::error_code{}, RADOS{std::move(r)}));
+    });
   } catch (const bs::system_error& err) {
     asio::post(ioctx.get_executor(),
               asio::append(std::move(c), err.code(), RADOS{nullptr}));

diff --git a/src/rgw/rgw_sal.cc b/src/rgw/rgw_sal.cc
index ebf9220510a6a0b7923835aeb00d8855ef5827e5..cb34963fdec322a4895c047218c33af47348a331 100644 (file)
--- a/src/rgw/rgw_sal.cc
+++ b/src/rgw/rgw_sal.cc
@@ -73,8 +73,9 @@ extern rgw::sal::Driver* newD4NFilter(rgw::sal::Driver* next, boost::asio::io_co
 std::optional<neorados::RADOS>
 make_neorados(CephContext* cct, boost::asio::io_context& io_context) {
   try {
-    auto neorados = neorados::RADOS::make_with_cct(cct, io_context,
-                                                  ceph::async::use_blocked);
+    auto neorados = neorados::RADOS::make_with_cct(boost::intrusive_ptr{cct},
+                                                   io_context,
+                                                   ceph::async::use_blocked);
     return neorados;
   } catch (const std::exception& e) {
     ldout(cct, 0) << "Failed constructing neroados handle: " << e.what()

diff --git a/src/test/neorados/list_pool.cc b/src/test/neorados/list_pool.cc
index 95e05bd2ab738bb1350850f3d66f9f68b0705704..e94fe2cce8cdfd24ed74ff96b26b7c4e61d732d7 100644 (file)
--- a/src/test/neorados/list_pool.cc
+++ b/src/test/neorados/list_pool.cc
@@ -136,7 +136,7 @@ int main(int argc, char** argv)
 
   try {
     ca::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p, ca::use_blocked);
+    auto r = R::RADOS::make_with_cct(cct, p, ca::use_blocked);
 
     auto pool_name = get_temp_pool_name("ceph_test_RADOS_list_pool"sv);
     r.create_pool(pool_name, std::nullopt, ca::use_blocked);

diff --git a/src/test/neorados/start_stop.cc b/src/test/neorados/start_stop.cc
index 12ef9b5aa50b07a1023c6df373b4d17a8ac1b818..c94fad5e9522141a265433ca25543a7427bf01b8 100644 (file)
--- a/src/test/neorados/start_stop.cc
+++ b/src/test/neorados/start_stop.cc
@@ -41,135 +41,113 @@ int main(int argc, char** argv)
 
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(30s);
   }
   std::this_thread::sleep_for(30s);
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(30s);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(1s);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(1s);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(1s);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(1s);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(1s);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(1s);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(1s);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(500ms);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(500ms);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(50ms);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(50ms);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(50ms);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(5ms);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(5ms);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(5ms);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(5ms);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(5ms);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(5us);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(5us);
   }
   {
     ceph::async::io_context_pool p(1);
-    auto r = R::RADOS::make_with_cct(cct.get(), p,
-                                        boost::asio::use_future).get();
+    auto r = R::RADOS::make_with_cct(cct, p, boost::asio::use_future).get();
     std::this_thread::sleep_for(5us);
   }
   return 0;