Add a non-polymorphic variant to RefCounted.

Using RefCounted<Child, NonPolymorphic> users can now build smart,
ref-counted pointers without paying the costs of a vtable when it's
possible.

src/core/lib/gprpp/ref_counted.h

@@ -34,14 +34,58 @@
 
 namespace grpc_core {
 
+// PolymorphicRefCount enforces polymorphic destruction of RefCounted.
+class PolymorphicRefCount {
+ public:
+  GRPC_ABSTRACT_BASE_CLASS
+
+ protected:
+  GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE
+
+  virtual ~PolymorphicRefCount() {}
+};
+
+// NonPolymorphicRefCount does not enforce polymorphic destruction of
+// RefCounted. Please refer to grpc_core::RefCounted for more details, and
+// when in doubt use PolymorphicRefCount.
+class NonPolymorphicRefCount {
+ public:
+  GRPC_ABSTRACT_BASE_CLASS
+
+ protected:
+  GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE
+
+  ~NonPolymorphicRefCount() {}
+};
+
 // 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 {
+// Use PolymorphicRefCount and NonPolymorphicRefCount to select between
+// different implementations of RefCounted.
+//
+// Note that NonPolymorphicRefCount does not support polymorphic destruction.
+// So, use NonPolymorphicRefCount only when both of the following conditions
+// are guaranteed to hold:
+// (a) Child is a concrete leaf class in RefCounted<Child>, and
+// (b) you are gauranteed to call Unref only on concrete leaf classes and not
+//     their parents.
+//
+// The following example is illegal, because calling Unref() will not call
+// the dtor of Child.
+//
+//    class Parent : public RefCounted<Parent, NonPolymorphicRefCount> {}
+//    class Child : public Parent {}
+//
+//    Child* ch;
+//    ch->Unref();
+//
+template <typename Child, typename Impl = PolymorphicRefCount>
+class RefCounted : public Impl {
  public:
   RefCountedPtr<Child> Ref() GRPC_MUST_USE_RESULT {
     IncrementRefCount();
@@ -69,7 +113,8 @@ class RefCounted {
 
   RefCounted() { gpr_ref_init(&refs_, 1); }
 
-  virtual ~RefCounted() {}
+  // Note: Depending on the Impl used, this dtor can be implicitly virtual.
+  ~RefCounted() {}
 
  private:
   // Allow RefCountedPtr<> to access IncrementRefCount().
@@ -87,8 +132,8 @@ 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>
+template <typename Child, typename Impl = PolymorphicRefCount>
-class RefCountedWithTracing {
+class RefCountedWithTracing : public Impl {
  public:
   RefCountedPtr<Child> Ref() GRPC_MUST_USE_RESULT {
     IncrementRefCount();
@@ -149,7 +194,8 @@ class RefCountedWithTracing {
       : RefCountedWithTracing() {}
 #endif
 
-  virtual ~RefCountedWithTracing() {}
+  // Note: Depending on the Impl used, this dtor can be implicitly virtual.
+  ~RefCountedWithTracing() {}
 
  private:
   // Allow RefCountedPtr<> to access IncrementRefCount().

src/core/lib/gprpp/ref_counted_ptr.h

@@ -21,6 +21,7 @@
 
 #include <grpc/support/port_platform.h>
 
+#include <type_traits>
 #include <utility>
 
 #include "src/core/lib/gprpp/memory.h"
@@ -74,6 +75,8 @@ class RefCountedPtr {
   }
   template <typename Y>
   RefCountedPtr(const RefCountedPtr<Y>& other) {
+    static_assert(std::has_virtual_destructor<T>::value,
+                  "T does not have a virtual dtor");
     if (other.value_ != nullptr) other.value_->IncrementRefCount();
     value_ = other.value_;
   }
@@ -89,6 +92,8 @@ class RefCountedPtr {
   }
   template <typename Y>
   RefCountedPtr& operator=(const RefCountedPtr<Y>& other) {
+    static_assert(std::has_virtual_destructor<T>::value,
+                  "T does not have a virtual dtor");
     // 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_->IncrementRefCount();
@@ -102,8 +107,14 @@ class RefCountedPtr {
   }
 
   // If value is non-null, we take ownership of a ref to it.
+  void reset(T* value) {
+    if (value_ != nullptr) value_->Unref();
+    value_ = value;
+  }
   template <typename Y>
   void reset(Y* value) {
+    static_assert(std::has_virtual_destructor<T>::value,
+                  "T does not have a virtual dtor");
     if (value_ != nullptr) value_->Unref();
     value_ = value;
   }

test/core/gprpp/ref_counted_test.cc

@@ -29,7 +29,10 @@ namespace {
 
 class Foo : public RefCounted<Foo> {
  public:
-  Foo() {}
+  Foo() {
+    static_assert(std::has_virtual_destructor<Foo>::value,
+                  "PolymorphicRefCount doesn't have a virtual dtor");
+  }
 };
 
 TEST(RefCounted, Basic) {
@@ -45,6 +48,28 @@ TEST(RefCounted, ExtraRef) {
   foo->Unref();
 }
 
+class FooNonPolymorphic
+    : public RefCounted<FooNonPolymorphic, NonPolymorphicRefCount> {
+ public:
+  FooNonPolymorphic() {
+    static_assert(!std::has_virtual_destructor<FooNonPolymorphic>::value,
+                  "NonPolymorphicRefCount has a virtual dtor");
+  }
+};
+
+TEST(RefCountedNonPolymorphic, Basic) {
+  FooNonPolymorphic* foo = New<FooNonPolymorphic>();
+  foo->Unref();
+}
+
+TEST(RefCountedNonPolymorphic, ExtraRef) {
+  FooNonPolymorphic* foo = New<FooNonPolymorphic>();
+  RefCountedPtr<FooNonPolymorphic> foop = foo->Ref();
+  foop.release();
+  foo->Unref();
+  foo->Unref();
+}
+
 // Note: We use DebugOnlyTraceFlag instead of TraceFlag to ensure that
 // things build properly in both debug and non-debug cases.
 DebugOnlyTraceFlag foo_tracer(true, "foo");
@@ -66,6 +91,26 @@ TEST(RefCountedWithTracing, Basic) {
   foo->Unref(DEBUG_LOCATION, "original_ref");
 }
 
+class FooNonPolymorphicWithTracing
+    : public RefCountedWithTracing<FooNonPolymorphicWithTracing,
+                                   NonPolymorphicRefCount> {
+ public:
+  FooNonPolymorphicWithTracing() : RefCountedWithTracing(&foo_tracer) {}
+};
+
+TEST(RefCountedNonPolymorphicWithTracing, Basic) {
+  FooNonPolymorphicWithTracing* foo = New<FooNonPolymorphicWithTracing>();
+  RefCountedPtr<FooNonPolymorphicWithTracing> foop =
+      foo->Ref(DEBUG_LOCATION, "extra_ref");
+  foop.release();
+  foo->Unref(DEBUG_LOCATION, "extra_ref");
+  // Can use the no-argument methods, too.
+  foop = foo->Ref();
+  foop.release();
+  foo->Unref();
+  foo->Unref(DEBUG_LOCATION, "original_ref");
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace grpc_core