From: Sage Weil <sage@inktank.com>
Date: Thu, 30 May 2013 18:07:06 +0000 (-0700)
Subject: mon: fix leak of health_monitor and config_key_service
X-Git-Tag: v0.64~33^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=c888d1d3f1b77e62d1a8796992e918d12a009b9d;p=ceph.git

mon: fix leak of health_monitor and config_key_service

Switch to using regular pointers here.  The lifecycle of these services is
very simple such that refcounting is overkill.

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

diff --git a/src/mon/ConfigKeyService.cc b/src/mon/ConfigKeyService.cc
index 3319d9e80a83b..10c16834c757b 100644
--- a/src/mon/ConfigKeyService.cc
+++ b/src/mon/ConfigKeyService.cc
@@ -16,9 +16,6 @@
 #include <stdlib.h>
 #include <limits.h>
 
-#include <boost/intrusive_ptr.hpp>
-#include "include/assert.h"
-
 #include "mon/Monitor.h"
 #include "mon/QuorumService.h"
 #include "mon/ConfigKeyService.h"
diff --git a/src/mon/ConfigKeyService.h b/src/mon/ConfigKeyService.h
index 49e15acdfc652..e379af610b9cb 100644
--- a/src/mon/ConfigKeyService.h
+++ b/src/mon/ConfigKeyService.h
@@ -14,9 +14,6 @@
 #ifndef CEPH_MON_CONFIG_KEY_SERVICE_H
 #define CEPH_MON_CONFIG_KEY_SERVICE_H
 
-#include <boost/intrusive_ptr.hpp>
-#include "include/assert.h"
-
 #include "mon/Monitor.h"
 #include "mon/QuorumService.h"
 
@@ -48,9 +45,6 @@ public:
     paxos(p)
   { }
   virtual ~ConfigKeyService() { }
-  ConfigKeyService *get() {
-    return static_cast<ConfigKeyService *>(RefCountedObject::get());
-  }
 
 
   /**
@@ -82,6 +76,5 @@ public:
    * @} // ConfigKeyService_Inherited_h
    */
 };
-typedef boost::intrusive_ptr<ConfigKeyService> ConfigKeyServiceRef;
 
 #endif // CEPH_MON_CONFIG_KEY_SERVICE_H
diff --git a/src/mon/DataHealthService.h b/src/mon/DataHealthService.h
index c94327f312942..a17171509c173 100644
--- a/src/mon/DataHealthService.h
+++ b/src/mon/DataHealthService.h
@@ -14,9 +14,6 @@
 #ifndef CEPH_MON_DATA_HEALTH_SERVICE_H
 #define CEPH_MON_DATA_HEALTH_SERVICE_H
 
-#include <boost/intrusive_ptr.hpp>
-// Because intusive_ptr clobbers our assert...
-#include "include/assert.h"
 #include <errno.h>
 
 #include "include/types.h"
@@ -66,9 +63,6 @@ public:
     set_update_period(g_conf->mon_health_data_update_interval);
   }
   virtual ~DataHealthService() { }
-  DataHealthService *get() {
-    return static_cast<DataHealthService *>(RefCountedObject::get());
-  }
 
   virtual void init() {
     generic_dout(20) << "data_health " << __func__ << dendl;
@@ -86,6 +80,5 @@ public:
     return "data_health";
   }
 };
-typedef boost::intrusive_ptr<DataHealthService> DataHealthServiceRef;
 
 #endif /* CEPH_MON_DATA_HEALTH_SERVICE_H */
diff --git a/src/mon/HealthMonitor.cc b/src/mon/HealthMonitor.cc
index 6239d32a87305..c6ab6f489180b 100644
--- a/src/mon/HealthMonitor.cc
+++ b/src/mon/HealthMonitor.cc
@@ -44,11 +44,11 @@ void HealthMonitor::init()
 {
   dout(10) << __func__ << dendl;
   assert(services.empty());
-  services[HealthService::SERVICE_HEALTH_DATA] =
-    HealthServiceRef(new DataHealthService(mon));
+  services[HealthService::SERVICE_HEALTH_DATA] = new DataHealthService(mon);
 
-  for (map<int,HealthServiceRef>::iterator it = services.begin();
-       it != services.end(); ++it) {
+  for (map<int,HealthService*>::iterator it = services.begin();
+       it != services.end();
+       ++it) {
     it->second->init();
   }
 }
@@ -71,9 +71,11 @@ void HealthMonitor::service_shutdown()
 {
   dout(0) << "HealthMonitor::service_shutdown "
           << services.size() << " services" << dendl;
-  for (map<int,HealthServiceRef>::iterator it = services.begin();
-      it != services.end(); ++it) {
+  for (map<int,HealthService*>::iterator it = services.begin();
+      it != services.end();
+       ++it) {
     it->second->shutdown();
+    delete it->second;
   }
   services.clear();
 }
@@ -87,8 +89,9 @@ health_status_t HealthMonitor::get_health(Formatter *f,
     f->open_array_section("health_services");
   }
 
-  for (map<int,HealthServiceRef>::iterator it = services.begin();
-       it != services.end(); ++it) {
+  for (map<int,HealthService*>::iterator it = services.begin();
+       it != services.end();
+       ++it) {
     health_status_t h = it->second->get_health(f, detail);
     if (overall > h)
       overall = h;
diff --git a/src/mon/HealthMonitor.h b/src/mon/HealthMonitor.h
index e0255d20081aa..a1c98a9bb7756 100644
--- a/src/mon/HealthMonitor.h
+++ b/src/mon/HealthMonitor.h
@@ -14,10 +14,6 @@
 #ifndef CEPH_HEALTH_MONITOR_H
 #define CEPH_HEALTH_MONITOR_H
 
-#include <boost/intrusive_ptr.hpp>
-// Because intusive_ptr clobbers our assert...
-#include "include/assert.h"
-
 #include "mon/Monitor.h"
 #include "mon/QuorumService.h"
 #include "mon/HealthService.h"
@@ -29,16 +25,15 @@
 
 class HealthMonitor : public QuorumService
 {
-  map<int,HealthServiceRef> services;
+  map<int,HealthService*> services;
 
 protected:
   virtual void service_shutdown();
 
 public:
   HealthMonitor(Monitor *m) : QuorumService(m) { }
-  virtual ~HealthMonitor() { }
-  HealthMonitor *get() {
-    return static_cast<HealthMonitor *>(RefCountedObject::get());
+  virtual ~HealthMonitor() {
+    assert(services.empty());
   }
 
 
@@ -52,7 +47,7 @@ public:
   virtual bool service_dispatch(Message *m);
 
   virtual void start_epoch() {
-    for (map<int,HealthServiceRef>::iterator it = services.begin();
+    for (map<int,HealthService*>::iterator it = services.begin();
          it != services.end(); ++it) {
       it->second->start(get_epoch());
     }
@@ -60,9 +55,9 @@ public:
 
   virtual void finish_epoch() {
     generic_dout(20) << "HealthMonitor::finish_epoch()" << dendl;
-    for (map<int,HealthServiceRef>::iterator it = services.begin();
+    for (map<int,HealthService*>::iterator it = services.begin();
          it != services.end(); ++it) {
-      assert(it->second.get() != NULL);
+      assert(it->second != NULL);
       it->second->finish();
     }
   }
@@ -82,6 +77,5 @@ public:
    * @} // HealthMonitor_Inherited_h
    */
 };
-typedef boost::intrusive_ptr<HealthMonitor> HealthMonitorRef;
 
 #endif // CEPH_HEALTH_MONITOR_H
diff --git a/src/mon/HealthService.h b/src/mon/HealthService.h
index 1e041f5739ebd..11d6e48575874 100644
--- a/src/mon/HealthService.h
+++ b/src/mon/HealthService.h
@@ -14,10 +14,6 @@
 #ifndef CEPH_MON_HEALTH_SERVICE_H
 #define CEPH_MON_HEALTH_SERVICE_H
 
-#include <boost/intrusive_ptr.hpp>
-// Because intusive_ptr clobbers our assert...
-#include "include/assert.h"
-
 #include "mon/Monitor.h"
 #include "mon/QuorumService.h"
 
@@ -41,14 +37,10 @@ struct HealthService : public QuorumService
   virtual bool service_dispatch(MMonHealth *m) = 0;
 
 public:
-  HealthService *get() {
-    return static_cast<HealthService *>(RefCountedObject::get());
-  }
   virtual health_status_t get_health(Formatter *f,
                           list<pair<health_status_t,string> > *detail) = 0;
   virtual int get_type() = 0;
   virtual string get_name() const = 0;
 };
-typedef boost::intrusive_ptr<HealthService> HealthServiceRef;
 
 #endif // CEPH_MON_HEALTH_SERVICE_H
diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index acfeb65da6726..4196a18a9a54c 100644
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -170,8 +170,8 @@ Monitor::Monitor(CephContext* cct_, string nm, MonitorDBStore *s,
   paxos_service[PAXOS_LOG] = new LogMonitor(this, paxos, "logm");
   paxos_service[PAXOS_AUTH] = new AuthMonitor(this, paxos, "auth");
 
-  health_monitor = QuorumServiceRef(new HealthMonitor(this));
-  config_key_service = ConfigKeyServiceRef(new ConfigKeyService(this, paxos));
+  health_monitor = new HealthMonitor(this);
+  config_key_service = new ConfigKeyService(this, paxos);
 
   mon_caps = new MonCaps();
   mon_caps->set_allow_all(true);
@@ -203,6 +203,8 @@ Monitor::~Monitor()
 {
   for (vector<PaxosService*>::iterator p = paxos_service.begin(); p != paxos_service.end(); ++p)
     delete *p;
+  delete health_monitor;
+  delete config_key_service;
   delete paxos;
   assert(session_map.sessions.empty());
   delete mon_caps;
diff --git a/src/mon/Monitor.h b/src/mon/Monitor.h
index 1443525e9e68c..b9203ae1e6150 100644
--- a/src/mon/Monitor.h
+++ b/src/mon/Monitor.h
@@ -51,9 +51,6 @@
 #include <memory>
 #include <tr1/memory>
 #include <errno.h>
-#include <boost/intrusive_ptr.hpp>
-// Because intusive_ptr clobbers our assert...
-#include "include/assert.h"
 
 
 #define CEPH_MON_PROTOCOL     10 /* cluster internal */
@@ -1249,8 +1246,8 @@ public:
   friend class PGMonitor;
   friend class LogMonitor;
 
-  boost::intrusive_ptr<QuorumService> health_monitor;
-  boost::intrusive_ptr<QuorumService> config_key_service;
+  QuorumService *health_monitor;
+  QuorumService *config_key_service;
 
   // -- sessions --
   MonSessionMap session_map;
diff --git a/src/mon/QuorumService.h b/src/mon/QuorumService.h
index 16eb8b0542af8..27971b7d599bb 100644
--- a/src/mon/QuorumService.h
+++ b/src/mon/QuorumService.h
@@ -14,10 +14,6 @@
 #ifndef CEPH_MON_QUORUM_SERVICE_H
 #define CEPH_MON_QUORUM_SERVICE_H
 
-#include <boost/intrusive_ptr.hpp>
-// Because intusive_ptr clobbers our assert...
-#include "include/assert.h"
-
 #include <errno.h>
 
 #include "include/types.h"
@@ -27,14 +23,14 @@
 
 #include "mon/Monitor.h"
 
-class QuorumService : public RefCountedObject
+class QuorumService
 {
   Context *tick_event;
   double tick_period;
 
   struct C_Tick : public Context {
-    boost::intrusive_ptr<QuorumService> s;
-    C_Tick(boost::intrusive_ptr<QuorumService> qs) : s(qs) { }
+    QuorumService *s;
+    C_Tick(QuorumService *qs) : s(qs) { }
     void finish(int r) {
       if (r < 0)
         return;
@@ -74,8 +70,7 @@ protected:
     if (tick_period <= 0)
       return;
 
-    tick_event = new C_Tick(
-        boost::intrusive_ptr<QuorumService>(this));
+    tick_event = new C_Tick(this);
     mon->timer.add_event_after(tick_period, tick_event);
   }
 
@@ -97,9 +92,6 @@ protected:
 
 public:
   virtual ~QuorumService() { }
-  QuorumService *get() {
-    return static_cast<QuorumService *>(RefCountedObject::get());
-  }
 
   void start(epoch_t new_epoch) {
     epoch = new_epoch;
@@ -138,6 +130,5 @@ public:
   virtual string get_name() const = 0;
 
 };
-typedef boost::intrusive_ptr<QuorumService> QuorumServiceRef;
 
 #endif /* CEPH_MON_QUORUM_SERVICE_H */
diff --git a/src/vstart.sh b/src/vstart.sh
index 58411efa4e69c..f3717d6535c6e 100755
--- a/src/vstart.sh
+++ b/src/vstart.sh
@@ -182,13 +182,11 @@ if [ "$debug" -eq 0 ]; then
 else
     echo "** going verbose **"
     CMONDEBUG='
-        lockdep = 1
 	debug mon = 20
         debug paxos = 20
         debug auth = 20
         debug ms = 1'
     COSDDEBUG='
-        lockdep = 1
         debug ms = 1
         debug osd = 25
         debug monc = 20
@@ -196,7 +194,6 @@ else
         debug filestore = 20
         debug objclass = 20'
     CMDSDEBUG='
-        lockdep = 1
         debug ms = 1
         debug mds = 20
         debug auth = 20