From 32e2e907e52f083a7203fdeb8308e8e842a67a51 Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Mon, 6 Jul 2020 14:38:58 -0400
Subject: [PATCH] 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>
---
 src/neorados/RADOS.cc     |  2 +-
 src/neorados/RADOSImpl.cc | 34 +++++++++++++++++++++++-----------
 src/neorados/RADOSImpl.h  | 38 ++++----------------------------------
 3 files changed, 28 insertions(+), 46 deletions(-)

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:
 
-- 
2.47.3