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

pull/14221/head
Mark D. Roth 7 years ago
parent 007f1197ac
commit 08d9f3df30
  1. 2
      BUILD
  2. 8
      src/core/ext/filters/client_channel/subchannel.cc
  3. 4
      src/core/ext/filters/client_channel/subchannel.h
  4. 41
      src/core/lib/gprpp/orphanable.h
  5. 40
      src/core/lib/gprpp/ref_counted.h
  6. 18
      src/core/lib/gprpp/ref_counted_ptr.h
  7. 20
      test/core/gprpp/orphanable_test.cc
  8. 7
      test/core/gprpp/ref_counted_ptr_test.cc
  9. 13
      test/core/gprpp/ref_counted_test.cc

@ -599,6 +599,7 @@ grpc_cc_library(
public_hdrs = ["src/core/lib/gprpp/orphanable.h"],
deps = [
"debug_location",
"ref_counted_ptr",
"gpr++_base",
"grpc_trace",
],
@ -610,6 +611,7 @@ grpc_cc_library(
public_hdrs = ["src/core/lib/gprpp/ref_counted.h"],
deps = [
"debug_location",
"ref_counted_ptr",
"gpr++_base",
"grpc_trace",
],

@ -738,8 +738,9 @@ grpc_arg grpc_create_subchannel_address_arg(const grpc_resolved_address* addr) {
}
namespace grpc_core {
ConnectedSubchannel::ConnectedSubchannel(grpc_channel_stack* channel_stack)
: grpc_core::RefCountedWithTracing(&grpc_trace_stream_refcount),
: RefCountedWithTracing<ConnectedSubchannel>(&grpc_trace_stream_refcount),
channel_stack_(channel_stack) {}
ConnectedSubchannel::~ConnectedSubchannel() {
@ -774,7 +775,9 @@ grpc_error* ConnectedSubchannel::CreateCall(const CallArgs& args,
args.arena,
sizeof(grpc_subchannel_call) + channel_stack_->call_stack_size);
grpc_call_stack* callstk = SUBCHANNEL_CALL_TO_CALL_STACK(*call);
Ref(DEBUG_LOCATION, "subchannel_call");
RefCountedPtr<ConnectedSubchannel> connection =
Ref(DEBUG_LOCATION, "subchannel_call");
connection.release(); // Ref is passed to the grpc_subchannel_call object.
(*call)->connection = this;
const grpc_call_element_args call_args = {
callstk, /* call_stack */
@ -796,4 +799,5 @@ grpc_error* ConnectedSubchannel::CreateCall(const CallArgs& args,
grpc_call_stack_set_pollset_or_pollset_set(callstk, args.pollent);
return GRPC_ERROR_NONE;
}
} // namespace grpc_core

@ -68,7 +68,8 @@ typedef struct grpc_subchannel_key grpc_subchannel_key;
#endif
namespace grpc_core {
class ConnectedSubchannel : public grpc_core::RefCountedWithTracing {
class ConnectedSubchannel : public RefCountedWithTracing<ConnectedSubchannel> {
public:
struct CallArgs {
grpc_polling_entity* pollent;
@ -93,6 +94,7 @@ class ConnectedSubchannel : public grpc_core::RefCountedWithTracing {
private:
grpc_channel_stack* channel_stack_;
};
} // namespace grpc_core
grpc_subchannel* grpc_subchannel_ref(

@ -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_;
};

@ -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_;
};

@ -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_; }

@ -58,18 +58,19 @@ TEST(MakeOrphanable, WithParameters) {
EXPECT_EQ(5, foo->value());
}
class Bar : public InternallyRefCounted {
class Bar : public InternallyRefCounted<Bar> {
public:
Bar() : Bar(0) {}
explicit Bar(int value) : value_(value) {}
void Orphan() override { Unref(); }
int value() const { return value_; }
void StartWork() { Ref(); }
void FinishWork() { Unref(); }
void StartWork() { self_ref_ = Ref(); }
void FinishWork() { self_ref_.reset(); }
private:
int value_;
RefCountedPtr<Bar> self_ref_;
};
TEST(OrphanablePtr, InternallyRefCounted) {
@ -82,19 +83,24 @@ TEST(OrphanablePtr, InternallyRefCounted) {
// things build properly in both debug and non-debug cases.
DebugOnlyTraceFlag baz_tracer(true, "baz");
class Baz : public InternallyRefCountedWithTracing {
class Baz : public InternallyRefCountedWithTracing<Baz> {
public:
Baz() : Baz(0) {}
explicit Baz(int value)
: InternallyRefCountedWithTracing(&baz_tracer), value_(value) {}
: InternallyRefCountedWithTracing<Baz>(&baz_tracer), value_(value) {}
void Orphan() override { Unref(); }
int value() const { return value_; }
void StartWork() { Ref(DEBUG_LOCATION, "work"); }
void FinishWork() { Unref(DEBUG_LOCATION, "work"); }
void StartWork() { self_ref_ = Ref(DEBUG_LOCATION, "work"); }
void FinishWork() {
// This is a little ugly, but it makes the logged ref and unref match up.
self_ref_.release();
Unref(DEBUG_LOCATION, "work");
}
private:
int value_;
RefCountedPtr<Baz> self_ref_;
};
TEST(OrphanablePtr, InternallyRefCountedWithTracing) {

@ -30,7 +30,7 @@ namespace grpc_core {
namespace testing {
namespace {
class Foo : public RefCounted {
class Foo : public RefCounted<Foo> {
public:
Foo() : value_(0) {}
@ -163,14 +163,15 @@ TEST(MakeRefCounted, Args) {
TraceFlag foo_tracer(true, "foo");
class FooWithTracing : public RefCountedWithTracing {
class FooWithTracing : public RefCountedWithTracing<FooWithTracing> {
public:
FooWithTracing() : RefCountedWithTracing(&foo_tracer) {}
};
TEST(RefCountedPtr, RefCountedWithTracing) {
RefCountedPtr<FooWithTracing> foo(New<FooWithTracing>());
foo->Ref(DEBUG_LOCATION, "foo");
RefCountedPtr<FooWithTracing> foo2 = foo->Ref(DEBUG_LOCATION, "foo");
foo2.release();
foo->Unref(DEBUG_LOCATION, "foo");
}

@ -27,7 +27,7 @@ namespace grpc_core {
namespace testing {
namespace {
class Foo : public RefCounted {
class Foo : public RefCounted<Foo> {
public:
Foo() {}
};
@ -39,7 +39,8 @@ TEST(RefCounted, Basic) {
TEST(RefCounted, ExtraRef) {
Foo* foo = New<Foo>();
foo->Ref();
RefCountedPtr<Foo> foop = foo->Ref();
foop.release();
foo->Unref();
foo->Unref();
}
@ -48,17 +49,19 @@ TEST(RefCounted, ExtraRef) {
// things build properly in both debug and non-debug cases.
DebugOnlyTraceFlag foo_tracer(true, "foo");
class FooWithTracing : public RefCountedWithTracing {
class FooWithTracing : public RefCountedWithTracing<FooWithTracing> {
public:
FooWithTracing() : RefCountedWithTracing(&foo_tracer) {}
};
TEST(RefCountedWithTracing, Basic) {
FooWithTracing* foo = New<FooWithTracing>();
foo->Ref(DEBUG_LOCATION, "extra_ref");
RefCountedPtr<FooWithTracing> foop = foo->Ref(DEBUG_LOCATION, "extra_ref");
foop.release();
foo->Unref(DEBUG_LOCATION, "extra_ref");
// Can use the no-argument methods, too.
foo->Ref();
foop = foo->Ref();
foop.release();
foo->Unref();
foo->Unref(DEBUG_LOCATION, "original_ref");
}

Loading…
Cancel
Save