From a81bcf723c1099f2bea5daf8b01b7d9853de323a Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 15 Oct 2015 00:15:54 -0400 Subject: [PATCH] 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 (cherry picked from commit fb62c78637d7092f48871d943282f45029bd6d29) --- src/common/ceph_context.cc | 2 +- src/common/ceph_context.h | 30 +++++++++++++++++++++++------- src/msg/async/AsyncMessenger.h | 2 +- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc index 50346edf2ace..dd1f48818128 100644 --- a/src/common/ceph_context.cc +++ b/src/common/ceph_context.cc @@ -456,7 +456,7 @@ CephContext::~CephContext() { join_service_thread(); - for (map::iterator it = _associated_objs.begin(); + for (map::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 a9ffde04eb35..1fc26687c3c8 100644 --- a/src/common/ceph_context.h +++ b/src/common/ceph_context.h @@ -20,10 +20,12 @@ #include #include +#include "include/assert.h" #include "include/buffer.h" #include "include/atomic.h" #include "common/cmdparse.h" #include "include/Spinlock.h" +#include 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(p); + _associated_objs[name] = new TypedSingletonWrapper(p); } else { - p = reinterpret_cast(_associated_objs[name]); + TypedSingletonWrapper *wrapper = + dynamic_cast *>(_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 + 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 _associated_objs; + std::map _associated_objs; // crypto CryptoNone *_crypto_none; diff --git a/src/msg/async/AsyncMessenger.h b/src/msg/async/AsyncMessenger.h index 685799f64bd4..fdd5844cc4e8 100644 --- 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; -- 2.47.3