From 7a7e4f56529c4da16ec6a3035e7b3bdf9dfb6f67 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 16 Aug 2018 12:04:24 -0700 Subject: Move C++ mu_guard class out of C-core public headers and fix style. --- BUILD | 1 + build.yaml | 1 + gRPC-C++.podspec | 2 ++ gRPC-Core.podspec | 2 ++ grpc.gemspec | 1 + include/grpc/support/sync.h | 16 --------- package.xml | 1 + .../client_channel/lb_policy/grpclb/grpclb.cc | 3 +- .../lb_policy/pick_first/pick_first.cc | 5 +-- .../lb_policy/round_robin/round_robin.cc | 5 +-- src/core/lib/channel/channelz_registry.cc | 7 ++-- src/core/lib/gprpp/mutex_lock.h | 42 ++++++++++++++++++++++ src/core/lib/iomgr/ev_epollex_linux.cc | 3 +- .../tsi/ssl/session_cache/ssl_session_cache.cc | 10 +++--- tools/doxygen/Doxyfile.c++.internal | 1 + tools/doxygen/Doxyfile.core.internal | 1 + tools/run_tests/generated/sources_and_headers.json | 2 ++ 17 files changed, 73 insertions(+), 30 deletions(-) create mode 100644 src/core/lib/gprpp/mutex_lock.h diff --git a/BUILD b/BUILD index 35cf86288d..7835e8ea25 100644 --- a/BUILD +++ b/BUILD @@ -560,6 +560,7 @@ grpc_cc_library( "src/core/lib/gprpp/fork.h", "src/core/lib/gprpp/manual_constructor.h", "src/core/lib/gprpp/memory.h", + "src/core/lib/gprpp/mutex_lock.h", "src/core/lib/gprpp/thd.h", "src/core/lib/profiling/timers.h", ], diff --git a/build.yaml b/build.yaml index bd9c4237a1..bb72c41a8c 100644 --- a/build.yaml +++ b/build.yaml @@ -196,6 +196,7 @@ filegroups: - src/core/lib/gprpp/fork.h - src/core/lib/gprpp/manual_constructor.h - src/core/lib/gprpp/memory.h + - src/core/lib/gprpp/mutex_lock.h - src/core/lib/gprpp/thd.h - src/core/lib/profiling/timers.h uses: diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 1d3cedb16b..ae3bd4b312 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -236,6 +236,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/fork.h', 'src/core/lib/gprpp/manual_constructor.h', 'src/core/lib/gprpp/memory.h', + 'src/core/lib/gprpp/mutex_lock.h', 'src/core/lib/gprpp/thd.h', 'src/core/lib/profiling/timers.h', 'src/core/ext/transport/chttp2/transport/bin_decoder.h', @@ -534,6 +535,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/fork.h', 'src/core/lib/gprpp/manual_constructor.h', 'src/core/lib/gprpp/memory.h', + 'src/core/lib/gprpp/mutex_lock.h', 'src/core/lib/gprpp/thd.h', 'src/core/lib/profiling/timers.h', 'src/core/lib/avl/avl.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 1998bc8b4c..146e5cc1ed 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -208,6 +208,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/fork.h', 'src/core/lib/gprpp/manual_constructor.h', 'src/core/lib/gprpp/memory.h', + 'src/core/lib/gprpp/mutex_lock.h', 'src/core/lib/gprpp/thd.h', 'src/core/lib/profiling/timers.h', 'src/core/lib/gpr/alloc.cc', @@ -844,6 +845,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/fork.h', 'src/core/lib/gprpp/manual_constructor.h', 'src/core/lib/gprpp/memory.h', + 'src/core/lib/gprpp/mutex_lock.h', 'src/core/lib/gprpp/thd.h', 'src/core/lib/profiling/timers.h', 'src/core/ext/transport/chttp2/transport/bin_decoder.h', diff --git a/grpc.gemspec b/grpc.gemspec index 55d53cb71d..a5252b06fd 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -105,6 +105,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/gprpp/fork.h ) s.files += %w( src/core/lib/gprpp/manual_constructor.h ) s.files += %w( src/core/lib/gprpp/memory.h ) + s.files += %w( src/core/lib/gprpp/mutex_lock.h ) s.files += %w( src/core/lib/gprpp/thd.h ) s.files += %w( src/core/lib/profiling/timers.h ) s.files += %w( src/core/lib/gpr/alloc.cc ) diff --git a/include/grpc/support/sync.h b/include/grpc/support/sync.h index 91d1fa79b5..da820dece5 100644 --- a/include/grpc/support/sync.h +++ b/include/grpc/support/sync.h @@ -277,22 +277,6 @@ GPRAPI intptr_t gpr_stats_read(const gpr_stats_counter* c); #ifdef __cplusplus } // extern "C" - -namespace grpc_core { - -class mu_guard { - public: - mu_guard(gpr_mu* mu) : mu_(mu) { gpr_mu_lock(mu); } - ~mu_guard() { gpr_mu_unlock(mu_); } - - mu_guard(const mu_guard&) = delete; - mu_guard& operator=(const mu_guard&) = delete; - - private: - gpr_mu* const mu_; -}; - -} // namespace grpc_core #endif #endif /* GRPC_SUPPORT_SYNC_H */ diff --git a/package.xml b/package.xml index 76bdd5ac8f..c89d490b00 100644 --- a/package.xml +++ b/package.xml @@ -110,6 +110,7 @@ + diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 6581385ff9..a624850f4b 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -92,6 +92,7 @@ #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/manual_constructor.h" #include "src/core/lib/gprpp/memory.h" +#include "src/core/lib/gprpp/mutex_lock.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/iomgr/combiner.h" @@ -1259,7 +1260,7 @@ void GrpcLb::FillChildRefsForChannelz(ChildRefsList* child_subchannels, ChildRefsList* child_channels) { // delegate to the RoundRobin to fill the children subchannels. rr_policy_->FillChildRefsForChannelz(child_subchannels, child_channels); - mu_guard guard(&lb_channel_mu_); + MutexLock lock(&lb_channel_mu_); if (lb_channel_ != nullptr) { grpc_core::channelz::ChannelNode* channel_node = grpc_channel_get_channelz_node(lb_channel_); diff --git a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc index 2b6a9ba8c5..0f5041ff51 100644 --- a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc +++ b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc @@ -27,6 +27,7 @@ #include "src/core/ext/filters/client_channel/subchannel.h" #include "src/core/ext/filters/client_channel/subchannel_index.h" #include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/gprpp/mutex_lock.h" #include "src/core/lib/iomgr/combiner.h" #include "src/core/lib/iomgr/sockaddr_utils.h" #include "src/core/lib/transport/connectivity_state.h" @@ -308,7 +309,7 @@ void PickFirst::NotifyOnStateChangeLocked(grpc_connectivity_state* current, void PickFirst::FillChildRefsForChannelz( ChildRefsList* child_subchannels_to_fill, ChildRefsList* ignored) { - mu_guard guard(&child_refs_mu_); + MutexLock lock(&child_refs_mu_); for (size_t i = 0; i < child_subchannels_.size(); ++i) { // TODO(ncteisen): implement a de dup loop that is not O(n^2). Might // have to implement lightweight set. For now, we don't care about @@ -335,7 +336,7 @@ void PickFirst::UpdateChildRefsLocked() { latest_pending_subchannel_list_->PopulateChildRefsList(&cs); } // atomically update the data that channelz will actually be looking at. - mu_guard guard(&child_refs_mu_); + MutexLock lock(&child_refs_mu_); child_subchannels_ = std::move(cs); } diff --git a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc index fea84331d8..c730b3bd2b 100644 --- a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc +++ b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc @@ -36,6 +36,7 @@ #include "src/core/ext/filters/client_channel/subchannel_index.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/debug/trace.h" +#include "src/core/lib/gprpp/mutex_lock.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/iomgr/combiner.h" #include "src/core/lib/iomgr/sockaddr_utils.h" @@ -400,7 +401,7 @@ bool RoundRobin::PickLocked(PickState* pick, grpc_error** error) { void RoundRobin::FillChildRefsForChannelz( ChildRefsList* child_subchannels_to_fill, ChildRefsList* ignored) { - mu_guard guard(&child_refs_mu_); + MutexLock lock(&child_refs_mu_); for (size_t i = 0; i < child_subchannels_.size(); ++i) { // TODO(ncteisen): implement a de dup loop that is not O(n^2). Might // have to implement lightweight set. For now, we don't care about @@ -427,7 +428,7 @@ void RoundRobin::UpdateChildRefsLocked() { latest_pending_subchannel_list_->PopulateChildRefsList(&cs); } // atomically update the data that channelz will actually be looking at. - mu_guard guard(&child_refs_mu_); + MutexLock lock(&child_refs_mu_); child_subchannels_ = std::move(cs); } diff --git a/src/core/lib/channel/channelz_registry.cc b/src/core/lib/channel/channelz_registry.cc index 38496b3d78..f79d2f0c17 100644 --- a/src/core/lib/channel/channelz_registry.cc +++ b/src/core/lib/channel/channelz_registry.cc @@ -23,6 +23,7 @@ #include "src/core/lib/channel/channelz_registry.h" #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/memory.h" +#include "src/core/lib/gprpp/mutex_lock.h" #include #include @@ -53,7 +54,7 @@ ChannelzRegistry::ChannelzRegistry() { gpr_mu_init(&mu_); } ChannelzRegistry::~ChannelzRegistry() { gpr_mu_destroy(&mu_); } intptr_t ChannelzRegistry::InternalRegisterEntry(const RegistryEntry& entry) { - mu_guard guard(&mu_); + MutexLock lock(&mu_); entities_.push_back(entry); intptr_t uuid = entities_.size(); return uuid; @@ -61,7 +62,7 @@ intptr_t ChannelzRegistry::InternalRegisterEntry(const RegistryEntry& entry) { void ChannelzRegistry::InternalUnregisterEntry(intptr_t uuid, EntityType type) { GPR_ASSERT(uuid >= 1); - mu_guard guard(&mu_); + MutexLock lock(&mu_); GPR_ASSERT(static_cast(uuid) <= entities_.size()); GPR_ASSERT(entities_[uuid - 1].type == type); entities_[uuid - 1].object = nullptr; @@ -69,7 +70,7 @@ void ChannelzRegistry::InternalUnregisterEntry(intptr_t uuid, EntityType type) { } void* ChannelzRegistry::InternalGetEntry(intptr_t uuid, EntityType type) { - mu_guard guard(&mu_); + MutexLock lock(&mu_); if (uuid < 1 || uuid > static_cast(entities_.size())) { return nullptr; } diff --git a/src/core/lib/gprpp/mutex_lock.h b/src/core/lib/gprpp/mutex_lock.h new file mode 100644 index 0000000000..54751d5fe4 --- /dev/null +++ b/src/core/lib/gprpp/mutex_lock.h @@ -0,0 +1,42 @@ +/* + * + * Copyright 2018 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. + * + */ + +#ifndef GRPC_CORE_LIB_GPRPP_MUTEX_LOCK_H +#define GRPC_CORE_LIB_GPRPP_MUTEX_LOCK_H + +#include + +#include + +namespace grpc_core { + +class MutexLock { + public: + explicit MutexLock(gpr_mu* mu) : mu_(mu) { gpr_mu_lock(mu); } + ~MutexLock() { gpr_mu_unlock(mu_); } + + MutexLock(const MutexLock&) = delete; + MutexLock& operator=(const MutexLock&) = delete; + + private: + gpr_mu* const mu_; +}; + +} // namespace grpc_core + +#endif /* GRPC_CORE_LIB_GPRPP_MUTEX_LOCK_H */ diff --git a/src/core/lib/iomgr/ev_epollex_linux.cc b/src/core/lib/iomgr/ev_epollex_linux.cc index 96eae30345..b082634af1 100644 --- a/src/core/lib/iomgr/ev_epollex_linux.cc +++ b/src/core/lib/iomgr/ev_epollex_linux.cc @@ -46,6 +46,7 @@ #include "src/core/lib/gpr/tls.h" #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/manual_constructor.h" +#include "src/core/lib/gprpp/mutex_lock.h" #include "src/core/lib/iomgr/block_annotate.h" #include "src/core/lib/iomgr/iomgr_internal.h" #include "src/core/lib/iomgr/is_epollexclusive_available.h" @@ -735,7 +736,7 @@ static void pollset_maybe_finish_shutdown(grpc_pollset* pollset) { static grpc_error* kick_one_worker(grpc_pollset_worker* specific_worker) { GPR_TIMER_SCOPE("kick_one_worker", 0); pollable* p = specific_worker->pollable_obj; - grpc_core::mu_guard lock(&p->mu); + grpc_core::MutexLock lock(&p->mu); GPR_ASSERT(specific_worker != nullptr); if (specific_worker->kicked) { if (grpc_polling_trace.enabled()) { diff --git a/src/core/tsi/ssl/session_cache/ssl_session_cache.cc b/src/core/tsi/ssl/session_cache/ssl_session_cache.cc index fe4f83a13d..ce74fde343 100644 --- a/src/core/tsi/ssl/session_cache/ssl_session_cache.cc +++ b/src/core/tsi/ssl/session_cache/ssl_session_cache.cc @@ -18,9 +18,9 @@ #include -#include "src/core/tsi/ssl/session_cache/ssl_session_cache.h" - +#include "src/core/lib/gprpp/mutex_lock.h" #include "src/core/tsi/ssl/session_cache/ssl_session.h" +#include "src/core/tsi/ssl/session_cache/ssl_session_cache.h" #include #include @@ -97,7 +97,7 @@ SslSessionLRUCache::~SslSessionLRUCache() { } size_t SslSessionLRUCache::Size() { - grpc_core::mu_guard guard(&lock_); + grpc_core::MutexLock lock(&lock_); return use_order_list_size_; } @@ -117,7 +117,7 @@ SslSessionLRUCache::Node* SslSessionLRUCache::FindLocked( } void SslSessionLRUCache::Put(const char* key, SslSessionPtr session) { - grpc_core::mu_guard guard(&lock_); + grpc_core::MutexLock lock(&lock_); Node* node = FindLocked(grpc_slice_from_static_string(key)); if (node != nullptr) { node->SetSession(std::move(session)); @@ -140,7 +140,7 @@ void SslSessionLRUCache::Put(const char* key, SslSessionPtr session) { } SslSessionPtr SslSessionLRUCache::Get(const char* key) { - grpc_core::mu_guard guard(&lock_); + grpc_core::MutexLock lock(&lock_); // Key is only used for lookups. grpc_slice key_slice = grpc_slice_from_static_string(key); Node* node = FindLocked(key_slice); diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 592b0b20e6..43ebf8cad9 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1058,6 +1058,7 @@ src/core/lib/gprpp/fork.h \ src/core/lib/gprpp/inlined_vector.h \ src/core/lib/gprpp/manual_constructor.h \ src/core/lib/gprpp/memory.h \ +src/core/lib/gprpp/mutex_lock.h \ src/core/lib/gprpp/orphanable.h \ src/core/lib/gprpp/ref_counted.h \ src/core/lib/gprpp/ref_counted_ptr.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index fa2ad93a45..0f5047a305 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1142,6 +1142,7 @@ src/core/lib/gprpp/fork.h \ src/core/lib/gprpp/inlined_vector.h \ src/core/lib/gprpp/manual_constructor.h \ src/core/lib/gprpp/memory.h \ +src/core/lib/gprpp/mutex_lock.h \ src/core/lib/gprpp/orphanable.h \ src/core/lib/gprpp/ref_counted.h \ src/core/lib/gprpp/ref_counted_ptr.h \ diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 34e23f09c2..f81f3bb677 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -9306,6 +9306,7 @@ "src/core/lib/gprpp/fork.h", "src/core/lib/gprpp/manual_constructor.h", "src/core/lib/gprpp/memory.h", + "src/core/lib/gprpp/mutex_lock.h", "src/core/lib/gprpp/thd.h", "src/core/lib/profiling/timers.h" ], @@ -9353,6 +9354,7 @@ "src/core/lib/gprpp/fork.h", "src/core/lib/gprpp/manual_constructor.h", "src/core/lib/gprpp/memory.h", + "src/core/lib/gprpp/mutex_lock.h", "src/core/lib/gprpp/thd.h", "src/core/lib/profiling/timers.h" ], -- cgit v1.2.3