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>

diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc
index 4df773f696afb6d9f256b86c33a9ed8577d291d4..7383ed7fd663f9f69e2e372eda1bf489f24fbf97 100644 (file)
--- a/src/common/ceph_context.cc
+++ b/src/common/ceph_context.cc
@@ -463,7 +463,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 3b639d9c987a4b820653f3fe63f81f472ecab91a..037f2d813a2e887043653c6d64cd7c3a1994f96c 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;
@@ -60,10 +62,6 @@ private:
   ~CephContext();
   atomic_t nref;
 public:
-  class AssociatedSingletonObject {
-   public:
-    virtual ~AssociatedSingletonObject() {}
-  };
   CephContext *get() {
     nref.inc();
     return this;
@@ -132,9 +130,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);
   }
@@ -149,6 +150,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);
 
@@ -182,7 +198,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
   CryptoHandler *_crypto_none;

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