mon/MonClient: use std::unique_ptr<>; fix leak

We were leaking several heap items from
LibRadosMiscConnectFailure.ConnectFailure when connect() was called
multiple times (after failure).  Use std::unique_ptr to avoid maintaining
fragile cleanup paths.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/mds/MDSDaemon.cc b/src/mds/MDSDaemon.cc
index dfbc44a4b84128040b160efb8473249a022e9117..58f59f60a6388532499e8786ea4586a61af09039 100644 (file)
--- a/src/mds/MDSDaemon.cc
+++ b/src/mds/MDSDaemon.cc
@@ -1324,8 +1324,9 @@ bool MDSDaemon::ms_verify_authorizer(Connection *con, int peer_type,
   EntityName name;
   uint64_t global_id;
 
-  is_valid = authorize_handler->verify_authorizer(cct, monc->rotating_secrets,
-                                                 authorizer_data, authorizer_reply, name, global_id, caps_info, session_key);
+  is_valid = authorize_handler->verify_authorizer(
+    cct, monc->rotating_secrets.get(),
+    authorizer_data, authorizer_reply, name, global_id, caps_info, session_key);
 
   if (is_valid) {
     entity_name_t n(con->get_peer_type(), global_id);

diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc
index 57d115269cc42e1131792711d8daae7dee7345fd..995228ae4017e9096e45f4100e8fded922006360 100644 (file)
--- a/src/mgr/DaemonServer.cc
+++ b/src/mgr/DaemonServer.cc
@@ -89,11 +89,12 @@ bool DaemonServer::ms_verify_authorizer(Connection *con,
   EntityName name;
   uint64_t global_id = 0;
 
-  is_valid = handler->verify_authorizer(cct, monc->rotating_secrets,
-                                                 authorizer_data,
-                                                  authorizer_reply, name,
-                                                  global_id, caps_info,
-                                                  session_key);
+  is_valid = handler->verify_authorizer(
+    cct, monc->rotating_secrets.get(),
+    authorizer_data,
+    authorizer_reply, name,
+    global_id, caps_info,
+    session_key);
 
   // TODO: invent some caps suitable for ceph-mgr
 

diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc
index 3db182f6da2838cc2f1073b74b6b41a757705469..b6634603f91fad483b1dabc7100000857337e4e4 100644 (file)
--- a/src/mon/MonClient.cc
+++ b/src/mon/MonClient.cc
@@ -53,17 +53,12 @@ MonClient::MonClient(CephContext *cct_) :
   no_keyring_disabled_cephx(false),
   log_client(NULL),
   more_log_pending(false),
-  auth_supported(NULL),
   hunting(true),
   want_monmap(true),
   want_keys(0), global_id(0),
   authenticate_err(0),
-  session_established_context(NULL),
   had_a_connection(false),
   reopen_interval_multiplier(1.0),
-  auth(NULL),
-  keyring(NULL),
-  rotating_secrets(NULL),
   last_mon_command_tid(0),
   version_req_id(0)
 {
@@ -71,11 +66,6 @@ MonClient::MonClient(CephContext *cct_) :
 
 MonClient::~MonClient()
 {
-  delete auth_supported;
-  delete session_established_context;
-  delete auth;
-  delete keyring;
-  delete rotating_secrets;
 }
 
 int MonClient::build_initial_monmap()
@@ -358,19 +348,19 @@ int MonClient::init()
   Mutex::Locker l(monc_lock);
 
   string method;
-    if (!cct->_conf->auth_supported.empty())
-      method = cct->_conf->auth_supported;
-    else if (entity_name.get_type() == CEPH_ENTITY_TYPE_OSD ||
-             entity_name.get_type() == CEPH_ENTITY_TYPE_MDS ||
-             entity_name.get_type() == CEPH_ENTITY_TYPE_MON)
-      method = cct->_conf->auth_cluster_required;
-    else
-      method = cct->_conf->auth_client_required;
-  auth_supported = new AuthMethodList(cct, method);
+  if (!cct->_conf->auth_supported.empty())
+    method = cct->_conf->auth_supported;
+  else if (entity_name.get_type() == CEPH_ENTITY_TYPE_OSD ||
+          entity_name.get_type() == CEPH_ENTITY_TYPE_MDS ||
+          entity_name.get_type() == CEPH_ENTITY_TYPE_MON)
+    method = cct->_conf->auth_cluster_required;
+  else
+    method = cct->_conf->auth_client_required;
+  auth_supported.reset(new AuthMethodList(cct, method));
   ldout(cct, 10) << "auth_supported " << auth_supported->get_supported_set() << " method " << method << dendl;
 
   int r = 0;
-  keyring = new KeyRing; // initializing keyring anyway
+  keyring.reset(new KeyRing); // initializing keyring anyway
 
   if (auth_supported->is_supported_auth(CEPH_AUTH_CEPHX)) {
     r = keyring->from_ceph_context(cct);
@@ -389,7 +379,8 @@ int MonClient::init()
     return r;
   }
 
-  rotating_secrets = new RotatingKeyRing(cct, cct->get_module_type(), keyring);
+  rotating_secrets.reset(
+    new RotatingKeyRing(cct, cct->get_module_type(), keyring.get()));
 
   initialized = true;
 
@@ -481,8 +472,8 @@ void MonClient::handle_auth(MAuthReply *m)
   bufferlist::iterator p = m->result_bl.begin();
   if (state == MC_STATE_NEGOTIATING) {
     if (!auth || (int)m->protocol != auth->get_protocol()) {
-      delete auth;
-      auth = get_auth_client_handler(cct, m->protocol, rotating_secrets);
+      auth.reset(get_auth_client_handler(cct, m->protocol,
+                                        rotating_secrets.get()));
       if (!auth) {
        ldout(cct, 10) << "no handler for protocol " << m->protocol << dendl;
        if (m->result == -ENOTSUP) {
@@ -552,8 +543,7 @@ void MonClient::handle_auth(MAuthReply *m)
        send_log();
       }
       if (session_established_context) {
-        cb = session_established_context;
-        session_established_context = NULL;
+        cb = session_established_context.release();
       }
     }
   

diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h
index d2a29953ff4c5b1382dea4643d75fd3ab63840a4..2c029afab39930638ef180f034d74a3b1ecf68c5 100644 (file)
--- a/src/mon/MonClient.h
+++ b/src/mon/MonClient.h
@@ -131,7 +131,7 @@ private:
 
   void send_log();
 
-  AuthMethodList *auth_supported;
+  std::unique_ptr<AuthMethodList> auth_supported;
 
   bool ms_dispatch(Message *m);
   bool ms_handle_reset(Connection *con);
@@ -164,7 +164,7 @@ private:
 
   list<Message*> waiting_for_session;
   utime_t last_rotating_renew_sent;
-  Context *session_established_context;
+  std::unique_ptr<Context> session_established_context;
   bool had_a_connection;
   double reopen_interval_multiplier;
 
@@ -243,7 +243,7 @@ private:
 
   // auth tickets
 public:
-  AuthClientHandler *auth;
+  std::unique_ptr<AuthClientHandler> auth;
 public:
   void renew_subs() {
     Mutex::Locker l(monc_lock);
@@ -286,8 +286,8 @@ public:
     return false;
   }
   
-  KeyRing *keyring;
-  RotatingKeyRing *rotating_secrets;
+  std::unique_ptr<KeyRing> keyring;
+  std::unique_ptr<RotatingKeyRing> rotating_secrets;
 
  public:
   explicit MonClient(CephContext *cct_);
@@ -331,8 +331,7 @@ public:
   void reopen_session(Context *cb=NULL) {
     Mutex::Locker l(monc_lock);
     if (cb) {
-      delete session_established_context;
-      session_established_context = cb;
+      session_established_context.reset(cb);
     }
     _reopen_session();
   }

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 8ba5c5c778deae944631a7608d124bfc98058bd2..3291d627b57748b4707d6777e9af937cdcf93d4a 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -6175,8 +6175,10 @@ bool OSD::ms_verify_authorizer(Connection *con, int peer_type,
   uint64_t global_id;
   uint64_t auid = CEPH_AUTH_UID_DEFAULT;
 
-  isvalid = authorize_handler->verify_authorizer(cct, monc->rotating_secrets,
-                                                authorizer_data, authorizer_reply, name, global_id, caps_info, session_key, &auid);
+  isvalid = authorize_handler->verify_authorizer(
+    cct, monc->rotating_secrets.get(),
+    authorizer_data, authorizer_reply, name, global_id, caps_info, session_key,
+    &auid);
 
   if (isvalid) {
     Session *s = static_cast<Session *>(con->get_priv());