diff options
author | ncteisen <ncteisen@gmail.com> | 2018-07-27 02:11:55 -0700 |
---|---|---|
committer | ncteisen <ncteisen@gmail.com> | 2018-07-27 02:11:55 -0700 |
commit | 012ddf15efb51e9b592c7cde6d3ba60aa685e818 (patch) | |
tree | f671b7f4b982913c80accd8da04e6d2ecaa029de | |
parent | 1f533d2ae562f6664e707c0390629b40b0fb1957 (diff) | |
parent | 78aca7bf85fee5eb9c47b430ce20452cec1faf2d (diff) |
Merge branch 'ref_counted_ptr_polymorphism_fix' of https://github.com/markdroth/grpc into channelz-subchannels
-rw-r--r-- | src/core/ext/filters/client_channel/client_channel_channelz.cc | 4 | ||||
-rw-r--r-- | src/core/lib/gprpp/orphanable.h | 6 | ||||
-rw-r--r-- | src/core/lib/gprpp/ref_counted.h | 3 | ||||
-rw-r--r-- | src/core/lib/gprpp/ref_counted_ptr.h | 70 | ||||
-rw-r--r-- | test/core/channel/channel_trace_test.cc | 10 | ||||
-rw-r--r-- | test/core/gprpp/ref_counted_ptr_test.cc | 26 |
6 files changed, 97 insertions, 22 deletions
diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.cc b/src/core/ext/filters/client_channel/client_channel_channelz.cc index b209acc2ea..f797ef1b09 100644 --- a/src/core/ext/filters/client_channel/client_channel_channelz.cc +++ b/src/core/ext/filters/client_channel/client_channel_channelz.cc @@ -142,8 +142,8 @@ grpc_arg ClientChannelNode::CreateChannelArg() { RefCountedPtr<ChannelNode> ClientChannelNode::MakeClientChannelNode( grpc_channel* channel, size_t channel_tracer_max_nodes, bool is_top_level_channel) { - return MakePolymorphicRefCounted<ChannelNode, ClientChannelNode>( - channel, channel_tracer_max_nodes, is_top_level_channel); + return MakeRefCounted<ClientChannelNode>(channel, channel_tracer_max_nodes, + is_top_level_channel); } SubchannelNode::SubchannelNode(grpc_subchannel* subchannel, diff --git a/src/core/lib/gprpp/orphanable.h b/src/core/lib/gprpp/orphanable.h index d0ec9b6461..3123e3f5a3 100644 --- a/src/core/lib/gprpp/orphanable.h +++ b/src/core/lib/gprpp/orphanable.h @@ -86,7 +86,8 @@ class InternallyRefCounted : public Orphanable { GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE // Allow RefCountedPtr<> to access Unref() and IncrementRefCount(). - friend class RefCountedPtr<Child>; + template <typename T> + friend class RefCountedPtr; InternallyRefCounted() { gpr_ref_init(&refs_, 1); } virtual ~InternallyRefCounted() {} @@ -129,7 +130,8 @@ class InternallyRefCountedWithTracing : public Orphanable { GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE // Allow RefCountedPtr<> to access Unref() and IncrementRefCount(). - friend class RefCountedPtr<Child>; + template <typename T> + friend class RefCountedPtr; InternallyRefCountedWithTracing() : InternallyRefCountedWithTracing(static_cast<TraceFlag*>(nullptr)) {} diff --git a/src/core/lib/gprpp/ref_counted.h b/src/core/lib/gprpp/ref_counted.h index 3d6a32ddcf..03c293f6ed 100644 --- a/src/core/lib/gprpp/ref_counted.h +++ b/src/core/lib/gprpp/ref_counted.h @@ -153,7 +153,8 @@ class RefCountedWithTracing { private: // Allow RefCountedPtr<> to access IncrementRefCount(). - friend class RefCountedPtr<Child>; + template <typename T> + friend class RefCountedPtr; void IncrementRefCount() { gpr_ref(&refs_); } diff --git a/src/core/lib/gprpp/ref_counted_ptr.h b/src/core/lib/gprpp/ref_counted_ptr.h index 534d3d03cb..91ca8eae63 100644 --- a/src/core/lib/gprpp/ref_counted_ptr.h +++ b/src/core/lib/gprpp/ref_counted_ptr.h @@ -36,7 +36,10 @@ class RefCountedPtr { RefCountedPtr(std::nullptr_t) {} // If value is non-null, we take ownership of a ref to it. - explicit RefCountedPtr(T* value) { value_ = value; } + template <typename Y> + explicit RefCountedPtr(Y* value) { + value_ = value; + } // Move support. RefCountedPtr(RefCountedPtr&& other) { @@ -49,6 +52,18 @@ class RefCountedPtr { other.value_ = nullptr; return *this; } + template <typename Y> + RefCountedPtr(RefCountedPtr<Y>&& other) { + value_ = other.value_; + other.value_ = nullptr; + } + template <typename Y> + RefCountedPtr& operator=(RefCountedPtr<Y>&& other) { + if (value_ != nullptr) value_->Unref(); + value_ = other.value_; + other.value_ = nullptr; + return *this; + } // Copy support. RefCountedPtr(const RefCountedPtr& other) { @@ -63,17 +78,37 @@ class RefCountedPtr { value_ = other.value_; return *this; } + template <typename Y> + RefCountedPtr(const RefCountedPtr<Y>& other) { + if (other.value_ != nullptr) other.value_->IncrementRefCount(); + value_ = other.value_; + } + template <typename Y> + RefCountedPtr& operator=(const RefCountedPtr<Y>& 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_->IncrementRefCount(); + if (value_ != nullptr) value_->Unref(); + value_ = other.value_; + return *this; + } ~RefCountedPtr() { if (value_ != nullptr) value_->Unref(); } // If value is non-null, we take ownership of a ref to it. - void reset(T* value = nullptr) { + template <typename Y> + void reset(Y* value) { if (value_ != nullptr) value_->Unref(); value_ = value; } + void reset() { + if (value_ != nullptr) value_->Unref(); + value_ = nullptr; + } + // 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 @@ -89,16 +124,34 @@ class RefCountedPtr { T& operator*() const { return *value_; } T* operator->() const { return value_; } - bool operator==(const RefCountedPtr& other) const { + template <typename Y> + bool operator==(const RefCountedPtr<Y>& other) const { return value_ == other.value_; } - bool operator==(const T* other) const { return value_ == other; } - bool operator!=(const RefCountedPtr& other) const { + + template <typename Y> + bool operator==(const Y* other) const { + return value_ == other; + } + + bool operator==(std::nullptr_t) const { return value_ == nullptr; } + + template <typename Y> + bool operator!=(const RefCountedPtr<Y>& other) const { return value_ != other.value_; } - bool operator!=(const T* other) const { return value_ != other; } + + template <typename Y> + bool operator!=(const Y* other) const { + return value_ != other; + } + + bool operator!=(std::nullptr_t) const { return value_ != nullptr; } private: + template <typename Y> + friend class RefCountedPtr; + T* value_ = nullptr; }; @@ -107,11 +160,6 @@ inline RefCountedPtr<T> MakeRefCounted(Args&&... args) { return RefCountedPtr<T>(New<T>(std::forward<Args>(args)...)); } -template <typename Parent, typename Child, typename... Args> -inline RefCountedPtr<Parent> MakePolymorphicRefCounted(Args&&... args) { - return RefCountedPtr<Parent>(New<Child>(std::forward<Args>(args)...)); -} - } // namespace grpc_core #endif /* GRPC_CORE_LIB_GPRPP_REF_COUNTED_PTR_H */ diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc index 3d6aff03eb..efa625bccf 100644 --- a/test/core/channel/channel_trace_test.cc +++ b/test/core/channel/channel_trace_test.cc @@ -187,8 +187,8 @@ TEST_P(ChannelTracerTest, ComplexTest) { AddSimpleTrace(&tracer); AddSimpleTrace(&tracer); AddSimpleTrace(&tracer); - sc1.reset(nullptr); - sc2.reset(nullptr); + sc1.reset(); + sc2.reset(); } // Test a case in which the parent channel has subchannels and the subchannels @@ -234,9 +234,9 @@ TEST_P(ChannelTracerTest, TestNesting) { grpc_slice_from_static_string("subchannel one inactive"), sc1); AddSimpleTrace(&tracer); ValidateChannelTrace(&tracer, 8, GetParam()); - sc1.reset(nullptr); - sc2.reset(nullptr); - conn1.reset(nullptr); + sc1.reset(); + sc2.reset(); + conn1.reset(); } INSTANTIATE_TEST_CASE_P(ChannelTracerTestSweep, ChannelTracerTest, diff --git a/test/core/gprpp/ref_counted_ptr_test.cc b/test/core/gprpp/ref_counted_ptr_test.cc index aa30b72282..6df6e348c6 100644 --- a/test/core/gprpp/ref_counted_ptr_test.cc +++ b/test/core/gprpp/ref_counted_ptr_test.cc @@ -127,7 +127,7 @@ TEST(RefCountedPtr, ResetFromNonNullToNull) { TEST(RefCountedPtr, ResetFromNullToNull) { RefCountedPtr<Foo> foo; EXPECT_EQ(nullptr, foo.get()); - foo.reset(nullptr); + foo.reset(); EXPECT_EQ(nullptr, foo.get()); } @@ -175,6 +175,30 @@ TEST(RefCountedPtr, RefCountedWithTracing) { foo->Unref(DEBUG_LOCATION, "foo"); } +class Parent : public RefCounted<Parent> { + public: + Parent() {} +}; + +class Child : public Parent { + public: + Child() {} +}; + +void FunctionTakingParent(RefCountedPtr<Parent> o) {} + +void FunctionTakingChild(RefCountedPtr<Child> o) {} + +TEST(RefCountedPtr, CanPassChildToFunctionExpectingParent) { + RefCountedPtr<Child> child = MakeRefCounted<Child>(); + FunctionTakingParent(child); +} + +TEST(RefCountedPtr, CanPassChildToFunctionExpectingChild) { + RefCountedPtr<Child> child = MakeRefCounted<Child>(); + FunctionTakingChild(child); +} + } // namespace } // namespace testing } // namespace grpc_core |