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 bfc09ae6930..836ffa9b014 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 4dd1f37db05..68b87da430c 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 bf9e75d0991..8d1d959da84 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: