ceph_context: remove unsafe cast for singletons

It was previously assumed that a CephContext singleton would
inherit from CephContext::AssociatedSingletonObject, but it was
not enforced.  This could result in unknown behavior when the
singleton is destroyed due to the implied virtual destructor.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit fb62c78637d7092f48871d943282f45029bd6d29)

diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc
index 50346edf2ace6a227b6f32fbf241a517db4d814c..dd1f488181285c814e37a03d2245fcb31e715472 100644 (file)
--- a/src/common/ceph_context.cc
+++ b/src/common/ceph_context.cc
@@ -456,7 +456,7 @@ CephContext::~CephContext()
 {
   join_service_thread();
 
-  for (map<string, AssociatedSingletonObject*>::iterator it = _associated_objs.begin();
+  for (map<string, SingletonWrapper*>::iterator it = _associated_objs.begin();
        it != _associated_objs.end(); ++it)
     delete it->second;
 

diff --git a/src/common/ceph_context.h b/src/common/ceph_context.h
index a9ffde04eb35b6eaf3481dc5e4624cbb37a0432d..1fc26687c3c857f1361f78cb5de044b8992bc9a6 100644 (file)
--- a/src/common/ceph_context.h
+++ b/src/common/ceph_context.h
@@ -20,10 +20,12 @@
 #include <string>
 #include <set>
 
+#include "include/assert.h"
 #include "include/buffer.h"
 #include "include/atomic.h"
 #include "common/cmdparse.h"
 #include "include/Spinlock.h"
+#include <boost/noncopyable.hpp>
 
 class AdminSocket;
 class CephContextServiceThread;
@@ -61,10 +63,6 @@ private:
   ~CephContext();
   atomic_t nref;
 public:
-  class AssociatedSingletonObject {
-   public:
-    virtual ~AssociatedSingletonObject() {}
-  };
   CephContext *get() {
     nref.inc();
     return this;
@@ -117,9 +115,12 @@ public:
     ceph_spin_lock(&_associated_objs_lock);
     if (!_associated_objs.count(name)) {
       p = new T(this);
-      _associated_objs[name] = reinterpret_cast<AssociatedSingletonObject*>(p);
+      _associated_objs[name] = new TypedSingletonWrapper<T>(p);
     } else {
-      p = reinterpret_cast<T*>(_associated_objs[name]);
+      TypedSingletonWrapper<T> *wrapper =
+        dynamic_cast<TypedSingletonWrapper<T> *>(_associated_objs[name]);
+      assert(wrapper != NULL);
+      p = wrapper->singleton;
     }
     ceph_spin_unlock(&_associated_objs_lock);
   }
@@ -134,6 +135,21 @@ public:
                                          std::ostream *message);
 
 private:
+  struct SingletonWrapper : boost::noncopyable {
+    virtual ~SingletonWrapper() {}
+  };
+
+  template <typename T>
+  struct TypedSingletonWrapper : public SingletonWrapper {
+    TypedSingletonWrapper(T *p) : singleton(p) {
+    }
+    virtual ~TypedSingletonWrapper() {
+      delete singleton;
+    }
+
+    T *singleton;
+  };
+
   CephContext(const CephContext &rhs);
   CephContext &operator=(const CephContext &rhs);
 
@@ -167,7 +183,7 @@ private:
   ceph::HeartbeatMap *_heartbeat_map;
 
   ceph_spinlock_t _associated_objs_lock;
-  std::map<std::string, AssociatedSingletonObject*> _associated_objs;
+  std::map<std::string, SingletonWrapper*> _associated_objs;
 
   // crypto
   CryptoNone *_crypto_none;

diff --git a/src/msg/async/AsyncMessenger.h b/src/msg/async/AsyncMessenger.h
index 685799f64bd4b1331fbecb0f53afaf941c175907..fdd5844cc4e8580fc8f195bfb3632289e765e99e 100644 (file)
--- a/src/msg/async/AsyncMessenger.h
+++ b/src/msg/async/AsyncMessenger.h
@@ -80,7 +80,7 @@ class Processor {
   void accept();
 };
 
-class WorkerPool: CephContext::AssociatedSingletonObject {
+class WorkerPool {
   WorkerPool(const WorkerPool &);
   WorkerPool& operator=(const WorkerPool &);
   CephContext *cct;