diff --git a/src/core/BUILD b/src/core/BUILD index 39967528ae9..99ce258ea65 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -1236,6 +1236,7 @@ grpc_cc_library( public_hdrs = ["lib/gprpp/dual_ref_counted.h"], deps = [ "down_cast", + "ref_counted", "//:debug_location", "//:gpr", "//:orphanable", diff --git a/src/core/lib/gprpp/dual_ref_counted.h b/src/core/lib/gprpp/dual_ref_counted.h index 5641b0970d4..38365db381b 100644 --- a/src/core/lib/gprpp/dual_ref_counted.h +++ b/src/core/lib/gprpp/dual_ref_counted.h @@ -28,6 +28,7 @@ #include "src/core/lib/gprpp/debug_location.h" #include "src/core/lib/gprpp/down_cast.h" #include "src/core/lib/gprpp/orphanable.h" +#include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" namespace grpc_core { @@ -46,15 +47,16 @@ namespace grpc_core { // // This will be used by CRTP (curiously-recurring template pattern), e.g.: // class MyClass : public RefCounted { ... }; -template -class DualRefCounted { +// +// Impl & UnrefBehavior are as per RefCounted. +template +class DualRefCounted : public Impl { public: // Not copyable nor movable. DualRefCounted(const DualRefCounted&) = delete; DualRefCounted& operator=(const DualRefCounted&) = delete; - virtual ~DualRefCounted() = default; - GRPC_MUST_USE_RESULT RefCountedPtr Ref() { IncrementRefCount(); return RefCountedPtr(static_cast(this)); @@ -215,7 +217,7 @@ class DualRefCounted { CHECK_GT(weak_refs, 0u); #endif if (GPR_UNLIKELY(prev_ref_pair == MakeRefPair(0, 1))) { - delete static_cast(this); + unref_behavior_(static_cast(this)); } } void WeakUnref(const DebugLocation& location, const char* reason) { @@ -242,7 +244,7 @@ class DualRefCounted { (void)reason; #endif if (GPR_UNLIKELY(prev_ref_pair == MakeRefPair(0, 1))) { - delete static_cast(this); + unref_behavior_(static_cast(this)); } } @@ -266,6 +268,9 @@ class DualRefCounted { // Ref count has dropped to zero, so the object is now orphaned. virtual void Orphaned() = 0; + // Note: Depending on the Impl used, this dtor can be implicitly virtual. + ~DualRefCounted() = default; + private: // Allow RefCountedPtr<> to access IncrementRefCount(). template @@ -358,6 +363,7 @@ class DualRefCounted { const char* trace_; #endif std::atomic refs_{0}; + GPR_NO_UNIQUE_ADDRESS UnrefBehavior unref_behavior_; }; } // namespace grpc_core diff --git a/test/core/gprpp/dual_ref_counted_test.cc b/test/core/gprpp/dual_ref_counted_test.cc index f34a3b8949b..1f7cf80e41d 100644 --- a/test/core/gprpp/dual_ref_counted_test.cc +++ b/test/core/gprpp/dual_ref_counted_test.cc @@ -21,6 +21,8 @@ #include "absl/log/check.h" #include "gtest/gtest.h" +#include "src/core/lib/gprpp/manual_constructor.h" +#include "src/core/lib/gprpp/ref_counted.h" #include "test/core/test_util/test_config.h" namespace grpc_core { @@ -115,6 +117,42 @@ TEST(DualRefCountedWithTracing, Basic) { foo->Unref(DEBUG_LOCATION, "original_ref"); } +class FooWithNoDelete final + : public DualRefCounted { + public: + FooWithNoDelete(bool* orphaned_called, bool* destructor_called) + : DualRefCounted( + "FooWithNoDelete"), + orphaned_called_(orphaned_called), + destructor_called_(destructor_called) {} + ~FooWithNoDelete() { *destructor_called_ = true; } + + void Orphaned() override { *orphaned_called_ = true; } + + private: + bool* const orphaned_called_; + bool* const destructor_called_; +}; + +TEST(DualRefCountedWithNoDelete, Basic) { + ManualConstructor foo; + bool destructor_called = false; + bool orphaned_called = false; + foo.Init(&orphaned_called, &destructor_called); + EXPECT_FALSE(orphaned_called); + EXPECT_FALSE(destructor_called); + foo->WeakRef().release(); + EXPECT_FALSE(orphaned_called); + EXPECT_FALSE(destructor_called); + foo->Unref(); + EXPECT_TRUE(orphaned_called); + EXPECT_FALSE(destructor_called); + foo->WeakUnref(); + EXPECT_TRUE(orphaned_called); + EXPECT_TRUE(destructor_called); +} + } // namespace } // namespace testing } // namespace grpc_core