common: refactor RefCountedObj and reduce header dependencies

- Protect constructors/destructors; RefCountedObj is an abstract class.

- Move some helpers to separate unit to avoid circular dependency with
  CephContext which could otherwise use RefCountedObj.

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt
index eadaf285d4885908ff15f960591b053033f0901a..1ecaf936754f912a21f065df9e317c1dc70b08e2 100644 (file)
--- a/src/common/CMakeLists.txt
+++ b/src/common/CMakeLists.txt
@@ -24,6 +24,7 @@ set(common_srcs
   OutputDataSocket.cc
   PluginRegistry.cc
   Readahead.cc
+  RefCountedObj.cc
   SloppyCRCMap.cc
   SubProcess.cc
   Thread.cc

diff --git a/src/common/RefCountedObj.cc b/src/common/RefCountedObj.cc
new file mode 100644 (file)
index 0000000..e77b631
--- /dev/null
+++ b/src/common/RefCountedObj.cc
@@ -0,0 +1,40 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+//
+#include "include/ceph_assert.h"
+
+#include "common/RefCountedObj.h"
+#include "common/ceph_context.h"
+#include "common/dout.h"
+#include "common/valgrind.h"
+
+RefCountedObject::~RefCountedObject()
+{
+  ceph_assert(nref == 0);
+}
+
+void RefCountedObject::put() const {
+  CephContext *local_cct = cct;
+  auto v = --nref;
+  if (local_cct) {
+    lsubdout(local_cct, refs, 1) << "RefCountedObject::put " << this << " "
+                  << (v + 1) << " -> " << v
+                  << dendl;
+  }
+  if (v == 0) {
+    ANNOTATE_HAPPENS_AFTER(&nref);
+    ANNOTATE_HAPPENS_BEFORE_FORGET_ALL(&nref);
+    delete this;
+  } else {
+    ANNOTATE_HAPPENS_BEFORE(&nref);
+  }
+}
+
+void RefCountedObject::_get() const {
+  auto v = ++nref;
+  ceph_assert(v > 1); /* it should never happen that _get() sees nref == 0 */
+  if (cct) {
+    lsubdout(cct, refs, 1) << "RefCountedObject::get " << this << " "
+            << (v - 1) << " -> " << v << dendl;
+  }
+}

diff --git a/src/common/RefCountedObj.h b/src/common/RefCountedObj.h
index 3ebbe78fd2ba4dae11b16074d259d12426ca8059..024d2f4d2c9126ec553b746b0dfe3d9a3891ad11 100644 (file)
--- a/src/common/RefCountedObj.h
+++ b/src/common/RefCountedObj.h
@@ -16,69 +16,50 @@
 #define CEPH_REFCOUNTEDOBJ_H
  
 #include "common/ceph_mutex.h"
-#include "common/ceph_context.h"
-#include "common/valgrind.h"
-#include "common/debug.h"
+#include "common/ref.h"
 
-#include <boost/smart_ptr/intrusive_ptr.hpp>
-
-// re-include our assert to clobber the system one; fix dout:
-#include "include/ceph_assert.h"
+#include <atomic>
 
 struct RefCountedObject {
 public:
-  RefCountedObject(CephContext *c = NULL, int n=1) : nref(n), cct(c) {}
-  virtual ~RefCountedObject() {
-    ceph_assert(nref == 0);
+  void set_cct(class CephContext *c) {
+    cct = c;
   }
-  
+
+  uint64_t get_nref() const {
+    return nref;
+  }
+
   const RefCountedObject *get() const {
-    int v = ++nref;
-    if (cct)
-      lsubdout(cct, refs, 1) << "RefCountedObject::get " << this << " "
-                            << (v - 1) << " -> " << v
-                            << dendl;
+    _get();
     return this;
   }
   RefCountedObject *get() {
-    int v = ++nref;
-    if (cct)
-      lsubdout(cct, refs, 1) << "RefCountedObject::get " << this << " "
-                            << (v - 1) << " -> " << v
-                            << dendl;
+    _get();
     return this;
   }
-  void put() const {
-    CephContext *local_cct = cct;
-    auto v = --nref;
-    if (local_cct)
-      lsubdout(local_cct, refs, 1) << "RefCountedObject::put " << this << " "
-                                  << (v + 1) << " -> " << v
-                                  << dendl;
-    if (v == 0) {
-      ANNOTATE_HAPPENS_AFTER(&nref);
-      ANNOTATE_HAPPENS_BEFORE_FORGET_ALL(&nref);
-      delete this;
-    } else {
-      ANNOTATE_HAPPENS_BEFORE(&nref);
-    }
-  }
-  void set_cct(CephContext *c) {
-    cct = c;
-  }
+  void put() const;
 
-  uint64_t get_nref() const {
-    return nref;
-  }
+protected:
+  RefCountedObject() = default;
+  RefCountedObject(const RefCountedObject& o) : cct(o.cct) {}
+  RefCountedObject& operator=(const RefCountedObject& o) = delete;
+  RefCountedObject(RefCountedObject&&) = delete;
+  RefCountedObject& operator=(RefCountedObject&&) = delete;
+  RefCountedObject(class CephContext* c = nullptr, int n = 1) : cct(c), nref(n) {}
+
+  virtual ~RefCountedObject();
 
 private:
+  void _get() const;
+
 #ifndef WITH_SEASTAR
-  mutable std::atomic<uint64_t> nref;
+  mutable std::atomic<uint64_t> nref{1};
 #else
   // crimson is single threaded at the moment
-  mutable uint64_t nref;
+  mutable uint64_t nref{1};
 #endif
-  CephContext *cct;
+  class CephContext *cct{nullptr};
 };
 
 #ifndef WITH_SEASTAR
@@ -88,14 +69,9 @@ private:
  *
  *  a refcounted condition, will be removed when all references are dropped
  */
-
 struct RefCountedCond : public RefCountedObject {
-  bool complete;
-  ceph::mutex lock = ceph::make_mutex("RefCountedCond::lock");
-  ceph::condition_variable cond;
-  int rval;
-
-  RefCountedCond() : complete(false), rval(0) {}
+  RefCountedCond() = default;
+  ~RefCountedCond() = default;
 
   int wait() {
     std::unique_lock l(lock);
@@ -115,6 +91,12 @@ struct RefCountedCond : public RefCountedObject {
   void done() {
     done(0);
   }
+
+private:
+  bool complete = false;
+  ceph::mutex lock = ceph::make_mutex("RefCountedCond::lock");
+  ceph::condition_variable cond;
+  int rval = 0;
 };
 
 /**
@@ -180,6 +162,6 @@ static inline void intrusive_ptr_release(const RefCountedObject *p) {
   p->put();
 }
 
-using RefCountedPtr = boost::intrusive_ptr<RefCountedObject>;
+using RefCountedPtr = ceph::ref_t<RefCountedObject>;
 
 #endif

diff --git a/src/crimson/CMakeLists.txt b/src/crimson/CMakeLists.txt
index 72c657616767a985badc81b14e0e731f43aebec3..a373cb3ba516e71be62975e6cf1d8d4709b13fb0 100644 (file)
--- a/src/crimson/CMakeLists.txt
+++ b/src/crimson/CMakeLists.txt
@@ -72,6 +72,7 @@ add_library(crimson-common STATIC
   ${PROJECT_SOURCE_DIR}/src/common/Thread.cc
   ${PROJECT_SOURCE_DIR}/src/common/HeartbeatMap.cc
   ${PROJECT_SOURCE_DIR}/src/common/PluginRegistry.cc
+  ${PROJECT_SOURCE_DIR}/src/common/RefCountedObj.cc
   ${PROJECT_SOURCE_DIR}/src/global/pidfile.cc
   ${PROJECT_SOURCE_DIR}/src/librbd/Features.cc
   ${PROJECT_SOURCE_DIR}/src/log/Log.cc