Skip to content

Commit

Permalink
ceph_context: remove unsafe cast for singletons
Browse files Browse the repository at this point in the history
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 fb62c78)
  • Loading branch information
Jason Dillaman committed Nov 4, 2015
1 parent 5c34819 commit da0416a
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/common/ceph_context.cc
Expand Up @@ -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;

Expand Down
30 changes: 23 additions & 7 deletions src/common/ceph_context.h
Expand Up @@ -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;
Expand Down Expand Up @@ -61,10 +63,6 @@ class CephContext {
~CephContext();
atomic_t nref;
public:
class AssociatedSingletonObject {
public:
virtual ~AssociatedSingletonObject() {}
};
CephContext *get() {
nref.inc();
return this;
Expand Down Expand Up @@ -117,9 +115,12 @@ class CephContext {
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);
}
Expand All @@ -134,6 +135,21 @@ class CephContext {
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);

Expand Down Expand Up @@ -167,7 +183,7 @@ class CephContext {
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;
Expand Down
2 changes: 1 addition & 1 deletion src/msg/async/AsyncMessenger.h
Expand Up @@ -80,7 +80,7 @@ class Processor {
void accept();
};

class WorkerPool: CephContext::AssociatedSingletonObject {
class WorkerPool {
WorkerPool(const WorkerPool &);
WorkerPool& operator=(const WorkerPool &);
CephContext *cct;
Expand Down

0 comments on commit da0416a

Please sign in to comment.