diff --git a/BUILD b/BUILD index 9c93a5c0990..6ba5e6f6554 100644 --- a/BUILD +++ b/BUILD @@ -545,7 +545,6 @@ grpc_cc_library( grpc_cc_library( name = "reference_counted", - srcs = ["src/core/lib/support/reference_counted.cc"], public_hdrs = ["src/core/lib/support/reference_counted.h"], language = "c++", deps = [ diff --git a/CMakeLists.txt b/CMakeLists.txt index 7e18465c926..7b214be4352 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -809,7 +809,6 @@ add_library(gpr src/core/lib/support/log_windows.cc src/core/lib/support/mpscq.cc src/core/lib/support/murmur_hash.cc - src/core/lib/support/reference_counted.cc src/core/lib/support/stack_lockfree.cc src/core/lib/support/string.cc src/core/lib/support/string_posix.cc diff --git a/Makefile b/Makefile index 848182d1bd1..a81e62b4409 100644 --- a/Makefile +++ b/Makefile @@ -2846,7 +2846,6 @@ LIBGPR_SRC = \ src/core/lib/support/log_windows.cc \ src/core/lib/support/mpscq.cc \ src/core/lib/support/murmur_hash.cc \ - src/core/lib/support/reference_counted.cc \ src/core/lib/support/stack_lockfree.cc \ src/core/lib/support/string.cc \ src/core/lib/support/string_posix.cc \ diff --git a/build.yaml b/build.yaml index 9ac86eb430b..57a65b37075 100644 --- a/build.yaml +++ b/build.yaml @@ -49,7 +49,6 @@ filegroups: - src/core/lib/support/log_windows.cc - src/core/lib/support/mpscq.cc - src/core/lib/support/murmur_hash.cc - - src/core/lib/support/reference_counted.cc - src/core/lib/support/stack_lockfree.cc - src/core/lib/support/string.cc - src/core/lib/support/string_posix.cc diff --git a/config.m4 b/config.m4 index 9d514c8552b..d2f2520feac 100644 --- a/config.m4 +++ b/config.m4 @@ -62,7 +62,6 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/support/log_windows.cc \ src/core/lib/support/mpscq.cc \ src/core/lib/support/murmur_hash.cc \ - src/core/lib/support/reference_counted.cc \ src/core/lib/support/stack_lockfree.cc \ src/core/lib/support/string.cc \ src/core/lib/support/string_posix.cc \ diff --git a/config.w32 b/config.w32 index 2f24fcd8b01..8a713751dcd 100644 --- a/config.w32 +++ b/config.w32 @@ -39,7 +39,6 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\support\\log_windows.cc " + "src\\core\\lib\\support\\mpscq.cc " + "src\\core\\lib\\support\\murmur_hash.cc " + - "src\\core\\lib\\support\\reference_counted.cc " + "src\\core\\lib\\support\\stack_lockfree.cc " + "src\\core\\lib\\support\\string.cc " + "src\\core\\lib\\support\\string_posix.cc " + diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index f623f26831e..bc500cc0d78 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -229,7 +229,6 @@ Pod::Spec.new do |s| 'src/core/lib/support/log_windows.cc', 'src/core/lib/support/mpscq.cc', 'src/core/lib/support/murmur_hash.cc', - 'src/core/lib/support/reference_counted.cc', 'src/core/lib/support/stack_lockfree.cc', 'src/core/lib/support/string.cc', 'src/core/lib/support/string_posix.cc', diff --git a/grpc.gemspec b/grpc.gemspec index da1b6c0aeff..d1e8b5254dc 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -126,7 +126,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/support/log_windows.cc ) s.files += %w( src/core/lib/support/mpscq.cc ) s.files += %w( src/core/lib/support/murmur_hash.cc ) - s.files += %w( src/core/lib/support/reference_counted.cc ) s.files += %w( src/core/lib/support/stack_lockfree.cc ) s.files += %w( src/core/lib/support/string.cc ) s.files += %w( src/core/lib/support/string_posix.cc ) diff --git a/grpc.gyp b/grpc.gyp index 0ee6529013b..fb153915ff0 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -184,7 +184,6 @@ 'src/core/lib/support/log_windows.cc', 'src/core/lib/support/mpscq.cc', 'src/core/lib/support/murmur_hash.cc', - 'src/core/lib/support/reference_counted.cc', 'src/core/lib/support/stack_lockfree.cc', 'src/core/lib/support/string.cc', 'src/core/lib/support/string_posix.cc', diff --git a/package.xml b/package.xml index 92eab40b87b..6f786a13ad9 100644 --- a/package.xml +++ b/package.xml @@ -138,7 +138,6 @@ - diff --git a/src/core/lib/support/debug_location.h b/src/core/lib/support/debug_location.h index 47c5082874c..0939da595d2 100644 --- a/src/core/lib/support/debug_location.h +++ b/src/core/lib/support/debug_location.h @@ -21,6 +21,9 @@ namespace grpc_core { +// Used for tracking file and line where a call is made for debug builds. +// No-op for non-debug builds. +// Callers can use the DEBUG_LOCATION macro in either case. #ifndef NDEBUG class DebugLocation { public: diff --git a/src/core/lib/support/reference_counted.cc b/src/core/lib/support/reference_counted.cc deleted file mode 100644 index 222628a6fab..00000000000 --- a/src/core/lib/support/reference_counted.cc +++ /dev/null @@ -1,58 +0,0 @@ -/* - * - * Copyright 2017 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -#include "src/core/lib/support/reference_counted.h" - -#include - -#include "src/core/lib/support/memory.h" - -namespace grpc_core { - -void ReferenceCounted::Ref(const DebugLocation& location, const char* reason) { - 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(); -} - -void ReferenceCounted::Ref() { gpr_ref(&refs_); } - -bool ReferenceCounted::Unref(const DebugLocation& location, - const char* reason) { - 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 unref %" PRIdPTR " -> %" PRIdPTR " %s", - trace_flag_->name(), this, location.file(), location.line(), - old_refs, old_refs - 1, reason); - } - return Unref(); -} - -bool ReferenceCounted::Unref() { - if (gpr_unref(&refs_)) { - Delete(this); - return true; - } - return false; -} - -} // namespace grpc_core diff --git a/src/core/lib/support/reference_counted.h b/src/core/lib/support/reference_counted.h index 42bedcbd3ad..ff74de082c7 100644 --- a/src/core/lib/support/reference_counted.h +++ b/src/core/lib/support/reference_counted.h @@ -20,19 +20,46 @@ #define GRPC_CORE_LIB_SUPPORT_REFERENCE_COUNTED_H #include +#include #include "src/core/lib/debug/trace.h" #include "src/core/lib/support/debug_location.h" +#include "src/core/lib/support/memory.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(). class ReferenceCounted { public: - void Ref(); - void Ref(const DebugLocation& location, const char* reason); + void Ref() { gpr_ref(&refs_); } - bool Unref(); - bool Unref(const DebugLocation& location, const char* reason); + void Ref(const DebugLocation& location, const char* reason) { + 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(); + } + + void Unref() { + if (gpr_unref(&refs_)) { + Delete(this); + } + } + + void Unref(const DebugLocation& location, const char* reason) { + 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 unref %" PRIdPTR " -> %" PRIdPTR " %s", + trace_flag_->name(), this, location.file(), location.line(), + old_refs, old_refs - 1, reason); + } + Unref(); + } // Not copyable nor movable. ReferenceCounted(const ReferenceCounted&) = delete; @@ -43,6 +70,8 @@ class ReferenceCounted { template friend void Delete(T*); + ReferenceCounted() : ReferenceCounted(nullptr) {} + explicit ReferenceCounted(TraceFlag* trace_flag) : trace_flag_(trace_flag) { gpr_ref_init(&refs_, 1); } @@ -50,7 +79,7 @@ class ReferenceCounted { virtual ~ReferenceCounted() {} private: - TraceFlag* trace_flag_; + TraceFlag* trace_flag_ = nullptr; gpr_refcount refs_; }; diff --git a/src/core/lib/support/reference_counted_ptr.h b/src/core/lib/support/reference_counted_ptr.h index 37823809431..1590549bfbb 100644 --- a/src/core/lib/support/reference_counted_ptr.h +++ b/src/core/lib/support/reference_counted_ptr.h @@ -53,8 +53,10 @@ class ReferenceCountedPtr { value_ = other.value_; } ReferenceCountedPtr& operator=(const ReferenceCountedPtr& other) { - if (value_ != nullptr) value_->Unref(); + // 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 (value_ != nullptr) value_->Unref(); value_ = other.value_; return *this; } diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 38f2a401330..330c4185c61 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -38,7 +38,6 @@ CORE_SOURCE_FILES = [ 'src/core/lib/support/log_windows.cc', 'src/core/lib/support/mpscq.cc', 'src/core/lib/support/murmur_hash.cc', - 'src/core/lib/support/reference_counted.cc', 'src/core/lib/support/stack_lockfree.cc', 'src/core/lib/support/string.cc', 'src/core/lib/support/string_posix.cc', diff --git a/test/core/support/reference_counted_ptr_test.cc b/test/core/support/reference_counted_ptr_test.cc index 942c7247245..14dd620887c 100644 --- a/test/core/support/reference_counted_ptr_test.cc +++ b/test/core/support/reference_counted_ptr_test.cc @@ -79,6 +79,19 @@ TEST(ReferenceCountedPtr, CopyAssignment) { EXPECT_EQ(foo.get(), foo2.get()); } +TEST(ReferenceCountedPtr, CopyAssignmentWhenEmpty) { + ReferenceCountedPtr foo; + ReferenceCountedPtr foo2; + foo2 = foo; + EXPECT_EQ(nullptr, foo.get()); + EXPECT_EQ(nullptr, foo2.get()); +} + +TEST(ReferenceCountedPtr, CopyAssignmentToSelf) { + ReferenceCountedPtr foo(New()); + foo = foo; +} + TEST(ReferenceCountedPtr, EnclosedScope) { ReferenceCountedPtr foo(New()); { diff --git a/test/core/support/reference_counted_test.cc b/test/core/support/reference_counted_test.cc index c5795429c40..b1f79e61e6d 100644 --- a/test/core/support/reference_counted_test.cc +++ b/test/core/support/reference_counted_test.cc @@ -28,7 +28,7 @@ namespace testing { class Foo : public ReferenceCounted { public: - Foo() : ReferenceCounted(nullptr) {} + Foo() {} }; TEST(ReferenceCounted, StackAllocated) { Foo foo; } diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 37ffff6b79d..8343f88a374 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1299,7 +1299,6 @@ src/core/lib/support/mpscq.cc \ src/core/lib/support/mpscq.h \ src/core/lib/support/murmur_hash.cc \ src/core/lib/support/murmur_hash.h \ -src/core/lib/support/reference_counted.cc \ src/core/lib/support/reference_counted.h \ src/core/lib/support/reference_counted_ptr.h \ src/core/lib/support/spinlock.h \ diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index ed5128d9c12..284a2098e3e 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -7808,7 +7808,6 @@ "src/core/lib/support/log_windows.cc", "src/core/lib/support/mpscq.cc", "src/core/lib/support/murmur_hash.cc", - "src/core/lib/support/reference_counted.cc", "src/core/lib/support/stack_lockfree.cc", "src/core/lib/support/string.cc", "src/core/lib/support/string_posix.cc",