Change Ref() methods to return a RefCountedPtr<>.

3 files changed, 83 insertions, 16 deletions

diff --git a/src/core/lib/gprpp/orphanable.h b/src/core/lib/gprpp/orphanable.h
index 50199730c9..6e127c2861 100644
--- a/src/core/lib/gprpp/orphanable.h
+++ b/src/core/lib/gprpp/orphanable.h
@@ -28,6 +28,7 @@
 #include "src/core/lib/gprpp/abstract.h"
 #include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/memory.h"
+#include "src/core/lib/gprpp/ref_counted_ptr.h"
 
 namespace grpc_core {
 
@@ -69,6 +70,7 @@ inline OrphanablePtr<T> MakeOrphanable(Args&&... args) {
 }
 
 // A type of Orphanable with internal ref-counting.
+template <typename Child>
 class InternallyRefCounted : public Orphanable {
  public:
   // Not copyable nor movable.
@@ -78,10 +80,20 @@ class InternallyRefCounted : public Orphanable {
   GRPC_ABSTRACT_BASE_CLASS
 
  protected:
+  // Allow Delete() to access destructor.
+  template <typename T>
+  friend void Delete(T*);
+
+  // Allow RefCountedPtr<> to access Unref() and IncrementRefCount().
+  friend class RefCountedPtr<Child>;
+
   InternallyRefCounted() { gpr_ref_init(&refs_, 1); }
   virtual ~InternallyRefCounted() {}
 
-  void Ref() { gpr_ref(&refs_); }
+  RefCountedPtr<Child> Ref() GRPC_MUST_USE_RESULT {
+    IncrementRefCount();
+    return RefCountedPtr<Child>(reinterpret_cast<Child*>(this));
+  }
 
   void Unref() {
     if (gpr_unref(&refs_)) {
@@ -89,11 +101,9 @@ class InternallyRefCounted : public Orphanable {
     }
   }
 
-  // Allow Delete() to access destructor.
-  template <typename T>
-  friend void Delete(T*);
-
  private:
+  void IncrementRefCount() { gpr_ref(&refs_); }
+
   gpr_refcount refs_;
 };
 
@@ -103,6 +113,7 @@ class InternallyRefCounted : public Orphanable {
 // pointers and legacy code that is manually calling Ref() and Unref().
 // Once all of our code is converted to idiomatic C++, we may be able to
 // eliminate this class.
+template <typename Child>
 class InternallyRefCountedWithTracing : public Orphanable {
  public:
   // Not copyable nor movable.
@@ -118,6 +129,9 @@ class InternallyRefCountedWithTracing : public Orphanable {
   template <typename T>
   friend void Delete(T*);
 
+  // Allow RefCountedPtr<> to access Unref() and IncrementRefCount().
+  friend class RefCountedPtr<Child>;
+
   InternallyRefCountedWithTracing()
       : InternallyRefCountedWithTracing(static_cast<TraceFlag*>(nullptr)) {}
 
@@ -133,18 +147,27 @@ class InternallyRefCountedWithTracing : public Orphanable {
 
   virtual ~InternallyRefCountedWithTracing() {}
 
-  void Ref() { gpr_ref(&refs_); }
+  RefCountedPtr<Child> Ref() GRPC_MUST_USE_RESULT {
+    IncrementRefCount();
+    return RefCountedPtr<Child>(reinterpret_cast<Child*>(this));
+  }
 
-  void Ref(const DebugLocation& location, const char* reason) {
+  RefCountedPtr<Child> Ref(const DebugLocation& location,
+                           const char* reason) GRPC_MUST_USE_RESULT {
     if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) {
       gpr_atm old_refs = gpr_atm_no_barrier_load(&refs_.count);
       gpr_log(GPR_DEBUG, "%s:%p %s:%d ref %" PRIdPTR " -> %" PRIdPTR " %s",
               trace_flag_->name(), this, location.file(), location.line(),
               old_refs, old_refs + 1, reason);
     }
-    Ref();
+    return Ref();
   }
 
+  // TODO(roth): Once all of our code is converted to C++ and can use
+  // RefCountedPtr<> instead of manual ref-counting, make the Unref() methods
+  // private, since they will only be used by RefCountedPtr<>, which is a
+  // friend of this class.
+
   void Unref() {
     if (gpr_unref(&refs_)) {
       Delete(this);
@@ -162,6 +185,8 @@ class InternallyRefCountedWithTracing : public Orphanable {
   }
 
  private:
+  void IncrementRefCount() { gpr_ref(&refs_); }
+
   TraceFlag* trace_flag_ = nullptr;
   gpr_refcount refs_;
 };
diff --git a/src/core/lib/gprpp/ref_counted.h b/src/core/lib/gprpp/ref_counted.h
index c68118a71a..16c7912fc6 100644
--- a/src/core/lib/gprpp/ref_counted.h
+++ b/src/core/lib/gprpp/ref_counted.h
@@ -26,16 +26,28 @@
 #include "src/core/lib/gprpp/abstract.h"
 #include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/memory.h"
+#include "src/core/lib/gprpp/ref_counted_ptr.h"
 
 namespace grpc_core {
 
 // A base class for reference-counted objects.
 // New objects should be created via New() and start with a refcount of 1.
 // When the refcount reaches 0, the object will be deleted via Delete().
+//
+// This will commonly be used by CRTP (curiously-recurring template pattern)
+// e.g., class MyClass : public RefCounted<MyClass>
+template <typename Child>
 class RefCounted {
  public:
-  void Ref() { gpr_ref(&refs_); }
+  RefCountedPtr<Child> Ref() GRPC_MUST_USE_RESULT {
+    IncrementRefCount();
+    return RefCountedPtr<Child>(reinterpret_cast<Child*>(this));
+  }
 
+  // TODO(roth): Once all of our code is converted to C++ and can use
+  // RefCountedPtr<> instead of manual ref-counting, make this method
+  // private, since it will only be used by RefCountedPtr<>, which is a
+  // friend of this class.
   void Unref() {
     if (gpr_unref(&refs_)) {
       Delete(this);
@@ -58,6 +70,11 @@ class RefCounted {
   virtual ~RefCounted() {}
 
  private:
+  // Allow RefCountedPtr<> to access IncrementRefCount().
+  friend class RefCountedPtr<Child>;
+
+  void IncrementRefCount() { gpr_ref(&refs_); }
+
   gpr_refcount refs_;
 };
 
@@ -67,20 +84,30 @@ class RefCounted {
 // pointers and legacy code that is manually calling Ref() and Unref().
 // Once all of our code is converted to idiomatic C++, we may be able to
 // eliminate this class.
+template <typename Child>
 class RefCountedWithTracing {
  public:
-  void Ref() { gpr_ref(&refs_); }
+  RefCountedPtr<Child> Ref() GRPC_MUST_USE_RESULT {
+    IncrementRefCount();
+    return RefCountedPtr<Child>(reinterpret_cast<Child*>(this));
+  }
 
-  void Ref(const DebugLocation& location, const char* reason) {
+  RefCountedPtr<Child> Ref(const DebugLocation& location,
+                           const char* reason) GRPC_MUST_USE_RESULT {
     if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) {
       gpr_atm old_refs = gpr_atm_no_barrier_load(&refs_.count);
       gpr_log(GPR_DEBUG, "%s:%p %s:%d ref %" PRIdPTR " -> %" PRIdPTR " %s",
               trace_flag_->name(), this, location.file(), location.line(),
               old_refs, old_refs + 1, reason);
     }
-    Ref();
+    return Ref();
   }
 
+  // TODO(roth): Once all of our code is converted to C++ and can use
+  // RefCountedPtr<> instead of manual ref-counting, make the Unref() methods
+  // private, since they will only be used by RefCountedPtr<>, which is a
+  // friend of this class.
+
   void Unref() {
     if (gpr_unref(&refs_)) {
       Delete(this);
@@ -124,6 +151,11 @@ class RefCountedWithTracing {
   virtual ~RefCountedWithTracing() {}
 
  private:
+  // Allow RefCountedPtr<> to access IncrementRefCount().
+  friend class RefCountedPtr<Child>;
+
+  void IncrementRefCount() { gpr_ref(&refs_); }
+
   TraceFlag* trace_flag_ = nullptr;
   gpr_refcount refs_;
 };
diff --git a/src/core/lib/gprpp/ref_counted_ptr.h b/src/core/lib/gprpp/ref_counted_ptr.h
index dda0f00d79..f82ba50da3 100644
--- a/src/core/lib/gprpp/ref_counted_ptr.h
+++ b/src/core/lib/gprpp/ref_counted_ptr.h
@@ -25,8 +25,8 @@
 
 namespace grpc_core {
 
-// A smart pointer class for objects that provide Ref() and Unref() methods,
-// such as those provided by the RefCounted base class.
+// A smart pointer class for objects that provide IncrementRefCount() and
+// Unref() methods, such as those provided by the RefCounted base class.
 template <typename T>
 class RefCountedPtr {
  public:
@@ -49,13 +49,13 @@ class RefCountedPtr {
 
   // Copy support.
   RefCountedPtr(const RefCountedPtr& other) {
-    if (other.value_ != nullptr) other.value_->Ref();
+    if (other.value_ != nullptr) other.value_->IncrementRefCount();
     value_ = other.value_;
   }
   RefCountedPtr& operator=(const RefCountedPtr& other) {
     // Note: Order of reffing and unreffing is important here in case value_
     // and other.value_ are the same object.
-    if (other.value_ != nullptr) other.value_->Ref();
+    if (other.value_ != nullptr) other.value_->IncrementRefCount();
     if (value_ != nullptr) value_->Unref();
     value_ = other.value_;
     return *this;
@@ -71,6 +71,16 @@ class RefCountedPtr {
     value_ = value;
   }
 
+  // TODO(roth): This method exists solely as a transition mechanism to allow
+  // us to pass a ref to idiomatic C code that does not use RefCountedPtr<>.
+  // Once all of our code has been converted to idiomatic C++, this
+  // method should go away.
+  T* release() {
+    T* value = value_;
+    value_ = nullptr;
+    return value;
+  }
+
   T* get() const { return value_; }
 
   T& operator*() const { return *value_; }