From: Jason Dillaman <dillaman@redhat.com>
Date: Mon, 6 Jul 2020 18:38:58 +0000 (-0400)
Subject: neorados: fixed valgrind memory leaks and errors
X-Git-Tag: v16.1.0~1799^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=32e2e907e52f083a7203fdeb8308e8e842a67a51;p=ceph.git

neorados: fixed valgrind memory leaks and errors

Keep the CephContext wrapped in an intrusive pointer to keep the
reference counting functional. Ensure all the messenger clients
(mon, mgr, and objecter) are shut down prior to destruction.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/neorados/RADOS.cc b/src/neorados/RADOS.cc
index bfc09ae69300..836ffa9b014a 100644
--- a/src/neorados/RADOS.cc
+++ b/src/neorados/RADOS.cc
@@ -1637,7 +1637,7 @@ const bs::error_category& error_category() noexcept {
 }
 
 CephContext* RADOS::cct() {
-  return impl->cct;
+  return impl->cct.get();
 }
 }
 
diff --git a/src/neorados/RADOSImpl.cc b/src/neorados/RADOSImpl.cc
index 4dd1f37db058..68b87da430c7 100644
--- a/src/neorados/RADOSImpl.cc
+++ b/src/neorados/RADOSImpl.cc
@@ -25,18 +25,17 @@ namespace neorados {
 namespace detail {
 
 RADOS::RADOS(boost::asio::io_context& ioctx,
-	     boost::intrusive_ptr<CephContext> _cct)
-  : Dispatcher(_cct.detach()),
+	     boost::intrusive_ptr<CephContext> cct)
+  : Dispatcher(cct.get()),
     ioctx(ioctx),
-    monclient(cct, ioctx),
-    moncsd(monclient),
-    mgrclient(cct, nullptr, &monclient.monmap),
-    mgrcsd(mgrclient) {
+    cct(cct),
+    monclient(cct.get(), ioctx),
+    mgrclient(cct.get(), nullptr, &monclient.monmap) {
   auto err = monclient.build_initial_monmap();
   if (err < 0)
     throw std::system_error(ceph::to_error_code(err));
 
-  messenger.reset(Messenger::create_client_messenger(cct, "radosclient"));
+  messenger.reset(Messenger::create_client_messenger(cct.get(), "radosclient"));
   if (!messenger)
     throw std::bad_alloc();
 
@@ -46,7 +45,7 @@ RADOS::RADOS(boost::asio::io_context& ioctx,
   messenger->set_default_policy(
     Messenger::Policy::lossy_client(CEPH_FEATURE_OSDREPLYMUX));
 
-  objecter.reset(new Objecter(cct, messenger.get(), &monclient,
+  objecter.reset(new Objecter(cct.get(), messenger.get(), &monclient,
 			      ioctx,
 			      cct->_conf->rados_mon_op_timeout,
 			      cct->_conf->rados_osd_op_timeout));
@@ -87,6 +86,20 @@ RADOS::RADOS(boost::asio::io_context& ioctx,
   instance_id = monclient.get_global_id();
 }
 
+RADOS::~RADOS() {
+  if (objecter && objecter->initialized) {
+    objecter->shutdown();
+  }
+
+  mgrclient.shutdown();
+  monclient.shutdown();
+
+  if (messenger) {
+    messenger->shutdown();
+    messenger->wait();
+  }
+}
+
 bool RADOS::ms_dispatch(Message *m)
 {
   switch (m->get_type()) {
@@ -107,6 +120,5 @@ bool RADOS::ms_handle_refused(Connection *con) {
   return false;
 }
 
-RADOS::~RADOS() = default;
-}
-}
+} // namespace detail
+} // namespace neorados
diff --git a/src/neorados/RADOSImpl.h b/src/neorados/RADOSImpl.h
index bf9e75d09919..8d1d959da840 100644
--- a/src/neorados/RADOSImpl.h
+++ b/src/neorados/RADOSImpl.h
@@ -37,49 +37,19 @@ namespace detail {
 class RADOS : public Dispatcher
 {
   friend ::neorados::RADOS;
-  struct MsgDeleter {
-    void operator()(Messenger* p) const {
-      if (p) {
-	p->shutdown();
-	p->wait();
-      }
-      delete p;
-    }
-  };
-
-  struct ObjDeleter {
-    void operator()(Objecter* p) const {
-      if (p) {
-	p->shutdown();
-      }
-      delete p;
-    }
-  };
-
-  template<typename T>
-  struct scoped_shutdown {
-    T& m;
-    scoped_shutdown(T& m) : m(m) {}
-
-    ~scoped_shutdown() {
-      m.shutdown();
-    }
-  };
 
   boost::asio::io_context& ioctx;
+  boost::intrusive_ptr<CephContext> cct;
+
   ceph::mutex lock = ceph::make_mutex("RADOS_unleashed::_::RADOSImpl");
   int instance_id = -1;
 
-  std::unique_ptr<Messenger, MsgDeleter> messenger;
+  std::unique_ptr<Messenger> messenger;
 
   MonClient monclient;
-  scoped_shutdown<MonClient> moncsd;
-
   MgrClient mgrclient;
-  scoped_shutdown<MgrClient> mgrcsd;
-
-  std::unique_ptr<Objecter, ObjDeleter> objecter;
 
+  std::unique_ptr<Objecter> objecter;
 
 public: