diff --git a/BUILD b/BUILD index c8c49ff4a41..de3b85375cb 100644 --- a/BUILD +++ b/BUILD @@ -355,6 +355,7 @@ grpc_cc_library( "gpr", "grpc", "grpc++_base", + "grpc_cfstream", "grpc++_codegen_base", "grpc++_codegen_base_src", "grpc++_codegen_proto", @@ -618,10 +619,6 @@ grpc_cc_library( grpc_cc_library( name = "atomic", - hdrs = [ - "src/core/lib/gprpp/atomic_with_atm.h", - "src/core/lib/gprpp/atomic_with_std.h", - ], language = "c++", public_hdrs = [ "src/core/lib/gprpp/atomic.h", @@ -677,6 +674,7 @@ grpc_cc_library( language = "c++", public_hdrs = ["src/core/lib/gprpp/ref_counted.h"], deps = [ + "atomic", "debug_location", "gpr_base", "grpc_trace", diff --git a/build.yaml b/build.yaml index 77ad81ddda2..73929526cd2 100644 --- a/build.yaml +++ b/build.yaml @@ -192,8 +192,6 @@ filegroups: - src/core/lib/gpr/useful.h - src/core/lib/gprpp/abstract.h - src/core/lib/gprpp/atomic.h - - src/core/lib/gprpp/atomic_with_atm.h - - src/core/lib/gprpp/atomic_with_std.h - src/core/lib/gprpp/fork.h - src/core/lib/gprpp/manual_constructor.h - src/core/lib/gprpp/memory.h diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 15ce090bd9b..ad436a663c5 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -251,8 +251,6 @@ Pod::Spec.new do |s| 'src/core/lib/gpr/useful.h', 'src/core/lib/gprpp/abstract.h', 'src/core/lib/gprpp/atomic.h', - 'src/core/lib/gprpp/atomic_with_atm.h', - 'src/core/lib/gprpp/atomic_with_std.h', 'src/core/lib/gprpp/fork.h', 'src/core/lib/gprpp/manual_constructor.h', 'src/core/lib/gprpp/memory.h', @@ -567,8 +565,6 @@ Pod::Spec.new do |s| 'src/core/lib/gpr/useful.h', 'src/core/lib/gprpp/abstract.h', 'src/core/lib/gprpp/atomic.h', - 'src/core/lib/gprpp/atomic_with_atm.h', - 'src/core/lib/gprpp/atomic_with_std.h', 'src/core/lib/gprpp/fork.h', 'src/core/lib/gprpp/manual_constructor.h', 'src/core/lib/gprpp/memory.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 92626f3e84b..23e2f739c39 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -206,8 +206,6 @@ Pod::Spec.new do |s| 'src/core/lib/gpr/useful.h', 'src/core/lib/gprpp/abstract.h', 'src/core/lib/gprpp/atomic.h', - 'src/core/lib/gprpp/atomic_with_atm.h', - 'src/core/lib/gprpp/atomic_with_std.h', 'src/core/lib/gprpp/fork.h', 'src/core/lib/gprpp/manual_constructor.h', 'src/core/lib/gprpp/memory.h', @@ -875,8 +873,6 @@ Pod::Spec.new do |s| 'src/core/lib/gpr/useful.h', 'src/core/lib/gprpp/abstract.h', 'src/core/lib/gprpp/atomic.h', - 'src/core/lib/gprpp/atomic_with_atm.h', - 'src/core/lib/gprpp/atomic_with_std.h', 'src/core/lib/gprpp/fork.h', 'src/core/lib/gprpp/manual_constructor.h', 'src/core/lib/gprpp/memory.h', diff --git a/grpc.gemspec b/grpc.gemspec index a4e25d7bb22..97455f7711b 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -100,8 +100,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/gpr/useful.h ) s.files += %w( src/core/lib/gprpp/abstract.h ) s.files += %w( src/core/lib/gprpp/atomic.h ) - s.files += %w( src/core/lib/gprpp/atomic_with_atm.h ) - s.files += %w( src/core/lib/gprpp/atomic_with_std.h ) 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 ) diff --git a/include/grpcpp/impl/codegen/client_interceptor.h b/include/grpcpp/impl/codegen/client_interceptor.h index 7dfe2290a3f..e36a9da79d2 100644 --- a/include/grpcpp/impl/codegen/client_interceptor.h +++ b/include/grpcpp/impl/codegen/client_interceptor.h @@ -76,7 +76,7 @@ class ClientRpcInfo { UNKNOWN // UNKNOWN is not API and will be removed later }; - ~ClientRpcInfo(){}; + ~ClientRpcInfo() {} // Delete copy constructor but allow default move constructor ClientRpcInfo(const ClientRpcInfo&) = delete; diff --git a/include/grpcpp/impl/codegen/interceptor.h b/include/grpcpp/impl/codegen/interceptor.h index 3af783a61b6..b0f57f71196 100644 --- a/include/grpcpp/impl/codegen/interceptor.h +++ b/include/grpcpp/impl/codegen/interceptor.h @@ -90,7 +90,7 @@ enum class InterceptionHookPoints { /// 5. Set some fields of an RPC at each interception point, when possible class InterceptorBatchMethods { public: - virtual ~InterceptorBatchMethods(){}; + virtual ~InterceptorBatchMethods() {} /// Determine whether the current batch has an interception hook point /// of type \a type virtual bool QueryInterceptionHookPoint(InterceptionHookPoints type) = 0; diff --git a/include/grpcpp/impl/codegen/server_callback.h b/include/grpcpp/impl/codegen/server_callback.h index a0e59215dd6..60c308b22e7 100644 --- a/include/grpcpp/impl/codegen/server_callback.h +++ b/include/grpcpp/impl/codegen/server_callback.h @@ -102,7 +102,7 @@ class ServerCallbackWriter { // Default implementation that can/should be overridden Write(msg, std::move(options)); Finish(std::move(s)); - }; + } protected: template @@ -125,7 +125,7 @@ class ServerCallbackReaderWriter { // Default implementation that can/should be overridden Write(msg, std::move(options)); Finish(std::move(s)); - }; + } protected: void BindReactor(ServerBidiReactor* reactor) { diff --git a/include/grpcpp/impl/codegen/server_interceptor.h b/include/grpcpp/impl/codegen/server_interceptor.h index 3e71b3fc55e..8875a28bf32 100644 --- a/include/grpcpp/impl/codegen/server_interceptor.h +++ b/include/grpcpp/impl/codegen/server_interceptor.h @@ -60,7 +60,7 @@ class ServerRpcInfo { /// Type categorizes RPCs by unary or streaming type enum class Type { UNARY, CLIENT_STREAMING, SERVER_STREAMING, BIDI_STREAMING }; - ~ServerRpcInfo(){}; + ~ServerRpcInfo() {} // Delete all copy and move constructors and assignments ServerRpcInfo(const ServerRpcInfo&) = delete; diff --git a/include/grpcpp/security/credentials.h b/include/grpcpp/security/credentials.h index d8c9e04d778..dfea3900048 100644 --- a/include/grpcpp/security/credentials.h +++ b/include/grpcpp/security/credentials.h @@ -95,7 +95,7 @@ class ChannelCredentials : private GrpcLibraryCodegen { std::unique_ptr> interceptor_creators) { return nullptr; - }; + } }; /// A call credentials object encapsulates the state needed by a client to diff --git a/include/grpcpp/server.h b/include/grpcpp/server.h index 885bd8de8d7..248f20452a5 100644 --- a/include/grpcpp/server.h +++ b/include/grpcpp/server.h @@ -189,7 +189,7 @@ class Server : public ServerInterface, private GrpcLibraryCodegen { /// \param num_cqs How many completion queues does \a cqs hold. void Start(ServerCompletionQueue** cqs, size_t num_cqs) override; - grpc_server* server() override { return server_; }; + grpc_server* server() override { return server_; } private: std::vector>* @@ -223,7 +223,7 @@ class Server : public ServerInterface, private GrpcLibraryCodegen { int max_receive_message_size() const override { return max_receive_message_size_; - }; + } CompletionQueue* CallbackCQ() override; diff --git a/package.xml b/package.xml index 7a1d26c47c5..09593730085 100644 --- a/package.xml +++ b/package.xml @@ -105,8 +105,6 @@ - - diff --git a/src/compiler/protobuf_plugin.h b/src/compiler/protobuf_plugin.h index b971af13109..a3e448aa89d 100644 --- a/src/compiler/protobuf_plugin.h +++ b/src/compiler/protobuf_plugin.h @@ -108,11 +108,11 @@ class ProtoBufService : public grpc_generator::Service { grpc::string name() const { return service_->name(); } - int method_count() const { return service_->method_count(); }; + int method_count() const { return service_->method_count(); } std::unique_ptr method(int i) const { return std::unique_ptr( new ProtoBufMethod(service_->method(i))); - }; + } grpc::string GetLeadingComments(const grpc::string prefix) const { return GetCommentsHelper(service_, true, prefix); @@ -166,7 +166,7 @@ class ProtoBufFile : public grpc_generator::File { grpc::string additional_headers() const { return ""; } - int service_count() const { return file_->service_count(); }; + int service_count() const { return file_->service_count(); } std::unique_ptr service(int i) const { return std::unique_ptr( new ProtoBufService(file_->service(i))); 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 63e381d64c7..fb7b530d044 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 @@ -596,7 +596,7 @@ void GrpcLb::BalancerCallState::StartQuery() { call_error = grpc_call_start_batch_and_execute( lb_call_, ops, (size_t)(op - ops), &lb_on_balancer_status_received_); GPR_ASSERT(GRPC_CALL_OK == call_error); -}; +} void GrpcLb::BalancerCallState::ScheduleNextClientLoadReportLocked() { const grpc_millis next_client_load_report_time = diff --git a/src/core/ext/filters/load_reporting/server_load_reporting_filter.cc b/src/core/ext/filters/load_reporting/server_load_reporting_filter.cc index 6a7231ff7db..d7fd73fd6b2 100644 --- a/src/core/ext/filters/load_reporting/server_load_reporting_filter.cc +++ b/src/core/ext/filters/load_reporting/server_load_reporting_filter.cc @@ -342,8 +342,8 @@ bool MaybeAddServerLoadReportingFilter(const grpc_channel_args& args) { // time if we build with the filter target. struct ServerLoadReportingFilterStaticRegistrar { ServerLoadReportingFilterStaticRegistrar() { - static std::atomic_bool registered{false}; - if (registered) return; + static grpc_core::Atomic registered{false}; + if (registered.Load(grpc_core::MemoryOrder::ACQUIRE)) return; RegisterChannelFilter( "server_load_reporting", GRPC_SERVER_CHANNEL, INT_MAX, @@ -356,7 +356,7 @@ struct ServerLoadReportingFilterStaticRegistrar { ::grpc::load_reporter::MeasureEndBytesReceived(); ::grpc::load_reporter::MeasureEndLatencyMs(); ::grpc::load_reporter::MeasureOtherCallMetric(); - registered = true; + registered.Store(true, grpc_core::MemoryOrder::RELEASE); } } server_load_reporting_filter_static_registrar; diff --git a/src/core/lib/gprpp/atomic.h b/src/core/lib/gprpp/atomic.h index 8b08fc4e9c4..622df1b7889 100644 --- a/src/core/lib/gprpp/atomic.h +++ b/src/core/lib/gprpp/atomic.h @@ -21,10 +21,78 @@ #include -#ifdef GPR_HAS_CXX11_ATOMIC -#include "src/core/lib/gprpp/atomic_with_std.h" -#else -#include "src/core/lib/gprpp/atomic_with_atm.h" -#endif +#include + +namespace grpc_core { + +enum class MemoryOrder { + RELAXED = std::memory_order_relaxed, + CONSUME = std::memory_order_consume, + ACQUIRE = std::memory_order_acquire, + RELEASE = std::memory_order_release, + ACQ_REL = std::memory_order_acq_rel, + SEQ_CST = std::memory_order_seq_cst +}; + +template +class Atomic { + public: + explicit Atomic(T val = T()) : storage_(val) {} + + T Load(MemoryOrder order) const { + return storage_.load(static_cast(order)); + } + + void Store(T val, MemoryOrder order) { + storage_.store(val, static_cast(order)); + } + + bool CompareExchangeWeak(T* expected, T desired, MemoryOrder success, + MemoryOrder failure) { + return GPR_ATM_INC_CAS_THEN( + storage_.compare_exchange_weak(*expected, desired, success, failure)); + } + + bool CompareExchangeStrong(T* expected, T desired, MemoryOrder success, + MemoryOrder failure) { + return GPR_ATM_INC_CAS_THEN(storage_.compare_exchange_weak( + *expected, desired, static_cast(success), + static_cast(failure))); + } + + template + T FetchAdd(Arg arg, MemoryOrder order = MemoryOrder::SEQ_CST) { + return GPR_ATM_INC_ADD_THEN(storage_.fetch_add( + static_cast(arg), static_cast(order))); + } + + template + T FetchSub(Arg arg, MemoryOrder order = MemoryOrder::SEQ_CST) { + return GPR_ATM_INC_ADD_THEN(storage_.fetch_sub( + static_cast(arg), static_cast(order))); + } + + // Atomically increment a counter only if the counter value is not zero. + // Returns true if increment took place; false if counter is zero. + bool IncrementIfNonzero(MemoryOrder load_order = MemoryOrder::ACQ_REL) { + T count = storage_.load(static_cast(load_order)); + do { + // If zero, we are done (without an increment). If not, we must do a CAS + // to maintain the contract: do not increment the counter if it is already + // zero + if (count == 0) { + return false; + } + } while (!storage_.AtomicCompareExchangeWeak( + &count, count + 1, static_cast(MemoryOrder::ACQ_REL), + static_cast(load_order))); + return true; + } + + private: + std::atomic storage_; +}; + +} // namespace grpc_core #endif /* GRPC_CORE_LIB_GPRPP_ATOMIC_H */ diff --git a/src/core/lib/gprpp/atomic_with_atm.h b/src/core/lib/gprpp/atomic_with_atm.h deleted file mode 100644 index 3d0021bb1ce..00000000000 --- a/src/core/lib/gprpp/atomic_with_atm.h +++ /dev/null @@ -1,57 +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. - * - */ - -#ifndef GRPC_CORE_LIB_GPRPP_ATOMIC_WITH_ATM_H -#define GRPC_CORE_LIB_GPRPP_ATOMIC_WITH_ATM_H - -#include - -#include - -namespace grpc_core { - -enum MemoryOrderRelaxed { memory_order_relaxed }; - -template -class atomic; - -template <> -class atomic { - public: - atomic() { gpr_atm_no_barrier_store(&x_, static_cast(false)); } - explicit atomic(bool x) { - gpr_atm_no_barrier_store(&x_, static_cast(x)); - } - - bool compare_exchange_strong(bool& expected, bool update, MemoryOrderRelaxed, - MemoryOrderRelaxed) { - if (!gpr_atm_no_barrier_cas(&x_, static_cast(expected), - static_cast(update))) { - expected = gpr_atm_no_barrier_load(&x_) != 0; - return false; - } - return true; - } - - private: - gpr_atm x_; -}; - -} // namespace grpc_core - -#endif /* GRPC_CORE_LIB_GPRPP_ATOMIC_WITH_ATM_H */ diff --git a/src/core/lib/gprpp/atomic_with_std.h b/src/core/lib/gprpp/atomic_with_std.h deleted file mode 100644 index a4ad16e5cf7..00000000000 --- a/src/core/lib/gprpp/atomic_with_std.h +++ /dev/null @@ -1,35 +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. - * - */ - -#ifndef GRPC_CORE_LIB_GPRPP_ATOMIC_WITH_STD_H -#define GRPC_CORE_LIB_GPRPP_ATOMIC_WITH_STD_H - -#include - -#include - -namespace grpc_core { - -template -using atomic = std::atomic; - -typedef std::memory_order memory_order; - -} // namespace grpc_core - -#endif /* GRPC_CORE_LIB_GPRPP_ATOMIC_WITH_STD_H */ diff --git a/src/core/lib/gprpp/ref_counted.h b/src/core/lib/gprpp/ref_counted.h index fa97ffcfed2..98a7edebf86 100644 --- a/src/core/lib/gprpp/ref_counted.h +++ b/src/core/lib/gprpp/ref_counted.h @@ -31,6 +31,7 @@ #include "src/core/lib/debug/trace.h" #include "src/core/lib/gprpp/abstract.h" +#include "src/core/lib/gprpp/atomic.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" @@ -88,9 +89,7 @@ class RefCount { } // Increases the ref-count by `n`. - void Ref(Value n = 1) { - GPR_ATM_INC_ADD_THEN(value_.fetch_add(n, std::memory_order_relaxed)); - } + void Ref(Value n = 1) { value_.FetchAdd(n, MemoryOrder::RELAXED); } void Ref(const DebugLocation& location, const char* reason, Value n = 1) { #ifndef NDEBUG if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) { @@ -106,8 +105,7 @@ class RefCount { // Similar to Ref() with an assert on the ref-count being non-zero. void RefNonZero() { #ifndef NDEBUG - const Value prior = - GPR_ATM_INC_ADD_THEN(value_.fetch_add(1, std::memory_order_relaxed)); + const Value prior = value_.FetchAdd(1, MemoryOrder::RELAXED); assert(prior > 0); #else Ref(); @@ -127,8 +125,7 @@ class RefCount { // Decrements the ref-count and returns true if the ref-count reaches 0. bool Unref() { - const Value prior = - GPR_ATM_INC_ADD_THEN(value_.fetch_sub(1, std::memory_order_acq_rel)); + const Value prior = value_.FetchSub(1, MemoryOrder::ACQ_REL); GPR_DEBUG_ASSERT(prior > 0); return prior == 1; } @@ -145,12 +142,12 @@ class RefCount { } private: - Value get() const { return value_.load(std::memory_order_relaxed); } + Value get() const { return value_.Load(MemoryOrder::RELAXED); } #ifndef NDEBUG TraceFlag* trace_flag_; #endif - std::atomic value_; + Atomic value_; }; // A base class for reference-counted objects. diff --git a/src/core/lib/gprpp/thd.h b/src/core/lib/gprpp/thd.h index fca9afed1d7..5631c5f1f0e 100644 --- a/src/core/lib/gprpp/thd.h +++ b/src/core/lib/gprpp/thd.h @@ -112,19 +112,22 @@ class Thread { } /// The destructor is strictly optional; either the thread never came to life - /// and the constructor itself killed it or it has already been joined and - /// the Join function kills it. The destructor shouldn't have to do anything. - ~Thread() { GPR_ASSERT(impl_ == nullptr); } + /// and the constructor itself killed it, or it has already been joined and + /// the Join function kills it, or it was detached (non-joinable) and it has + /// run to completion and is now killing itself. The destructor shouldn't have + /// to do anything. + ~Thread() { GPR_ASSERT(!options_.joinable() || impl_ == nullptr); } void Start() { if (impl_ != nullptr) { GPR_ASSERT(state_ == ALIVE); state_ = STARTED; impl_->Start(); - if (!options_.joinable()) { - state_ = DONE; - impl_ = nullptr; - } + // If the Thread is not joinable, then the impl_ will cause the deletion + // of this Thread object when the thread function completes. Since no + // other operation is allowed to a detached thread after Start, there is + // no need to change the value of the impl_ or state_ . The next operation + // on this object will be the deletion, which will trigger the destructor. } else { GPR_ASSERT(state_ == FAILED); } @@ -140,7 +143,7 @@ class Thread { } else { GPR_ASSERT(state_ == FAILED); } - }; + } private: Thread(const Thread&) = delete; diff --git a/src/core/lib/iomgr/buffer_list.h b/src/core/lib/iomgr/buffer_list.h index 3dba15312d6..8bb271867c2 100644 --- a/src/core/lib/iomgr/buffer_list.h +++ b/src/core/lib/iomgr/buffer_list.h @@ -160,6 +160,6 @@ void grpc_tcp_set_write_timestamps_callback(void (*fn)(void*, grpc_core::Timestamps*, grpc_error* error)); -}; /* namespace grpc_core */ +} /* namespace grpc_core */ #endif /* GRPC_CORE_LIB_IOMGR_BUFFER_LIST_H */ diff --git a/src/core/lib/surface/lame_client.cc b/src/core/lib/surface/lame_client.cc index 5a84428b0ee..5f5f10d2ebf 100644 --- a/src/core/lib/surface/lame_client.cc +++ b/src/core/lib/surface/lame_client.cc @@ -25,10 +25,9 @@ #include #include -#include "src/core/lib/gprpp/atomic.h" - #include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/gpr/string.h" +#include "src/core/lib/gprpp/atomic.h" #include "src/core/lib/surface/api_trace.h" #include "src/core/lib/surface/call.h" #include "src/core/lib/surface/channel.h" @@ -43,7 +42,7 @@ struct CallData { grpc_call_combiner* call_combiner; grpc_linked_mdelem status; grpc_linked_mdelem details; - grpc_core::atomic filled_metadata; + grpc_core::Atomic filled_metadata; }; struct ChannelData { @@ -54,9 +53,8 @@ struct ChannelData { static void fill_metadata(grpc_call_element* elem, grpc_metadata_batch* mdb) { CallData* calld = static_cast(elem->call_data); bool expected = false; - if (!calld->filled_metadata.compare_exchange_strong( - expected, true, grpc_core::memory_order_relaxed, - grpc_core::memory_order_relaxed)) { + if (!calld->filled_metadata.CompareExchangeStrong( + &expected, true, MemoryOrder::RELAXED, MemoryOrder::RELAXED)) { return; } ChannelData* chand = static_cast(elem->channel_data); diff --git a/src/cpp/common/core_codegen.cc b/src/cpp/common/core_codegen.cc index 9430dcc9881..ab5f601fdd4 100644 --- a/src/cpp/common/core_codegen.cc +++ b/src/cpp/common/core_codegen.cc @@ -81,7 +81,7 @@ void CoreCodegen::gpr_free(void* p) { return ::gpr_free(p); } void CoreCodegen::grpc_init() { ::grpc_init(); } void CoreCodegen::grpc_shutdown() { ::grpc_shutdown(); } -void CoreCodegen::gpr_mu_init(gpr_mu* mu) { ::gpr_mu_init(mu); }; +void CoreCodegen::gpr_mu_init(gpr_mu* mu) { ::gpr_mu_init(mu); } void CoreCodegen::gpr_mu_destroy(gpr_mu* mu) { ::gpr_mu_destroy(mu); } void CoreCodegen::gpr_mu_lock(gpr_mu* mu) { ::gpr_mu_lock(mu); } void CoreCodegen::gpr_mu_unlock(gpr_mu* mu) { ::gpr_mu_unlock(mu); } diff --git a/src/cpp/server/load_reporter/get_cpu_stats_linux.cc b/src/cpp/server/load_reporter/get_cpu_stats_linux.cc index 9c1fd0cd0b8..561d4f50482 100644 --- a/src/cpp/server/load_reporter/get_cpu_stats_linux.cc +++ b/src/cpp/server/load_reporter/get_cpu_stats_linux.cc @@ -32,7 +32,10 @@ std::pair GetCpuStatsImpl() { FILE* fp; fp = fopen("/proc/stat", "r"); uint64_t user, nice, system, idle; - fscanf(fp, "cpu %lu %lu %lu %lu", &user, &nice, &system, &idle); + if (fscanf(fp, "cpu %lu %lu %lu %lu", &user, &nice, &system, &idle) != 4) { + // Something bad happened with the information, so assume it's all invalid + user = nice = system = idle = 0; + } fclose(fp); busy = user + nice + system; total = busy + idle; diff --git a/src/cpp/server/server_cc.cc b/src/cpp/server/server_cc.cc index 05f78dbe6fe..7eb0f2372b6 100644 --- a/src/cpp/server/server_cc.cc +++ b/src/cpp/server/server_cc.cc @@ -1251,6 +1251,6 @@ CompletionQueue* Server::CallbackCQ() { shutdown_callback->TakeCQ(callback_cq_); } return callback_cq_; -}; +} } // namespace grpc diff --git a/src/python/grpcio_health_checking/grpc_health/v1/health.py b/src/python/grpcio_health_checking/grpc_health/v1/health.py index b08297a5d71..15494fafdbc 100644 --- a/src/python/grpcio_health_checking/grpc_health/v1/health.py +++ b/src/python/grpcio_health_checking/grpc_health/v1/health.py @@ -82,6 +82,7 @@ class HealthServicer(_health_pb2_grpc.HealthServicer): self._send_response_callbacks = {} self.Watch.__func__.experimental_non_blocking = experimental_non_blocking self.Watch.__func__.experimental_thread_pool = experimental_thread_pool + self._gracefully_shutting_down = False def _on_close_callback(self, send_response_callback, service): @@ -135,9 +136,30 @@ class HealthServicer(_health_pb2_grpc.HealthServicer): the service """ with self._lock: - self._server_status[service] = status - if service in self._send_response_callbacks: - for send_response_callback in self._send_response_callbacks[ - service]: - send_response_callback( - _health_pb2.HealthCheckResponse(status=status)) + if self._gracefully_shutting_down: + return + else: + self._server_status[service] = status + if service in self._send_response_callbacks: + for send_response_callback in self._send_response_callbacks[ + service]: + send_response_callback( + _health_pb2.HealthCheckResponse(status=status)) + + def enter_graceful_shutdown(self): + """Permanently sets the status of all services to NOT_SERVING. + + This should be invoked when the server is entering a graceful shutdown + period. After this method is invoked, future attempts to set the status + of a service will be ignored. + + This is an EXPERIMENTAL API. + """ + with self._lock: + if self._gracefully_shutting_down: + return + else: + for service in self._server_status: + self.set(service, + _health_pb2.HealthCheckResponse.NOT_SERVING) # pylint: disable=no-member + self._gracefully_shutting_down = True diff --git a/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py b/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py index f92596f5c9b..1098d38c83e 100644 --- a/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py +++ b/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py @@ -176,6 +176,7 @@ class BaseWatchTests(object): self.assertTrue(response_queue1.empty()) self.assertTrue(response_queue2.empty()) + @unittest.skip("https://github.com/grpc/grpc/issues/18127") def test_cancelled_watch_removed_from_watch_list(self): request = health_pb2.HealthCheckRequest(service=_WATCH_SERVICE) response_queue = queue.Queue() @@ -194,7 +195,7 @@ class BaseWatchTests(object): thread.join() # Wait, if necessary, for serving thread to process client cancellation - timeout = time.time() + test_constants.SHORT_TIMEOUT + timeout = time.time() + test_constants.TIME_ALLOWANCE while time.time( ) < timeout and self._servicer._send_response_callbacks[_WATCH_SERVICE]: time.sleep(1) @@ -203,6 +204,30 @@ class BaseWatchTests(object): 'watch set should be empty') self.assertTrue(response_queue.empty()) + def test_graceful_shutdown(self): + request = health_pb2.HealthCheckRequest(service='') + response_queue = queue.Queue() + rendezvous = self._stub.Watch(request) + thread = threading.Thread( + target=_consume_responses, args=(rendezvous, response_queue)) + thread.start() + + response = response_queue.get(timeout=test_constants.SHORT_TIMEOUT) + self.assertEqual(health_pb2.HealthCheckResponse.SERVING, + response.status) + + self._servicer.enter_graceful_shutdown() + response = response_queue.get(timeout=test_constants.SHORT_TIMEOUT) + self.assertEqual(health_pb2.HealthCheckResponse.NOT_SERVING, + response.status) + + # This should be a no-op. + self._servicer.set('', health_pb2.HealthCheckResponse.SERVING) + + rendezvous.cancel() + thread.join() + self.assertTrue(response_queue.empty()) + class HealthServicerTest(BaseWatchTests.WatchTests): diff --git a/test/cpp/end2end/BUILD b/test/cpp/end2end/BUILD index 1970f3693cb..a9db19dfe8e 100644 --- a/test/cpp/end2end/BUILD +++ b/test/cpp/end2end/BUILD @@ -638,7 +638,6 @@ grpc_cc_test( "//:gpr", "//:grpc", "//:grpc++", - "//:grpc_cfstream", "//src/proto/grpc/testing:echo_messages_proto", "//src/proto/grpc/testing:echo_proto", "//src/proto/grpc/testing:simple_messages_proto", diff --git a/test/cpp/qps/client_callback.cc b/test/cpp/qps/client_callback.cc index 0d637c07fef..815780e40ff 100644 --- a/test/cpp/qps/client_callback.cc +++ b/test/cpp/qps/client_callback.cc @@ -253,18 +253,20 @@ class CallbackStreamingPingPongReactor final : client_(client), ctx_(std::move(ctx)), messages_issued_(0) {} void StartNewRpc() { - if (client_->ThreadCompleted()) return; ctx_->stub_->experimental_async()->StreamingCall(&(ctx_->context_), this); write_time_ = UsageTimer::Now(); StartWrite(client_->request()); + writes_done_started_.clear(); StartCall(); } void OnWriteDone(bool ok) override { - if (!ok || client_->ThreadCompleted()) { - if (!ok) gpr_log(GPR_ERROR, "Error writing RPC"); + if (!ok) { + gpr_log(GPR_ERROR, "Error writing RPC"); + } + if ((!ok || client_->ThreadCompleted()) && + !writes_done_started_.test_and_set()) { StartWritesDone(); - return; } StartRead(&ctx_->response_); } @@ -278,7 +280,9 @@ class CallbackStreamingPingPongReactor final if (!ok) { gpr_log(GPR_ERROR, "Error reading RPC"); } - StartWritesDone(); + if (!writes_done_started_.test_and_set()) { + StartWritesDone(); + } return; } write_time_ = UsageTimer::Now(); @@ -295,8 +299,6 @@ class CallbackStreamingPingPongReactor final } void ScheduleRpc() { - if (client_->ThreadCompleted()) return; - if (!client_->IsClosedLoop()) { gpr_timespec next_issue_time = client_->NextRPCIssueTime(); // Start an alarm callback to run the internal callback after @@ -312,6 +314,7 @@ class CallbackStreamingPingPongReactor final CallbackStreamingPingPongClient* client_; std::unique_ptr ctx_; + std::atomic_flag writes_done_started_; Client::Thread* thread_ptr_; // Needed to update histogram entries double write_time_; // Track ping-pong round start time int messages_issued_; // Messages issued by this stream diff --git a/third_party/cares/cares.BUILD b/third_party/cares/cares.BUILD index fd14007e804..54b8c57b1d6 100644 --- a/third_party/cares/cares.BUILD +++ b/third_party/cares/cares.BUILD @@ -3,6 +3,11 @@ config_setting( values = {"cpu": "darwin"}, ) +config_setting( + name = "darwin_x86_64", + values = {"cpu": "darwin_x86_64"}, +) + config_setting( name = "windows", values = {"cpu": "x64_windows"}, @@ -54,6 +59,7 @@ genrule( ":ios_armv7s": ["@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h"], ":ios_arm64": ["@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h"], ":darwin": ["@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h"], + ":darwin_x86_64": ["@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h"], ":windows": ["@com_github_grpc_grpc//third_party/cares:config_windows/ares_config.h"], ":android": ["@com_github_grpc_grpc//third_party/cares:config_android/ares_config.h"], "//conditions:default": ["@com_github_grpc_grpc//third_party/cares:config_linux/ares_config.h"], diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 2c194c420f3..664a6b3acfe 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1068,8 +1068,6 @@ src/core/lib/gpr/tmpfile.h \ src/core/lib/gpr/useful.h \ src/core/lib/gprpp/abstract.h \ src/core/lib/gprpp/atomic.h \ -src/core/lib/gprpp/atomic_with_atm.h \ -src/core/lib/gprpp/atomic_with_std.h \ src/core/lib/gprpp/debug_location.h \ src/core/lib/gprpp/fork.h \ src/core/lib/gprpp/inlined_vector.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index d1a2debd7e3..1899f119b42 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1157,8 +1157,6 @@ src/core/lib/gpr/wrap_memcpy.cc \ src/core/lib/gprpp/README.md \ src/core/lib/gprpp/abstract.h \ src/core/lib/gprpp/atomic.h \ -src/core/lib/gprpp/atomic_with_atm.h \ -src/core/lib/gprpp/atomic_with_std.h \ src/core/lib/gprpp/debug_location.h \ src/core/lib/gprpp/fork.cc \ src/core/lib/gprpp/fork.h \ diff --git a/tools/http2_interop/doc.go b/tools/http2_interop/doc.go index 6c6b5cb1938..9ae736a7566 100644 --- a/tools/http2_interop/doc.go +++ b/tools/http2_interop/doc.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + // http2interop project doc.go /* diff --git a/tools/http2_interop/frame.go b/tools/http2_interop/frame.go index 12689e9b33d..a2df52ff4ae 100644 --- a/tools/http2_interop/frame.go +++ b/tools/http2_interop/frame.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/frameheader.go b/tools/http2_interop/frameheader.go index 84f6fa5c558..148268b2371 100644 --- a/tools/http2_interop/frameheader.go +++ b/tools/http2_interop/frameheader.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/goaway.go b/tools/http2_interop/goaway.go index 289442d615b..2321709fdc4 100644 --- a/tools/http2_interop/goaway.go +++ b/tools/http2_interop/goaway.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/http1frame.go b/tools/http2_interop/http1frame.go index 68ab197b652..e79d2fde5a8 100644 --- a/tools/http2_interop/http1frame.go +++ b/tools/http2_interop/http1frame.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/http2interop.go b/tools/http2_interop/http2interop.go index fa113961f2a..3af5134f9d8 100644 --- a/tools/http2_interop/http2interop.go +++ b/tools/http2_interop/http2interop.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/http2interop_test.go b/tools/http2_interop/http2interop_test.go index fb314da1964..989b60590c3 100644 --- a/tools/http2_interop/http2interop_test.go +++ b/tools/http2_interop/http2interop_test.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/ping.go b/tools/http2_interop/ping.go index 6011eed4511..4c6868bb414 100644 --- a/tools/http2_interop/ping.go +++ b/tools/http2_interop/ping.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/s6.5.go b/tools/http2_interop/s6.5.go index 4295c46f73a..89ca57f221a 100644 --- a/tools/http2_interop/s6.5.go +++ b/tools/http2_interop/s6.5.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/s6.5_test.go b/tools/http2_interop/s6.5_test.go index 063fd5664c8..61e8a4080e1 100644 --- a/tools/http2_interop/s6.5_test.go +++ b/tools/http2_interop/s6.5_test.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/settings.go b/tools/http2_interop/settings.go index 544cec01ee7..6db7c273daf 100644 --- a/tools/http2_interop/settings.go +++ b/tools/http2_interop/settings.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/testsuite.go b/tools/http2_interop/testsuite.go index 51d36e217ed..c361eec9cb0 100644 --- a/tools/http2_interop/testsuite.go +++ b/tools/http2_interop/testsuite.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/http2_interop/unknownframe.go b/tools/http2_interop/unknownframe.go index 0450e7e976c..dacb249b74f 100644 --- a/tools/http2_interop/unknownframe.go +++ b/tools/http2_interop/unknownframe.go @@ -1,3 +1,17 @@ +// Copyright 2019 The 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. + package http2interop import ( diff --git a/tools/internal_ci/linux/grpc_flaky_network_in_docker.sh b/tools/internal_ci/linux/grpc_flaky_network_in_docker.sh index 60bb49b639a..eb6216c62c3 100755 --- a/tools/internal_ci/linux/grpc_flaky_network_in_docker.sh +++ b/tools/internal_ci/linux/grpc_flaky_network_in_docker.sh @@ -28,4 +28,4 @@ cd /var/local/git/grpc/test/cpp/end2end # iptables is used to drop traffic between client and server apt-get install -y iptables -bazel test --spawn_strategy=standalone --genrule_strategy=standalone --test_output=all :flaky_network_test +bazel test --spawn_strategy=standalone --genrule_strategy=standalone --test_output=all :flaky_network_test --test_env=GRPC_VERBOSITY=debug --test_env=GRPC_TRACE=channel,client_channel,call_error,connectivity_state diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 84d5c45095f..f94357b2c62 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -9288,8 +9288,6 @@ "src/core/lib/gpr/useful.h", "src/core/lib/gprpp/abstract.h", "src/core/lib/gprpp/atomic.h", - "src/core/lib/gprpp/atomic_with_atm.h", - "src/core/lib/gprpp/atomic_with_std.h", "src/core/lib/gprpp/fork.h", "src/core/lib/gprpp/manual_constructor.h", "src/core/lib/gprpp/memory.h", @@ -9336,8 +9334,6 @@ "src/core/lib/gpr/useful.h", "src/core/lib/gprpp/abstract.h", "src/core/lib/gprpp/atomic.h", - "src/core/lib/gprpp/atomic_with_atm.h", - "src/core/lib/gprpp/atomic_with_std.h", "src/core/lib/gprpp/fork.h", "src/core/lib/gprpp/manual_constructor.h", "src/core/lib/gprpp/memory.h",