From ca73801714034108c1b55e0c175202cb7cc37e87 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 21 Oct 2019 09:16:29 -0700 Subject: [PATCH 01/30] Remove unused-parameter warnings, round 2 (1 of 19) --- src/compiler/cpp_generator.cc | 28 ++++++----------- src/compiler/generator_helpers.h | 2 +- src/compiler/ruby_plugin.cc | 4 +-- .../filters/client_channel/backup_poller.cc | 2 +- .../client_channel/channel_connectivity.cc | 4 +-- .../filters/client_channel/client_channel.cc | 31 ++++++++++--------- .../client_channel/client_channel_factory.cc | 2 +- .../client_channel/global_subchannel_pool.cc | 10 +++--- .../health/health_check_client.cc | 22 +++++++------ .../client_channel/http_connect_handshaker.cc | 6 ++-- 10 files changed, 53 insertions(+), 58 deletions(-) diff --git a/src/compiler/cpp_generator.cc b/src/compiler/cpp_generator.cc index ec507baede7..9da45831169 100644 --- a/src/compiler/cpp_generator.cc +++ b/src/compiler/cpp_generator.cc @@ -584,7 +584,7 @@ void PrintHeaderClientMethod(grpc_generator::Printer* printer, void PrintHeaderClientMethodCallbackInterfacesStart( grpc_generator::Printer* printer, - std::map* vars) { + std::map* /*vars*/) { // This declares the interface for the callback-based API. The components // are pure; even though this is new (post-1.0) API, it can be pure because // it is an entirely new interface that happens to be scoped within @@ -599,10 +599,7 @@ void PrintHeaderClientMethodCallbackInterfacesStart( void PrintHeaderClientMethodCallbackInterfaces( grpc_generator::Printer* printer, const grpc_generator::Method* method, - std::map* vars, bool is_public) { - // Reserve is_public for future expansion - assert(is_public); - + std::map* vars) { (*vars)["Method"] = method->name(); (*vars)["Request"] = method->input_type_name(); (*vars)["Response"] = method->output_type_name(); @@ -646,7 +643,7 @@ void PrintHeaderClientMethodCallbackInterfaces( void PrintHeaderClientMethodCallbackInterfacesEnd( grpc_generator::Printer* printer, - std::map* vars) { + std::map* /*vars*/) { printer->Outdent(); printer->Print("};\n"); @@ -662,7 +659,7 @@ void PrintHeaderClientMethodCallbackInterfacesEnd( void PrintHeaderClientMethodCallbackStart( grpc_generator::Printer* printer, - std::map* vars) { + std::map* /*vars*/) { // This declares the stub entry for the callback-based API. printer->Print("class experimental_async final :\n"); printer->Print(" public StubInterface::experimental_async_interface {\n"); @@ -670,13 +667,9 @@ void PrintHeaderClientMethodCallbackStart( printer->Indent(); } -void PrintHeaderClientMethodCallback(grpc_generator::Printer* printer, - const grpc_generator::Method* method, - std::map* vars, - bool is_public) { - // Reserve is_public for future expansion - assert(is_public); - +void PrintHeaderClientMethodCallback( + grpc_generator::Printer* printer, const grpc_generator::Method* method, + std::map* vars) { (*vars)["Method"] = method->name(); (*vars)["Request"] = method->input_type_name(); (*vars)["Response"] = method->output_type_name(); @@ -723,7 +716,7 @@ void PrintHeaderClientMethodCallback(grpc_generator::Printer* printer, void PrintHeaderClientMethodCallbackEnd( grpc_generator::Printer* printer, - std::map* vars) { + std::map* /*vars*/) { printer->Outdent(); printer->Print(" private:\n"); printer->Indent(); @@ -1372,7 +1365,7 @@ void PrintHeaderService(grpc_generator::Printer* printer, for (int i = 0; i < service->method_count(); ++i) { printer->Print(service->method(i)->GetLeadingComments("//").c_str()); PrintHeaderClientMethodCallbackInterfaces(printer, service->method(i).get(), - vars, true); + vars); printer->Print(service->method(i)->GetTrailingComments("//").c_str()); } PrintHeaderClientMethodCallbackInterfacesEnd(printer, vars); @@ -1397,8 +1390,7 @@ void PrintHeaderService(grpc_generator::Printer* printer, } PrintHeaderClientMethodCallbackStart(printer, vars); for (int i = 0; i < service->method_count(); ++i) { - PrintHeaderClientMethodCallback(printer, service->method(i).get(), vars, - true); + PrintHeaderClientMethodCallback(printer, service->method(i).get(), vars); } PrintHeaderClientMethodCallbackEnd(printer, vars); printer->Outdent(); diff --git a/src/compiler/generator_helpers.h b/src/compiler/generator_helpers.h index 747096f0657..2b6b2bacb01 100644 --- a/src/compiler/generator_helpers.h +++ b/src/compiler/generator_helpers.h @@ -166,7 +166,7 @@ inline MethodType GetMethodType( } } -inline void Split(const grpc::string& s, char delim, +inline void Split(const grpc::string& s, char /*delim*/, std::vector* append_to) { std::istringstream iss(s); grpc::string piece; diff --git a/src/compiler/ruby_plugin.cc b/src/compiler/ruby_plugin.cc index 9c234bd7cbc..a1d2e818567 100644 --- a/src/compiler/ruby_plugin.cc +++ b/src/compiler/ruby_plugin.cc @@ -30,9 +30,9 @@ class RubyGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { ~RubyGrpcGenerator() {} bool Generate(const grpc::protobuf::FileDescriptor* file, - const grpc::string& parameter, + const grpc::string& /*parameter*/, grpc::protobuf::compiler::GeneratorContext* context, - grpc::string* error) const { + grpc::string* /*error*/) const { grpc::string code = grpc_ruby_generator::GetServices(file); if (code.size() == 0) { return true; // don't generate a file if there are no services diff --git a/src/core/ext/filters/client_channel/backup_poller.cc b/src/core/ext/filters/client_channel/backup_poller.cc index 2e2d8d607de..6ebb4085511 100644 --- a/src/core/ext/filters/client_channel/backup_poller.cc +++ b/src/core/ext/filters/client_channel/backup_poller.cc @@ -89,7 +89,7 @@ static void backup_poller_shutdown_unref(backup_poller* p) { } } -static void done_poller(void* arg, grpc_error* error) { +static void done_poller(void* arg, grpc_error* /*error*/) { backup_poller_shutdown_unref(static_cast(arg)); } diff --git a/src/core/ext/filters/client_channel/channel_connectivity.cc b/src/core/ext/filters/client_channel/channel_connectivity.cc index 58170e66f96..4ebb976efe5 100644 --- a/src/core/ext/filters/client_channel/channel_connectivity.cc +++ b/src/core/ext/filters/client_channel/channel_connectivity.cc @@ -90,7 +90,7 @@ static void delete_state_watcher(state_watcher* w) { gpr_free(w); } -static void finished_completion(void* pw, grpc_cq_completion* ignored) { +static void finished_completion(void* pw, grpc_cq_completion* /*ignored*/) { bool should_delete = false; state_watcher* w = static_cast(pw); gpr_mu_lock(&w->mu); @@ -198,7 +198,7 @@ typedef struct watcher_timer_init_arg { gpr_timespec deadline; } watcher_timer_init_arg; -static void watcher_timer_init(void* arg, grpc_error* error_ignored) { +static void watcher_timer_init(void* arg, grpc_error* /*error_ignored*/) { watcher_timer_init_arg* wa = static_cast(arg); grpc_timer_init(&wa->w->alarm, grpc_timespec_to_millis_round_up(wa->deadline), diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 033816a0b43..b94d7c7efcc 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -584,10 +584,10 @@ class CallData { // A predicate type and some useful implementations for PendingBatchesFail(). typedef bool (*YieldCallCombinerPredicate)( const CallCombinerClosureList& closures); - static bool YieldCallCombiner(const CallCombinerClosureList& closures) { + static bool YieldCallCombiner(const CallCombinerClosureList& /*closures*/) { return true; } - static bool NoYieldCallCombiner(const CallCombinerClosureList& closures) { + static bool NoYieldCallCombiner(const CallCombinerClosureList& /*closures*/) { return false; } static bool YieldCallCombinerIfPendingBatchesFound( @@ -1052,7 +1052,7 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface { private: static void ApplyUpdateInControlPlaneCombiner(void* arg, - grpc_error* error) { + grpc_error* /*error*/) { Updater* self = static_cast(arg); if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_routing_trace)) { gpr_log(GPR_INFO, @@ -1188,7 +1188,7 @@ void ChannelData::ExternalConnectivityWatcher::Cancel() { } void ChannelData::ExternalConnectivityWatcher::AddWatcherLocked( - void* arg, grpc_error* ignored) { + void* arg, grpc_error* /*ignored*/) { ExternalConnectivityWatcher* self = static_cast(arg); // This assumes that the closure is scheduled on the ExecCtx scheduler @@ -1201,7 +1201,7 @@ void ChannelData::ExternalConnectivityWatcher::AddWatcherLocked( } void ChannelData::ExternalConnectivityWatcher::RemoveWatcherLocked( - void* arg, grpc_error* ignored) { + void* arg, grpc_error* /*ignored*/) { ExternalConnectivityWatcher* self = static_cast(arg); self->chand_->state_tracker_.RemoveWatcher(self); @@ -1228,7 +1228,7 @@ class ChannelData::ConnectivityWatcherAdder { } private: - static void AddWatcherLocked(void* arg, grpc_error* error) { + static void AddWatcherLocked(void* arg, grpc_error* /*error*/) { ConnectivityWatcherAdder* self = static_cast(arg); self->chand_->state_tracker_.AddWatcher(self->initial_state_, @@ -1262,7 +1262,7 @@ class ChannelData::ConnectivityWatcherRemover { } private: - static void RemoveWatcherLocked(void* arg, grpc_error* error) { + static void RemoveWatcherLocked(void* arg, grpc_error* /*error*/) { ConnectivityWatcherRemover* self = static_cast(arg); self->chand_->state_tracker_.RemoveWatcher(self->watcher_); @@ -1809,7 +1809,7 @@ grpc_error* ChannelData::DoPingLocked(grpc_transport_op* op) { return result.error; } -void ChannelData::StartTransportOpLocked(void* arg, grpc_error* ignored) { +void ChannelData::StartTransportOpLocked(void* arg, grpc_error* /*ignored*/) { grpc_transport_op* op = static_cast(arg); grpc_channel_element* elem = static_cast(op->handler_private.extra_arg); @@ -1937,7 +1937,7 @@ ChannelData::GetConnectedSubchannelInDataPlane( return connected_subchannel->Ref(); } -void ChannelData::TryToConnectLocked(void* arg, grpc_error* error_ignored) { +void ChannelData::TryToConnectLocked(void* arg, grpc_error* /*error_ignored*/) { auto* chand = static_cast(arg); if (chand->resolving_lb_policy_ != nullptr) { chand->resolving_lb_policy_->ExitIdleLocked(); @@ -2050,7 +2050,7 @@ grpc_error* CallData::Init(grpc_call_element* elem, } void CallData::Destroy(grpc_call_element* elem, - const grpc_call_final_info* final_info, + const grpc_call_final_info* /*final_info*/, grpc_closure* then_schedule_closure) { CallData* calld = static_cast(elem->call_data); if (GPR_LIKELY(calld->subchannel_call_ != nullptr)) { @@ -2445,7 +2445,7 @@ void CallData::PendingBatchesFail( // This is called via the call combiner, so access to calld is synchronized. void CallData::ResumePendingBatchInCallCombiner(void* arg, - grpc_error* ignored) { + grpc_error* /*ignored*/) { grpc_transport_stream_op_batch* batch = static_cast(arg); SubchannelCall* subchannel_call = @@ -2908,7 +2908,7 @@ void CallData::RecvMessageReady(void* arg, grpc_error* error) { // recv_trailing_metadata handling // -void CallData::GetCallStatus(grpc_call_element* elem, +void CallData::GetCallStatus(grpc_call_element* /*elem*/, grpc_metadata_batch* md_batch, grpc_error* error, grpc_status_code* status, grpc_mdelem** server_pushback_md) { @@ -3111,7 +3111,7 @@ void CallData::RecvTrailingMetadataReady(void* arg, grpc_error* error) { void CallData::AddClosuresForCompletedPendingBatch( grpc_call_element* elem, SubchannelCallBatchData* batch_data, - SubchannelCallRetryState* retry_state, grpc_error* error, + SubchannelCallRetryState* /*retry_state*/, grpc_error* error, CallCombinerClosureList* closures) { PendingBatch* pending = PendingBatchFind( elem, "completed", [batch_data](grpc_transport_stream_op_batch* batch) { @@ -3238,7 +3238,7 @@ void CallData::OnComplete(void* arg, grpc_error* error) { // subchannel batch construction // -void CallData::StartBatchInCallCombiner(void* arg, grpc_error* ignored) { +void CallData::StartBatchInCallCombiner(void* arg, grpc_error* /*ignored*/) { grpc_transport_stream_op_batch* batch = static_cast(arg); SubchannelCall* subchannel_call = @@ -3608,7 +3608,8 @@ void CallData::AddSubchannelBatchesForPendingBatches( } } -void CallData::StartRetriableSubchannelBatches(void* arg, grpc_error* ignored) { +void CallData::StartRetriableSubchannelBatches(void* arg, + grpc_error* /*ignored*/) { grpc_call_element* elem = static_cast(arg); ChannelData* chand = static_cast(elem->channel_data); CallData* calld = static_cast(elem->call_data); diff --git a/src/core/ext/filters/client_channel/client_channel_factory.cc b/src/core/ext/filters/client_channel/client_channel_factory.cc index 671a38430ef..c0d853ebe5f 100644 --- a/src/core/ext/filters/client_channel/client_channel_factory.cc +++ b/src/core/ext/filters/client_channel/client_channel_factory.cc @@ -29,7 +29,7 @@ namespace grpc_core { namespace { void* factory_arg_copy(void* f) { return f; } -void factory_arg_destroy(void* f) {} +void factory_arg_destroy(void* /*f*/) {} int factory_arg_cmp(void* factory1, void* factory2) { return GPR_ICMP(factory1, factory2); } diff --git a/src/core/ext/filters/client_channel/global_subchannel_pool.cc b/src/core/ext/filters/client_channel/global_subchannel_pool.cc index 96a0244eb24..b5e1feb149a 100644 --- a/src/core/ext/filters/client_channel/global_subchannel_pool.cc +++ b/src/core/ext/filters/client_channel/global_subchannel_pool.cc @@ -140,28 +140,28 @@ RefCountedPtr* GlobalSubchannelPool::instance_ = nullptr; namespace { -void sck_avl_destroy(void* p, void* user_data) { +void sck_avl_destroy(void* p, void* /*user_data*/) { SubchannelKey* key = static_cast(p); Delete(key); } -void* sck_avl_copy(void* p, void* unused) { +void* sck_avl_copy(void* p, void* /*unused*/) { const SubchannelKey* key = static_cast(p); auto* new_key = New(*key); return static_cast(new_key); } -long sck_avl_compare(void* a, void* b, void* unused) { +long sck_avl_compare(void* a, void* b, void* /*unused*/) { const SubchannelKey* key_a = static_cast(a); const SubchannelKey* key_b = static_cast(b); return key_a->Cmp(*key_b); } -void scv_avl_destroy(void* p, void* user_data) { +void scv_avl_destroy(void* p, void* /*user_data*/) { GRPC_SUBCHANNEL_WEAK_UNREF((Subchannel*)p, "global_subchannel_pool"); } -void* scv_avl_copy(void* p, void* unused) { +void* scv_avl_copy(void* p, void* /*unused*/) { GRPC_SUBCHANNEL_WEAK_REF((Subchannel*)p, "global_subchannel_pool"); return p; } diff --git a/src/core/ext/filters/client_channel/health/health_check_client.cc b/src/core/ext/filters/client_channel/health/health_check_client.cc index 6091184ec94..b5bca9887c9 100644 --- a/src/core/ext/filters/client_channel/health/health_check_client.cc +++ b/src/core/ext/filters/client_channel/health/health_check_client.cc @@ -377,8 +377,8 @@ void HealthCheckClient::CallState::StartCall() { StartBatch(&recv_trailing_metadata_batch_); } -void HealthCheckClient::CallState::StartBatchInCallCombiner(void* arg, - grpc_error* error) { +void HealthCheckClient::CallState::StartBatchInCallCombiner( + void* arg, grpc_error* /*error*/) { grpc_transport_stream_op_batch* batch = static_cast(arg); SubchannelCall* call = @@ -396,21 +396,22 @@ void HealthCheckClient::CallState::StartBatch( } void HealthCheckClient::CallState::AfterCallStackDestruction( - void* arg, grpc_error* error) { + void* arg, grpc_error* /*error*/) { HealthCheckClient::CallState* self = static_cast(arg); Delete(self); } void HealthCheckClient::CallState::OnCancelComplete(void* arg, - grpc_error* error) { + grpc_error* /*error*/) { HealthCheckClient::CallState* self = static_cast(arg); GRPC_CALL_COMBINER_STOP(&self->call_combiner_, "health_cancel"); self->call_->Unref(DEBUG_LOCATION, "cancel"); } -void HealthCheckClient::CallState::StartCancel(void* arg, grpc_error* error) { +void HealthCheckClient::CallState::StartCancel(void* arg, + grpc_error* /*error*/) { HealthCheckClient::CallState* self = static_cast(arg); auto* batch = grpc_make_transport_stream_op( @@ -432,7 +433,8 @@ void HealthCheckClient::CallState::Cancel() { } } -void HealthCheckClient::CallState::OnComplete(void* arg, grpc_error* error) { +void HealthCheckClient::CallState::OnComplete(void* arg, + grpc_error* /*error*/) { HealthCheckClient::CallState* self = static_cast(arg); GRPC_CALL_COMBINER_STOP(&self->call_combiner_, "on_complete"); @@ -441,8 +443,8 @@ void HealthCheckClient::CallState::OnComplete(void* arg, grpc_error* error) { self->call_->Unref(DEBUG_LOCATION, "on_complete"); } -void HealthCheckClient::CallState::RecvInitialMetadataReady(void* arg, - grpc_error* error) { +void HealthCheckClient::CallState::RecvInitialMetadataReady( + void* arg, grpc_error* /*error*/) { HealthCheckClient::CallState* self = static_cast(arg); GRPC_CALL_COMBINER_STOP(&self->call_combiner_, "recv_initial_metadata_ready"); @@ -524,7 +526,7 @@ void HealthCheckClient::CallState::OnByteStreamNext(void* arg, } void HealthCheckClient::CallState::RecvMessageReady(void* arg, - grpc_error* error) { + grpc_error* /*error*/) { HealthCheckClient::CallState* self = static_cast(arg); GRPC_CALL_COMBINER_STOP(&self->call_combiner_, "recv_message_ready"); @@ -583,7 +585,7 @@ void HealthCheckClient::CallState::RecvTrailingMetadataReady( } void HealthCheckClient::CallState::CallEndedRetry(void* arg, - grpc_error* error) { + grpc_error* /*error*/) { HealthCheckClient::CallState* self = static_cast(arg); self->CallEnded(true /* retry */); diff --git a/src/core/ext/filters/client_channel/http_connect_handshaker.cc b/src/core/ext/filters/client_channel/http_connect_handshaker.cc index 660f6b85c00..64ed7746761 100644 --- a/src/core/ext/filters/client_channel/http_connect_handshaker.cc +++ b/src/core/ext/filters/client_channel/http_connect_handshaker.cc @@ -246,7 +246,7 @@ void HttpConnectHandshaker::Shutdown(grpc_error* why) { GRPC_ERROR_UNREF(why); } -void HttpConnectHandshaker::DoHandshake(grpc_tcp_server_acceptor* acceptor, +void HttpConnectHandshaker::DoHandshake(grpc_tcp_server_acceptor* /*acceptor*/, grpc_closure* on_handshake_done, HandshakerArgs* args) { // Check for HTTP CONNECT channel arg. @@ -340,8 +340,8 @@ HttpConnectHandshaker::HttpConnectHandshaker() { class HttpConnectHandshakerFactory : public HandshakerFactory { public: - void AddHandshakers(const grpc_channel_args* args, - grpc_pollset_set* interested_parties, + void AddHandshakers(const grpc_channel_args* /*args*/, + grpc_pollset_set* /*interested_parties*/, HandshakeManager* handshake_mgr) override { handshake_mgr->Add(MakeRefCounted()); } From 32760aca00e9bd36a86500135375e53c4b714cc5 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 21 Oct 2019 09:16:33 -0700 Subject: [PATCH 02/30] Remove unused-parameter warnings, round 2 (12 of 19) --- src/core/lib/surface/channel_ping.cc | 2 +- src/core/lib/surface/completion_queue.cc | 23 +++++++++++-------- .../lib/surface/completion_queue_factory.cc | 2 +- src/core/lib/surface/init.cc | 2 +- src/core/lib/surface/init_secure.cc | 4 ++-- src/core/lib/surface/lame_client.cc | 8 +++---- src/core/lib/surface/server.cc | 19 +++++++-------- src/core/lib/transport/byte_stream.cc | 4 ++-- src/core/lib/transport/connectivity_state.cc | 2 +- src/core/lib/transport/metadata_batch.cc | 6 +++++ 10 files changed, 41 insertions(+), 31 deletions(-) diff --git a/src/core/lib/surface/channel_ping.cc b/src/core/lib/surface/channel_ping.cc index bae945942f2..be02a051df4 100644 --- a/src/core/lib/surface/channel_ping.cc +++ b/src/core/lib/surface/channel_ping.cc @@ -35,7 +35,7 @@ typedef struct { grpc_cq_completion completion_storage; } ping_result; -static void ping_destroy(void* arg, grpc_cq_completion* storage) { +static void ping_destroy(void* arg, grpc_cq_completion* /*storage*/) { gpr_free(arg); } diff --git a/src/core/lib/surface/completion_queue.cc b/src/core/lib/surface/completion_queue.cc index bb249331e12..c836fcf660e 100644 --- a/src/core/lib/surface/completion_queue.cc +++ b/src/core/lib/surface/completion_queue.cc @@ -529,7 +529,8 @@ grpc_completion_queue* grpc_completion_queue_create_internal( } static void cq_init_next( - void* data, grpc_experimental_completion_queue_functor* shutdown_callback) { + void* data, + grpc_experimental_completion_queue_functor* /*shutdown_callback*/) { new (data) cq_next_data(); } @@ -539,7 +540,8 @@ static void cq_destroy_next(void* data) { } static void cq_init_pluck( - void* data, grpc_experimental_completion_queue_functor* shutdown_callback) { + void* data, + grpc_experimental_completion_queue_functor* /*shutdown_callback*/) { new (data) cq_pluck_data(); } @@ -582,7 +584,7 @@ void grpc_cq_internal_ref(grpc_completion_queue* cq) { cq->owning_refs.Ref(debug_location, reason); } -static void on_pollset_shutdown_done(void* arg, grpc_error* error) { +static void on_pollset_shutdown_done(void* arg, grpc_error* /*error*/) { grpc_completion_queue* cq = static_cast(arg); GRPC_CQ_INTERNAL_UNREF(cq, "pollset_destroy"); } @@ -630,20 +632,21 @@ static void cq_check_tag(grpc_completion_queue* cq, void* tag, bool lock_cq) { GPR_ASSERT(found); } #else -static void cq_check_tag(grpc_completion_queue* cq, void* tag, bool lock_cq) {} +static void cq_check_tag(grpc_completion_queue* /*cq*/, void* /*tag*/, + bool /*lock_cq*/) {} #endif -static bool cq_begin_op_for_next(grpc_completion_queue* cq, void* tag) { +static bool cq_begin_op_for_next(grpc_completion_queue* cq, void* /*tag*/) { cq_next_data* cqd = static_cast DATA_FROM_CQ(cq); return cqd->pending_events.IncrementIfNonzero(); } -static bool cq_begin_op_for_pluck(grpc_completion_queue* cq, void* tag) { +static bool cq_begin_op_for_pluck(grpc_completion_queue* cq, void* /*tag*/) { cq_pluck_data* cqd = static_cast DATA_FROM_CQ(cq); return cqd->pending_events.IncrementIfNonzero(); } -static bool cq_begin_op_for_callback(grpc_completion_queue* cq, void* tag) { +static bool cq_begin_op_for_callback(grpc_completion_queue* cq, void* /*tag*/) { cq_callback_data* cqd = static_cast DATA_FROM_CQ(cq); return cqd->pending_events.IncrementIfNonzero(); } @@ -669,7 +672,7 @@ bool grpc_cq_begin_op(grpc_completion_queue* cq, void* tag) { static void cq_end_op_for_next( grpc_completion_queue* cq, void* tag, grpc_error* error, void (*done)(void* done_arg, grpc_cq_completion* storage), void* done_arg, - grpc_cq_completion* storage, bool internal) { + grpc_cq_completion* storage, bool /*internal*/) { GPR_TIMER_SCOPE("cq_end_op_for_next", 0); if (GRPC_TRACE_FLAG_ENABLED(grpc_api_trace) || @@ -748,7 +751,7 @@ static void cq_end_op_for_next( static void cq_end_op_for_pluck( grpc_completion_queue* cq, void* tag, grpc_error* error, void (*done)(void* done_arg, grpc_cq_completion* storage), void* done_arg, - grpc_cq_completion* storage, bool internal) { + grpc_cq_completion* storage, bool /*internal*/) { GPR_TIMER_SCOPE("cq_end_op_for_pluck", 0); cq_pluck_data* cqd = static_cast DATA_FROM_CQ(cq); @@ -939,7 +942,7 @@ static void dump_pending_tags(grpc_completion_queue* cq) { gpr_free(out); } #else -static void dump_pending_tags(grpc_completion_queue* cq) {} +static void dump_pending_tags(grpc_completion_queue* /*cq*/) {} #endif static grpc_event cq_next(grpc_completion_queue* cq, gpr_timespec deadline, diff --git a/src/core/lib/surface/completion_queue_factory.cc b/src/core/lib/surface/completion_queue_factory.cc index 2616c156e45..72aa03adfd4 100644 --- a/src/core/lib/surface/completion_queue_factory.cc +++ b/src/core/lib/surface/completion_queue_factory.cc @@ -28,7 +28,7 @@ */ static grpc_completion_queue* default_create( - const grpc_completion_queue_factory* factory, + const grpc_completion_queue_factory* /*factory*/, const grpc_completion_queue_attributes* attr) { return grpc_completion_queue_create_internal( attr->cq_completion_type, attr->cq_polling_type, attr->cq_shutdown_cb); diff --git a/src/core/lib/surface/init.cc b/src/core/lib/surface/init.cc index 2812427f7a0..d8bc4e4dd32 100644 --- a/src/core/lib/surface/init.cc +++ b/src/core/lib/surface/init.cc @@ -197,7 +197,7 @@ void grpc_shutdown_internal_locked(void) { grpc_destroy_static_metadata_ctx(); } -void grpc_shutdown_internal(void* ignored) { +void grpc_shutdown_internal(void* /*ignored*/) { GRPC_API_TRACE("grpc_shutdown_internal", 0, ()); grpc_core::MutexLock lock(&g_init_mu); // We have released lock from the shutdown thread and it is possible that diff --git a/src/core/lib/surface/init_secure.cc b/src/core/lib/surface/init_secure.cc index 233089437e9..f2c236dd79e 100644 --- a/src/core/lib/surface/init_secure.cc +++ b/src/core/lib/surface/init_secure.cc @@ -37,7 +37,7 @@ void grpc_security_pre_init(void) {} static bool maybe_prepend_client_auth_filter( - grpc_channel_stack_builder* builder, void* arg) { + grpc_channel_stack_builder* builder, void* /*arg*/) { const grpc_channel_args* args = grpc_channel_stack_builder_get_channel_arguments(builder); if (args) { @@ -52,7 +52,7 @@ static bool maybe_prepend_client_auth_filter( } static bool maybe_prepend_server_auth_filter( - grpc_channel_stack_builder* builder, void* arg) { + grpc_channel_stack_builder* builder, void* /*arg*/) { const grpc_channel_args* args = grpc_channel_stack_builder_get_channel_arguments(builder); if (args) { diff --git a/src/core/lib/surface/lame_client.cc b/src/core/lib/surface/lame_client.cc index 565f386ba74..e2a0cfd2dd3 100644 --- a/src/core/lib/surface/lame_client.cc +++ b/src/core/lib/surface/lame_client.cc @@ -94,8 +94,8 @@ static void lame_start_transport_stream_op_batch( calld->call_combiner); } -static void lame_get_channel_info(grpc_channel_element* elem, - const grpc_channel_info* channel_info) {} +static void lame_get_channel_info(grpc_channel_element* /*elem*/, + const grpc_channel_info* /*channel_info*/) {} static void lame_start_transport_op(grpc_channel_element* elem, grpc_transport_op* op) { @@ -133,8 +133,8 @@ static grpc_error* lame_init_call_elem(grpc_call_element* elem, return GRPC_ERROR_NONE; } -static void lame_destroy_call_elem(grpc_call_element* elem, - const grpc_call_final_info* final_info, +static void lame_destroy_call_elem(grpc_call_element* /*elem*/, + const grpc_call_final_info* /*final_info*/, grpc_closure* then_schedule_closure) { GRPC_CLOSURE_SCHED(then_schedule_closure, GRPC_ERROR_NONE); } diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index 16186fa0377..eed32a9a23d 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -302,7 +302,7 @@ struct shutdown_cleanup_args { grpc_slice slice; }; -static void shutdown_cleanup(void* arg, grpc_error* error) { +static void shutdown_cleanup(void* arg, grpc_error* /*error*/) { struct shutdown_cleanup_args* a = static_cast(arg); grpc_slice_unref_internal(a->slice); @@ -367,7 +367,7 @@ static void request_matcher_destroy(request_matcher* rm) { gpr_free(rm->requests_per_cq); } -static void kill_zombie(void* elem, grpc_error* error) { +static void kill_zombie(void* elem, grpc_error* /*error*/) { grpc_call_unref( grpc_call_from_top_element(static_cast(elem))); } @@ -449,7 +449,7 @@ static void orphan_channel(channel_data* chand) { chand->next = chand->prev = chand; } -static void finish_destroy_channel(void* cd, grpc_error* error) { +static void finish_destroy_channel(void* cd, grpc_error* /*error*/) { channel_data* chand = static_cast(cd); grpc_server* server = chand->server; GRPC_CHANNEL_INTERNAL_UNREF(chand->channel, "server"); @@ -477,7 +477,7 @@ static void destroy_channel(channel_data* chand) { op); } -static void done_request_event(void* req, grpc_cq_completion* c) { +static void done_request_event(void* req, grpc_cq_completion* /*c*/) { gpr_free(req); } @@ -672,7 +672,8 @@ static int num_listeners(grpc_server* server) { return n; } -static void done_shutdown_event(void* server, grpc_cq_completion* completion) { +static void done_shutdown_event(void* server, + grpc_cq_completion* /*completion*/) { server_unref(static_cast(server)); } @@ -850,7 +851,7 @@ static void got_initial_metadata(void* ptr, grpc_error* error) { } } -static void accept_stream(void* cd, grpc_transport* transport, +static void accept_stream(void* cd, grpc_transport* /*transport*/, const void* transport_server_data) { channel_data* chand = static_cast(cd); /* create a call */ @@ -895,8 +896,8 @@ static grpc_error* server_init_call_elem(grpc_call_element* elem, } static void server_destroy_call_elem(grpc_call_element* elem, - const grpc_call_final_info* final_info, - grpc_closure* ignored) { + const grpc_call_final_info* /*final_info*/, + grpc_closure* /*ignored*/) { call_data* calld = static_cast(elem->call_data); calld->~call_data(); channel_data* chand = static_cast(elem->channel_data); @@ -1258,7 +1259,7 @@ void done_published_shutdown(void* done_arg, grpc_cq_completion* storage) { gpr_free(storage); } -static void listener_destroy_done(void* s, grpc_error* error) { +static void listener_destroy_done(void* s, grpc_error* /*error*/) { grpc_server* server = static_cast(s); gpr_mu_lock(&server->mu_global); server->listeners_destroyed++; diff --git a/src/core/lib/transport/byte_stream.cc b/src/core/lib/transport/byte_stream.cc index 1dd234c3761..82f34ca1504 100644 --- a/src/core/lib/transport/byte_stream.cc +++ b/src/core/lib/transport/byte_stream.cc @@ -53,8 +53,8 @@ void SliceBufferByteStream::Orphan() { // filter stack. } -bool SliceBufferByteStream::Next(size_t max_size_hint, - grpc_closure* on_complete) { +bool SliceBufferByteStream::Next(size_t /*max_size_hint*/, + grpc_closure* /*on_complete*/) { GPR_DEBUG_ASSERT(backing_buffer_.count > 0); return true; } diff --git a/src/core/lib/transport/connectivity_state.cc b/src/core/lib/transport/connectivity_state.cc index c553ba240ca..b51fdfdb13d 100644 --- a/src/core/lib/transport/connectivity_state.cc +++ b/src/core/lib/transport/connectivity_state.cc @@ -72,7 +72,7 @@ class AsyncConnectivityStateWatcherInterface::Notifier { } private: - static void SendNotification(void* arg, grpc_error* ignored) { + static void SendNotification(void* arg, grpc_error* /*ignored*/) { Notifier* self = static_cast(arg); if (GRPC_TRACE_FLAG_ENABLED(grpc_connectivity_state_trace)) { gpr_log(GPR_INFO, "watcher %p: delivering async notification for %s", diff --git a/src/core/lib/transport/metadata_batch.cc b/src/core/lib/transport/metadata_batch.cc index 560342cecc6..66749b671c7 100644 --- a/src/core/lib/transport/metadata_batch.cc +++ b/src/core/lib/transport/metadata_batch.cc @@ -50,6 +50,9 @@ static void assert_valid_list(grpc_mdelem_list* list) { verified_count++; } GPR_ASSERT(list->count == verified_count); +#else + // Avoid unused-parameter warning for debug-only parameter + (void)list; #endif /* NDEBUG */ } @@ -64,6 +67,9 @@ static void assert_valid_callouts(grpc_metadata_batch* batch) { } grpc_slice_unref_internal(key_interned); } +#else + // Avoid unused-parameter warning for debug-only parameter + (void)batch; #endif } From d469403b6e82c3a8101975985bf8aa6c5ce6ac89 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 21 Oct 2019 09:16:34 -0700 Subject: [PATCH 03/30] Remove unused-parameter warnings, round 2 (16 of 19) --- test/core/end2end/tests/max_concurrent_streams.cc | 2 +- test/core/end2end/tests/max_connection_idle.cc | 2 +- test/core/end2end/tests/negative_deadline.cc | 2 +- test/core/end2end/tests/payload.cc | 2 +- test/core/end2end/tests/proxy_auth.cc | 2 +- test/core/end2end/tests/registered_call.cc | 2 +- test/core/end2end/tests/server_finishes_request.cc | 2 +- test/core/end2end/tests/simple_delayed_request.cc | 2 +- test/core/gpr/alloc_test.cc | 2 +- test/core/iomgr/buffer_list_test.cc | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/test/core/end2end/tests/max_concurrent_streams.cc b/test/core/end2end/tests/max_concurrent_streams.cc index 86a8d224435..b3cc7af02ba 100644 --- a/test/core/end2end/tests/max_concurrent_streams.cc +++ b/test/core/end2end/tests/max_concurrent_streams.cc @@ -83,7 +83,7 @@ static void end_test(grpc_end2end_test_fixture* f) { grpc_completion_queue_destroy(f->shutdown_cq); } -static void simple_request_body(grpc_end2end_test_config config, +static void simple_request_body(grpc_end2end_test_config /*config*/, grpc_end2end_test_fixture f) { grpc_call* c; grpc_call* s; diff --git a/test/core/end2end/tests/max_connection_idle.cc b/test/core/end2end/tests/max_connection_idle.cc index faa1383207d..87cbeff049d 100644 --- a/test/core/end2end/tests/max_connection_idle.cc +++ b/test/core/end2end/tests/max_connection_idle.cc @@ -42,7 +42,7 @@ static void drain_cq(grpc_completion_queue* cq) { } while (ev.type != GRPC_QUEUE_SHUTDOWN); } -static void simple_request_body(grpc_end2end_test_config config, +static void simple_request_body(grpc_end2end_test_config /*config*/, grpc_end2end_test_fixture* f) { grpc_call* c; grpc_call* s; diff --git a/test/core/end2end/tests/negative_deadline.cc b/test/core/end2end/tests/negative_deadline.cc index 2b2ff126c4a..464267c92f1 100644 --- a/test/core/end2end/tests/negative_deadline.cc +++ b/test/core/end2end/tests/negative_deadline.cc @@ -86,7 +86,7 @@ static void end_test(grpc_end2end_test_fixture* f) { grpc_completion_queue_destroy(f->shutdown_cq); } -static void simple_request_body(grpc_end2end_test_config config, +static void simple_request_body(grpc_end2end_test_config /*config*/, grpc_end2end_test_fixture f, size_t num_ops) { grpc_call* c; cq_verifier* cqv = cq_verifier_create(f.cq); diff --git a/test/core/end2end/tests/payload.cc b/test/core/end2end/tests/payload.cc index cb6eb47de93..6e2bc1690b6 100644 --- a/test/core/end2end/tests/payload.cc +++ b/test/core/end2end/tests/payload.cc @@ -100,7 +100,7 @@ static grpc_slice generate_random_slice() { return out; } -static void request_response_with_payload(grpc_end2end_test_config config, +static void request_response_with_payload(grpc_end2end_test_config /*config*/, grpc_end2end_test_fixture f) { /* Create large request and response bodies. These are big enough to require * multiple round trips to deliver to the peer, and their exact contents of diff --git a/test/core/end2end/tests/proxy_auth.cc b/test/core/end2end/tests/proxy_auth.cc index 3b12869d2f0..3e9d7a3388d 100644 --- a/test/core/end2end/tests/proxy_auth.cc +++ b/test/core/end2end/tests/proxy_auth.cc @@ -90,7 +90,7 @@ static void end_test(grpc_end2end_test_fixture* f) { grpc_completion_queue_destroy(f->shutdown_cq); } -static void simple_request_body(grpc_end2end_test_config config, +static void simple_request_body(grpc_end2end_test_config /*config*/, grpc_end2end_test_fixture f) { grpc_call* c; grpc_call* s; diff --git a/test/core/end2end/tests/registered_call.cc b/test/core/end2end/tests/registered_call.cc index 3e05fd1421f..badd802335b 100644 --- a/test/core/end2end/tests/registered_call.cc +++ b/test/core/end2end/tests/registered_call.cc @@ -86,7 +86,7 @@ static void end_test(grpc_end2end_test_fixture* f) { grpc_completion_queue_destroy(f->shutdown_cq); } -static void simple_request_body(grpc_end2end_test_config config, +static void simple_request_body(grpc_end2end_test_config /*config*/, grpc_end2end_test_fixture f, void* rc) { grpc_call* c; grpc_call* s; diff --git a/test/core/end2end/tests/server_finishes_request.cc b/test/core/end2end/tests/server_finishes_request.cc index c81b309a1e6..9ee23c91ea0 100644 --- a/test/core/end2end/tests/server_finishes_request.cc +++ b/test/core/end2end/tests/server_finishes_request.cc @@ -85,7 +85,7 @@ static void end_test(grpc_end2end_test_fixture* f) { grpc_completion_queue_destroy(f->shutdown_cq); } -static void simple_request_body(grpc_end2end_test_config config, +static void simple_request_body(grpc_end2end_test_config /*config*/, grpc_end2end_test_fixture f) { grpc_call* c; grpc_call* s; diff --git a/test/core/end2end/tests/simple_delayed_request.cc b/test/core/end2end/tests/simple_delayed_request.cc index 8d116999459..dc4fdbd6e81 100644 --- a/test/core/end2end/tests/simple_delayed_request.cc +++ b/test/core/end2end/tests/simple_delayed_request.cc @@ -75,7 +75,7 @@ static void simple_delayed_request_body(grpc_end2end_test_config config, grpc_end2end_test_fixture* f, grpc_channel_args* client_args, grpc_channel_args* server_args, - long delay_us) { + long /*delay_us*/) { grpc_call* c; grpc_call* s; cq_verifier* cqv = cq_verifier_create(f->cq); diff --git a/test/core/gpr/alloc_test.cc b/test/core/gpr/alloc_test.cc index 0b110e2b77d..55fe6d95a8f 100644 --- a/test/core/gpr/alloc_test.cc +++ b/test/core/gpr/alloc_test.cc @@ -25,7 +25,7 @@ static void* fake_malloc(size_t size) { return (void*)size; } -static void* fake_realloc(void* addr, size_t size) { return (void*)size; } +static void* fake_realloc(void* /*addr*/, size_t size) { return (void*)size; } static void fake_free(void* addr) { *(static_cast(addr)) = static_cast(0xdeadd00d); diff --git a/test/core/iomgr/buffer_list_test.cc b/test/core/iomgr/buffer_list_test.cc index 7602067528d..2c179c4a047 100644 --- a/test/core/iomgr/buffer_list_test.cc +++ b/test/core/iomgr/buffer_list_test.cc @@ -131,6 +131,6 @@ int main(int argc, char** argv) { #else /* GRPC_LINUX_ERRQUEUE */ -int main(int argc, char** argv) { return 0; } +int main(int /*argc*/, char** /*argv*/) { return 0; } #endif /* GRPC_LINUX_ERRQUEUE */ From 98f33e8e17392e5d403d94eb55250e2846381f81 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 15:05:03 -0700 Subject: [PATCH 04/30] Enable all AsyncIO tests for Bazel --- src/python/grpcio_tests/commands.py | 2 + .../grpcio_tests/tests_aio/unit/BUILD.bazel | 19 +++- .../unit/{sync_server.py => _test_server.py} | 29 ++--- .../tests_aio/unit/channel_test.py | 24 +++-- .../grpcio_tests/tests_aio/unit/init_test.py | 12 ++- .../grpcio_tests/tests_aio/unit/test_base.py | 101 ------------------ 6 files changed, 51 insertions(+), 136 deletions(-) rename src/python/grpcio_tests/tests_aio/unit/{sync_server.py => _test_server.py} (60%) delete mode 100644 src/python/grpcio_tests/tests_aio/unit/test_base.py diff --git a/src/python/grpcio_tests/commands.py b/src/python/grpcio_tests/commands.py index 2912ba113c9..f7c4f08dd55 100644 --- a/src/python/grpcio_tests/commands.py +++ b/src/python/grpcio_tests/commands.py @@ -120,6 +120,8 @@ class TestAio(setuptools.Command): pass def run(self): + from grpc.experimental import aio + aio.init_grpc_aio() self._add_eggs_to_path() import tests diff --git a/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel b/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel index c6d0a9f7728..f635dc534be 100644 --- a/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel +++ b/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel @@ -17,9 +17,20 @@ package( default_visibility = ["//visibility:public"], ) -GRPC_ASYNC_TESTS = [ - "server_test.py", -] +GRPC_ASYNC_TESTS = glob(["*_test.py"]) + + +py_library( + name = "_test_server", + srcs_version = "PY3", + srcs = ["_test_server.py"], + deps = [ + "//src/python/grpcio/grpc:grpcio", + "//src/proto/grpc/testing:py_messages_proto", + "//src/proto/grpc/testing:test_py_pb2_grpc", + "//src/proto/grpc/testing:empty_py_pb2", + ] +) [ py_test( @@ -29,10 +40,12 @@ GRPC_ASYNC_TESTS = [ main=test_file_name, python_version="PY3", deps=[ + ":_test_server", "//src/python/grpcio/grpc:grpcio", "//src/proto/grpc/testing:py_messages_proto", "//src/proto/grpc/testing:benchmark_service_py_pb2_grpc", "//src/proto/grpc/testing:benchmark_service_py_pb2", + "//src/python/grpcio_tests/tests/unit/framework/common", "//external:six" ], imports=["../../",], diff --git a/src/python/grpcio_tests/tests_aio/unit/sync_server.py b/src/python/grpcio_tests/tests_aio/unit/_test_server.py similarity index 60% rename from src/python/grpcio_tests/tests_aio/unit/sync_server.py rename to src/python/grpcio_tests/tests_aio/unit/_test_server.py index 82def8ed4c0..1afe3082ac1 100644 --- a/src/python/grpcio_tests/tests_aio/unit/sync_server.py +++ b/src/python/grpcio_tests/tests_aio/unit/_test_server.py @@ -18,38 +18,27 @@ from concurrent import futures from time import sleep import grpc +from grpc.experimental import aio from src.proto.grpc.testing import messages_pb2 from src.proto.grpc.testing import test_pb2_grpc from tests.unit.framework.common import test_constants -# TODO (https://github.com/grpc/grpc/issues/19762) -# Change for an asynchronous server version once it's implemented. class TestServiceServicer(test_pb2_grpc.TestServiceServicer): - def UnaryCall(self, request, context): + async def UnaryCall(self, request, context): return messages_pb2.SimpleResponse() - def EmptyCall(self, request, context): + async def EmptyCall(self, request, context): while True: sleep(test_constants.LONG_TIMEOUT) -if __name__ == "__main__": - parser = argparse.ArgumentParser(description='Synchronous gRPC server.') - parser.add_argument( - '--host_and_port', - required=True, - type=str, - nargs=1, - help='the host and port to listen.') - args = parser.parse_args() - - server = grpc.server( - futures.ThreadPoolExecutor(max_workers=1), - options=(('grpc.so_reuseport', 1),)) +async def start_test_server(): + server = aio.server(options=(('grpc.so_reuseport', 0),)) test_pb2_grpc.add_TestServiceServicer_to_server(TestServiceServicer(), server) - server.add_insecure_port(args.host_and_port[0]) - server.start() - server.wait_for_termination() + port = server.add_insecure_port('[::]:0') + await server.start() + # NOTE(lidizheng) returning the server to prevent it from deallocation + return 'localhost:%d' % port, server diff --git a/src/python/grpcio_tests/tests_aio/unit/channel_test.py b/src/python/grpcio_tests/tests_aio/unit/channel_test.py index cdf7a4cd49a..3bacf3c01dd 100644 --- a/src/python/grpcio_tests/tests_aio/unit/channel_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/channel_test.py @@ -12,23 +12,26 @@ # See the License for the specific language governing permissions and # limitations under the License. +import asyncio import logging import unittest import grpc from grpc.experimental import aio -from tests_aio.unit import test_base from src.proto.grpc.testing import messages_pb2 from tests.unit.framework.common import test_constants +from tests_aio.unit._test_server import start_test_server -class TestChannel(test_base.AioTestBase): +class TestChannel(unittest.TestCase): def test_async_context(self): async def coro(): - async with aio.insecure_channel(self.server_target) as channel: + server_target, unused_server = await start_test_server() + + async with aio.insecure_channel(server_target) as channel: hi = channel.unary_unary( '/grpc.testing.TestService/UnaryCall', request_serializer=messages_pb2.SimpleRequest. @@ -37,12 +40,14 @@ class TestChannel(test_base.AioTestBase): ) await hi(messages_pb2.SimpleRequest()) - self.loop.run_until_complete(coro()) + asyncio.get_event_loop().run_until_complete(coro()) def test_unary_unary(self): async def coro(): - channel = aio.insecure_channel(self.server_target) + server_target, unused_server = await start_test_server() + + channel = aio.insecure_channel(server_target) hi = channel.unary_unary( '/grpc.testing.TestService/UnaryCall', request_serializer=messages_pb2.SimpleRequest.SerializeToString, @@ -53,12 +58,14 @@ class TestChannel(test_base.AioTestBase): await channel.close() - self.loop.run_until_complete(coro()) + asyncio.get_event_loop().run_until_complete(coro()) def test_unary_call_times_out(self): async def coro(): - async with aio.insecure_channel(self.server_target) as channel: + server_target, unused_server = await start_test_server() + + async with aio.insecure_channel(server_target) as channel: empty_call_with_sleep = channel.unary_unary( "/grpc.testing.TestService/EmptyCall", request_serializer=messages_pb2.SimpleRequest. @@ -83,9 +90,10 @@ class TestChannel(test_base.AioTestBase): self.assertIsNotNone( exception_context.exception.trailing_metadata()) - self.loop.run_until_complete(coro()) + asyncio.get_event_loop().run_until_complete(coro()) if __name__ == '__main__': + aio.init_grpc_aio() logging.basicConfig() unittest.main(verbosity=2) diff --git a/src/python/grpcio_tests/tests_aio/unit/init_test.py b/src/python/grpcio_tests/tests_aio/unit/init_test.py index 8371d44574e..d575cfeb780 100644 --- a/src/python/grpcio_tests/tests_aio/unit/init_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/init_test.py @@ -12,12 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. +import asyncio import logging import unittest import grpc from grpc.experimental import aio -from tests_aio.unit import test_base +from tests_aio.unit._test_server import start_test_server class TestAioRpcError(unittest.TestCase): @@ -59,17 +60,20 @@ class TestAioRpcError(unittest.TestCase): second_aio_rpc_error.__class__) -class TestInsecureChannel(test_base.AioTestBase): +class TestInsecureChannel(unittest.TestCase): def test_insecure_channel(self): async def coro(): - channel = aio.insecure_channel(self.server_target) + server_target, unused_server = await start_test_server() + + channel = aio.insecure_channel(server_target) self.assertIsInstance(channel, aio.Channel) - self.loop.run_until_complete(coro()) + asyncio.get_event_loop().run_until_complete(coro()) if __name__ == '__main__': + aio.init_grpc_aio() logging.basicConfig() unittest.main(verbosity=2) diff --git a/src/python/grpcio_tests/tests_aio/unit/test_base.py b/src/python/grpcio_tests/tests_aio/unit/test_base.py deleted file mode 100644 index 0b325523e0f..00000000000 --- a/src/python/grpcio_tests/tests_aio/unit/test_base.py +++ /dev/null @@ -1,101 +0,0 @@ -# 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. - -import os -import sys -import subprocess - -import asyncio -import unittest -import socket - -from grpc.experimental import aio -from tests_aio.unit import sync_server - - -def _get_free_loopback_tcp_port(): - if socket.has_ipv6: - tcp_socket = socket.socket(socket.AF_INET6) - host = "::1" - host_target = "[::1]" - else: - tcp_socket = socket.socket(socket.AF_INET) - host = "127.0.0.1" - host_target = host - tcp_socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1) - tcp_socket.bind((host, 0)) - address_tuple = tcp_socket.getsockname() - return tcp_socket, "%s:%s" % (host_target, address_tuple[1]) - - -class _Server: - """_Server is an wrapper for a sync-server subprocess. - - The synchronous server is executed in another process which initializes - implicitly the grpc using the synchronous configuration. Both worlds - can not coexist within the same process. - """ - - def __init__(self, host_and_port): # pylint: disable=W0621 - self._host_and_port = host_and_port - self._handle = None - - def start(self): - assert self._handle is None - - try: - from google3.pyglib import resources - executable = resources.GetResourceFilename( - "google3/third_party/py/grpc/sync_server") - args = [executable, '--host_and_port', self._host_and_port] - except ImportError: - executable = sys.executable - directory, _ = os.path.split(os.path.abspath(__file__)) - filename = directory + '/sync_server.py' - args = [ - executable, filename, '--host_and_port', self._host_and_port - ] - - self._handle = subprocess.Popen(args) - - def terminate(self): - if not self._handle: - return - - self._handle.terminate() - self._handle.wait() - self._handle = None - - -class AioTestBase(unittest.TestCase): - - def setUp(self): - self._socket, self._target = _get_free_loopback_tcp_port() - self._server = _Server(self._target) - self._server.start() - self._loop = asyncio.new_event_loop() - asyncio.set_event_loop(self._loop) - aio.init_grpc_aio() - - def tearDown(self): - self._server.terminate() - self._socket.close() - - @property - def loop(self): - return self._loop - - @property - def server_target(self): - return self._target From b231e7f88f6b8ff2d7673b70277cb97c6d8f5f95 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 15:11:53 -0700 Subject: [PATCH 05/30] Add three more Kokoro job for Linux, macOS and Windows --- .../grpc_basictests_python_aio.cfg | 30 ++++++++++++++++++ .../grpc_basictests_python_aio.cfg | 31 +++++++++++++++++++ .../grpc_basictests_python_aio.cfg | 30 ++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 tools/internal_ci/linux/pull_request/grpc_basictests_python_aio.cfg create mode 100644 tools/internal_ci/macos/pull_request/grpc_basictests_python_aio.cfg create mode 100644 tools/internal_ci/windows/pull_request/grpc_basictests_python_aio.cfg diff --git a/tools/internal_ci/linux/pull_request/grpc_basictests_python_aio.cfg b/tools/internal_ci/linux/pull_request/grpc_basictests_python_aio.cfg new file mode 100644 index 00000000000..cfbc853c95f --- /dev/null +++ b/tools/internal_ci/linux/pull_request/grpc_basictests_python_aio.cfg @@ -0,0 +1,30 @@ +# 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. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/linux/grpc_run_tests_matrix.sh" +timeout_mins: 60 +action { + define_artifacts { + regex: "**/*sponge_log.*" + regex: "github/grpc/reports/**" + } +} + +env_vars { + key: "RUN_TESTS_FLAGS" + value: "-f basictests linux python-aio --inner_jobs 16 -j 2 --internal_ci --max_time=3600" +} diff --git a/tools/internal_ci/macos/pull_request/grpc_basictests_python_aio.cfg b/tools/internal_ci/macos/pull_request/grpc_basictests_python_aio.cfg new file mode 100644 index 00000000000..7d4d4ab3880 --- /dev/null +++ b/tools/internal_ci/macos/pull_request/grpc_basictests_python_aio.cfg @@ -0,0 +1,31 @@ +# 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. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/macos/grpc_run_tests_matrix.sh" +gfile_resources: "/bigstore/grpc-testing-secrets/gcp_credentials/GrpcTesting-d0eeee2db331.json" +timeout_mins: 60 +action { + define_artifacts { + regex: "**/*sponge_log.*" + regex: "github/grpc/reports/**" + } +} + +env_vars { + key: "RUN_TESTS_FLAGS" + value: "-f basictests macos python-aio --internal_ci -j 1 --inner_jobs 4 --max_time=3600" +} diff --git a/tools/internal_ci/windows/pull_request/grpc_basictests_python_aio.cfg b/tools/internal_ci/windows/pull_request/grpc_basictests_python_aio.cfg new file mode 100644 index 00000000000..75649635ffc --- /dev/null +++ b/tools/internal_ci/windows/pull_request/grpc_basictests_python_aio.cfg @@ -0,0 +1,30 @@ +# 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. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/windows/grpc_run_tests_matrix.bat" +timeout_mins: 60 +action { + define_artifacts { + regex: "**/*sponge_log.*" + regex: "github/grpc/reports/**" + } +} + +env_vars { + key: "RUN_TESTS_FLAGS" + value: "-f basictests windows python-aio -j 1 --inner_jobs 8 --internal_ci --max_time=3600" +} From 58cdbe7a8315a86e689cf7fa4c8bf74011f50255 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 15:15:41 -0700 Subject: [PATCH 06/30] Add the server_test to tests.json --- src/python/grpcio_tests/tests_aio/tests.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/python/grpcio_tests/tests_aio/tests.json b/src/python/grpcio_tests/tests_aio/tests.json index 23439717b18..0fab86e49bc 100644 --- a/src/python/grpcio_tests/tests_aio/tests.json +++ b/src/python/grpcio_tests/tests_aio/tests.json @@ -2,5 +2,6 @@ "_sanity._sanity_test.AioSanityTest", "unit.channel_test.TestChannel", "unit.init_test.TestAioRpcError", - "unit.init_test.TestInsecureChannel" + "unit.init_test.TestInsecureChannel", + "unit.server_test.TestServer" ] From e2dcb2883d602f373901960ce2f5b1c1ec8e84ff Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 15:24:30 -0700 Subject: [PATCH 07/30] Make runner support single threaded mode --- src/python/grpcio_tests/commands.py | 2 +- src/python/grpcio_tests/tests/_runner.py | 49 +++++++++++++++--------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/python/grpcio_tests/commands.py b/src/python/grpcio_tests/commands.py index f7c4f08dd55..7e39200d3d8 100644 --- a/src/python/grpcio_tests/commands.py +++ b/src/python/grpcio_tests/commands.py @@ -127,7 +127,7 @@ class TestAio(setuptools.Command): import tests loader = tests.Loader() loader.loadTestsFromNames(['tests_aio']) - runner = tests.Runner() + runner = tests.Runner(dedicated_threads=False) result = runner.run(loader.suite) if not result.wasSuccessful(): sys.exit('Test failure') diff --git a/src/python/grpcio_tests/tests/_runner.py b/src/python/grpcio_tests/tests/_runner.py index 9ef0f176840..0b2186fc7a3 100644 --- a/src/python/grpcio_tests/tests/_runner.py +++ b/src/python/grpcio_tests/tests/_runner.py @@ -117,8 +117,15 @@ class AugmentedCase(collections.namedtuple('AugmentedCase', ['case', 'id'])): class Runner(object): - def __init__(self): + def __init__(self, dedicated_threads=True): + """Constructs the Runner object. + + Args: + dedicated_threads: A bool indicates whether to spawn each unit test + in separate thread or not. + """ self._skipped_tests = [] + self._dedicated_threads = dedicated_threads def skip_tests(self, tests): self._skipped_tests = tests @@ -194,24 +201,27 @@ class Runner(object): sys.stdout.write('Running {}\n'.format( augmented_case.case.id())) sys.stdout.flush() - case_thread = threading.Thread( - target=augmented_case.case.run, args=(result,)) - try: - with stdout_pipe, stderr_pipe: - case_thread.start() - while case_thread.is_alive(): - check_kill_self() - time.sleep(0) - case_thread.join() - except: # pylint: disable=try-except-raise - # re-raise the exception after forcing the with-block to end - raise - result.set_output(augmented_case.case, stdout_pipe.output(), - stderr_pipe.output()) - sys.stdout.write(result_out.getvalue()) - sys.stdout.flush() - result_out.truncate(0) - check_kill_self() + if self._dedicated_threads: + case_thread = threading.Thread( + target=augmented_case.case.run, args=(result,)) + try: + with stdout_pipe, stderr_pipe: + case_thread.start() + while case_thread.is_alive(): + check_kill_self() + time.sleep(0) + case_thread.join() + except: # pylint: disable=try-except-raise + # re-raise the exception after forcing the with-block to end + raise + result.set_output(augmented_case.case, stdout_pipe.output(), + stderr_pipe.output()) + sys.stdout.write(result_out.getvalue()) + sys.stdout.flush() + result_out.truncate(0) + check_kill_self() + else: + augmented_case.case.run(result) result.stopTestRun() stdout_pipe.close() stderr_pipe.close() @@ -223,3 +233,4 @@ class Runner(object): with open('report.xml', 'wb') as report_xml_file: _result.jenkins_junit_xml(result).write(report_xml_file) return result + From 0765cf940be6aed77f2fa22c4ec9caee0f20d59c Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 15:44:54 -0700 Subject: [PATCH 08/30] Refactor the way AsyncIO is tested under run_tests.py --- src/python/grpcio_tests/tests/_runner.py | 3 +- .../grpc_basictests_python_aio.cfg | 30 ------------------ .../grpc_basictests_python_aio.cfg | 31 ------------------- .../grpc_basictests_python_aio.cfg | 30 ------------------ tools/run_tests/run_tests.py | 25 ++++++++++++--- tools/run_tests/run_tests_matrix.py | 2 +- 6 files changed, 22 insertions(+), 99 deletions(-) delete mode 100644 tools/internal_ci/linux/pull_request/grpc_basictests_python_aio.cfg delete mode 100644 tools/internal_ci/macos/pull_request/grpc_basictests_python_aio.cfg delete mode 100644 tools/internal_ci/windows/pull_request/grpc_basictests_python_aio.cfg diff --git a/src/python/grpcio_tests/tests/_runner.py b/src/python/grpcio_tests/tests/_runner.py index 0b2186fc7a3..361e99e5f8b 100644 --- a/src/python/grpcio_tests/tests/_runner.py +++ b/src/python/grpcio_tests/tests/_runner.py @@ -215,7 +215,7 @@ class Runner(object): # re-raise the exception after forcing the with-block to end raise result.set_output(augmented_case.case, stdout_pipe.output(), - stderr_pipe.output()) + stderr_pipe.output()) sys.stdout.write(result_out.getvalue()) sys.stdout.flush() result_out.truncate(0) @@ -233,4 +233,3 @@ class Runner(object): with open('report.xml', 'wb') as report_xml_file: _result.jenkins_junit_xml(result).write(report_xml_file) return result - diff --git a/tools/internal_ci/linux/pull_request/grpc_basictests_python_aio.cfg b/tools/internal_ci/linux/pull_request/grpc_basictests_python_aio.cfg deleted file mode 100644 index cfbc853c95f..00000000000 --- a/tools/internal_ci/linux/pull_request/grpc_basictests_python_aio.cfg +++ /dev/null @@ -1,30 +0,0 @@ -# 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. - -# Config file for the internal CI (in protobuf text format) - -# Location of the continuous shell script in repository. -build_file: "grpc/tools/internal_ci/linux/grpc_run_tests_matrix.sh" -timeout_mins: 60 -action { - define_artifacts { - regex: "**/*sponge_log.*" - regex: "github/grpc/reports/**" - } -} - -env_vars { - key: "RUN_TESTS_FLAGS" - value: "-f basictests linux python-aio --inner_jobs 16 -j 2 --internal_ci --max_time=3600" -} diff --git a/tools/internal_ci/macos/pull_request/grpc_basictests_python_aio.cfg b/tools/internal_ci/macos/pull_request/grpc_basictests_python_aio.cfg deleted file mode 100644 index 7d4d4ab3880..00000000000 --- a/tools/internal_ci/macos/pull_request/grpc_basictests_python_aio.cfg +++ /dev/null @@ -1,31 +0,0 @@ -# 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. - -# Config file for the internal CI (in protobuf text format) - -# Location of the continuous shell script in repository. -build_file: "grpc/tools/internal_ci/macos/grpc_run_tests_matrix.sh" -gfile_resources: "/bigstore/grpc-testing-secrets/gcp_credentials/GrpcTesting-d0eeee2db331.json" -timeout_mins: 60 -action { - define_artifacts { - regex: "**/*sponge_log.*" - regex: "github/grpc/reports/**" - } -} - -env_vars { - key: "RUN_TESTS_FLAGS" - value: "-f basictests macos python-aio --internal_ci -j 1 --inner_jobs 4 --max_time=3600" -} diff --git a/tools/internal_ci/windows/pull_request/grpc_basictests_python_aio.cfg b/tools/internal_ci/windows/pull_request/grpc_basictests_python_aio.cfg deleted file mode 100644 index 75649635ffc..00000000000 --- a/tools/internal_ci/windows/pull_request/grpc_basictests_python_aio.cfg +++ /dev/null @@ -1,30 +0,0 @@ -# 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. - -# Config file for the internal CI (in protobuf text format) - -# Location of the continuous shell script in repository. -build_file: "grpc/tools/internal_ci/windows/grpc_run_tests_matrix.bat" -timeout_mins: 60 -action { - define_artifacts { - regex: "**/*sponge_log.*" - regex: "github/grpc/reports/**" - } -} - -env_vars { - key: "RUN_TESTS_FLAGS" - value: "-f basictests windows python-aio -j 1 --inner_jobs 8 --internal_ci --max_time=3600" -} diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 7d2d56530af..a697cb74afa 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -703,9 +703,16 @@ class PythonConfig( class PythonLanguage(object): - _DEFAULT_COMMAND = 'test_lite' - _TEST_SPECS_FILE = 'src/python/grpcio_tests/tests/tests.json' - _TEST_FOLDER = 'test' + _TEST_SPECS_FILE = { + 'native': 'src/python/grpcio_tests/tests/tests.json', + 'gevent': 'src/python/grpcio_tests/tests/tests.json', + 'asyncio': 'src/python/grpcio_tests/tests_aio/tests.json', + } + _TEST_FOLDER = { + 'native': 'test', + 'gevent': 'test', + 'asyncio': 'test_aio', + } def configure(self, config, args): self.config = config @@ -793,9 +800,17 @@ class PythonLanguage(object): venv_relative_python = ['bin/python'] toolchain = ['unix'] - test_command = self._DEFAULT_COMMAND - if args.iomgr_platform == 'gevent': + # Selects the corresponding testing mode. + # See src/python/grpcio_tests/commands.py for implementation details. + if args.iomgr_platform == 'native': + test_command = 'test_lite' + elif args.iomgr_platform == 'gevent': test_command = 'test_gevent' + elif args.iomgr_platform == 'asyncio': + test_command = 'test_aio' + else: + raise ValueError( + 'Unsupported IO Manager platform: %s' % args.iomgr_platform) runner = [ os.path.abspath('tools/run_tests/helper_scripts/run_python.sh') ] diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index 644ae04905c..70f628025a9 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -231,7 +231,7 @@ def _create_test_jobs(extra_args=[], inner_jobs=_DEFAULT_INNER_JOBS): languages=['python'], configs=['opt'], platforms=['linux', 'macos', 'windows'], - iomgr_platforms=['native', 'gevent'], + iomgr_platforms=['native', 'gevent', 'asyncio'], labels=['basictests', 'multilang'], extra_args=extra_args + ['--report_multi_target'], inner_jobs=inner_jobs) From 8261106c4ba6b63c04c44d1de9bd57439c9103a7 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 16:13:18 -0700 Subject: [PATCH 09/30] Fix misused variable in run_tests.py --- tools/run_tests/run_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index a697cb74afa..3d3005e7b66 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -721,7 +721,7 @@ class PythonLanguage(object): def test_specs(self): # load list of known test suites - with open(self._TEST_SPECS_FILE) as tests_json_file: + with open(self._TEST_SPECS_FILE[self.args.iomgr_platform]) as tests_json_file: tests_json = json.load(tests_json_file) environment = dict(_FORCE_ENVIRON_FOR_WRAPPERS) return [ @@ -731,7 +731,7 @@ class PythonLanguage(object): environ=dict( list(environment.items()) + [( 'GRPC_PYTHON_TESTRUNNER_FILTER', str(suite_name))]), - shortname='%s.%s.%s' % (config.name, self._TEST_FOLDER, + shortname='%s.%s.%s' % (config.name, self._TEST_FOLDER[self.args.iomgr_platform], suite_name), ) for suite_name in tests_json for config in self.pythons ] @@ -1502,7 +1502,7 @@ argp.add_argument( ) argp.add_argument( '--iomgr_platform', - choices=['native', 'uv', 'gevent'], + choices=['native', 'uv', 'gevent', 'asyncio'], default='native', help='Selects iomgr platform to build on') argp.add_argument( From 5c965fa5a0a4bcca15589c047fe6cd4dfa4badd4 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 16:14:04 -0700 Subject: [PATCH 10/30] Remove PythonAioLanguage from run_tests.py --- tools/run_tests/run_tests.py | 33 +++++++-------------------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 3d3005e7b66..8e16c85c965 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -815,6 +815,13 @@ class PythonLanguage(object): os.path.abspath('tools/run_tests/helper_scripts/run_python.sh') ] + if args.iomgr_platform == 'asyncio': + if args.compiler not in ('python3.6', 'python3.7', 'python3.8'): + raise Exception( + 'Compiler %s not supported with IO Manager platform:' % ( + args.compiler, + args.iomgr_platform)) + config_vars = _PythonConfigVars( shell, builder, builder_prefix_arguments, venv_relative_python, toolchain, runner, test_command, args.iomgr_platform) @@ -902,31 +909,6 @@ class PythonLanguage(object): return 'python' -class PythonAioLanguage(PythonLanguage): - - _DEFAULT_COMMAND = 'test_aio' - _TEST_SPECS_FILE = 'src/python/grpcio_tests/tests_aio/tests.json' - _TEST_FOLDER = 'test_aio' - - def configure(self, config, args): - self.config = config - self.args = args - self.pythons = self._get_pythons(self.args) - - def _get_pythons(self, args): - """Get python runtimes to test with, based on current platform, architecture, compiler etc.""" - - if args.compiler not in ('python3.6', 'python3.7', 'python3.8'): - raise Exception('Compiler %s not supported.' % args.compiler) - if args.iomgr_platform not in ('native'): - raise Exception( - 'Iomgr platform %s not supported.' % args.iomgr_platform) - return super()._get_pythons(args) - - def __str__(self): - return 'python_aio' - - class RubyLanguage(object): def configure(self, config, args): @@ -1334,7 +1316,6 @@ _LANGUAGES = { 'php': PhpLanguage(), 'php7': Php7Language(), 'python': PythonLanguage(), - 'python-aio': PythonAioLanguage(), 'ruby': RubyLanguage(), 'csharp': CSharpLanguage(), 'objc': ObjCLanguage(), From 81499d3c1ba1ae0b089fd478bebd02a9c4dd829a Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 16:22:12 -0700 Subject: [PATCH 11/30] Make YAPF happy for run_tests.py --- tools/run_tests/run_tests.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 8e16c85c965..ab85efa1d79 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -182,9 +182,9 @@ def get_c_tests(travis, test_lang): with open('tools/run_tests/generated/tests.json') as f: js = json.load(f) return [ - tgt for tgt in js - if tgt['language'] == test_lang and platform_string() in - tgt[platforms_str] and not (travis and tgt['flaky']) + tgt for tgt in js if tgt['language'] == test_lang and + platform_string() in tgt[platforms_str] and + not (travis and tgt['flaky']) ] @@ -721,7 +721,8 @@ class PythonLanguage(object): def test_specs(self): # load list of known test suites - with open(self._TEST_SPECS_FILE[self.args.iomgr_platform]) as tests_json_file: + with open(self._TEST_SPECS_FILE[ + self.args.iomgr_platform]) as tests_json_file: tests_json = json.load(tests_json_file) environment = dict(_FORCE_ENVIRON_FOR_WRAPPERS) return [ @@ -731,8 +732,9 @@ class PythonLanguage(object): environ=dict( list(environment.items()) + [( 'GRPC_PYTHON_TESTRUNNER_FILTER', str(suite_name))]), - shortname='%s.%s.%s' % (config.name, self._TEST_FOLDER[self.args.iomgr_platform], - suite_name), + shortname='%s.%s.%s' % + (config.name, self._TEST_FOLDER[self.args.iomgr_platform], + suite_name), ) for suite_name in tests_json for config in self.pythons ] @@ -818,9 +820,8 @@ class PythonLanguage(object): if args.iomgr_platform == 'asyncio': if args.compiler not in ('python3.6', 'python3.7', 'python3.8'): raise Exception( - 'Compiler %s not supported with IO Manager platform:' % ( - args.compiler, - args.iomgr_platform)) + 'Compiler %s not supported with IO Manager platform:' % + (args.compiler, args.iomgr_platform)) config_vars = _PythonConfigVars( shell, builder, builder_prefix_arguments, venv_relative_python, @@ -1636,8 +1637,7 @@ if any(language.make_options() for language in languages): # together, and is only used under gcov. All other configs should build languages individually. language_make_options = list( set([ - make_option - for lang in languages + make_option for lang in languages for make_option in lang.make_options() ])) From e75376ae609878e29c77335c510262964cec1196 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 16:23:56 -0700 Subject: [PATCH 12/30] Make YAPF happy again --- tools/run_tests/run_tests.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index ab85efa1d79..1d2c1b56033 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -182,9 +182,9 @@ def get_c_tests(travis, test_lang): with open('tools/run_tests/generated/tests.json') as f: js = json.load(f) return [ - tgt for tgt in js if tgt['language'] == test_lang and - platform_string() in tgt[platforms_str] and - not (travis and tgt['flaky']) + tgt for tgt in js + if tgt['language'] == test_lang and platform_string() in + tgt[platforms_str] and not (travis and tgt['flaky']) ] @@ -1637,7 +1637,8 @@ if any(language.make_options() for language in languages): # together, and is only used under gcov. All other configs should build languages individually. language_make_options = list( set([ - make_option for lang in languages + make_option + for lang in languages for make_option in lang.make_options() ])) From 14fff13f489004eebb5fdf1ad6570b20dcf86293 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 17:27:25 -0700 Subject: [PATCH 13/30] Test AsyncIO only on 3.6 --- tools/run_tests/run_tests.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 1d2c1b56033..d9c96963a77 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -817,12 +817,6 @@ class PythonLanguage(object): os.path.abspath('tools/run_tests/helper_scripts/run_python.sh') ] - if args.iomgr_platform == 'asyncio': - if args.compiler not in ('python3.6', 'python3.7', 'python3.8'): - raise Exception( - 'Compiler %s not supported with IO Manager platform:' % - (args.compiler, args.iomgr_platform)) - config_vars = _PythonConfigVars( shell, builder, builder_prefix_arguments, venv_relative_python, toolchain, runner, test_command, args.iomgr_platform) @@ -867,15 +861,25 @@ class PythonLanguage(object): pypy32_config = _pypy_config_generator( name='pypy3', major='3', config_vars=config_vars) + if args.iomgr_platform == 'asyncio': + if args.compiler not in ('python3.6', 'python3.7', 'python3.8'): + raise Exception( + 'Compiler %s not supported with IO Manager platform: %s' % + (args.compiler, args.iomgr_platform)) + + if args.compiler == 'default': if os.name == 'nt': return (python35_config,) else: - return ( - python27_config, - python36_config, - python37_config, - ) + if args.iomgr_platform == 'asyncio': + return (python36_config,) + else: + return ( + python27_config, + python36_config, + python37_config, + ) elif args.compiler == 'python2.7': return (python27_config,) elif args.compiler == 'python3.4': From e7e121fdbd154752e944ab98126ae621120b3720 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 17:28:36 -0700 Subject: [PATCH 14/30] Allow compiler 'default' for AsyncIO test --- tools/run_tests/run_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index d9c96963a77..61215a92336 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -862,7 +862,7 @@ class PythonLanguage(object): name='pypy3', major='3', config_vars=config_vars) if args.iomgr_platform == 'asyncio': - if args.compiler not in ('python3.6', 'python3.7', 'python3.8'): + if args.compiler not in ('default', 'python3.6', 'python3.7', 'python3.8'): raise Exception( 'Compiler %s not supported with IO Manager platform: %s' % (args.compiler, args.iomgr_platform)) From dcf609966e3629e540aab928351dd215f4e44bb4 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 18:28:46 -0700 Subject: [PATCH 15/30] Add comments about why runner failed --- src/python/grpcio_tests/commands.py | 4 +++- src/python/grpcio_tests/tests/_runner.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/python/grpcio_tests/commands.py b/src/python/grpcio_tests/commands.py index 7e39200d3d8..436ca4d3c7a 100644 --- a/src/python/grpcio_tests/commands.py +++ b/src/python/grpcio_tests/commands.py @@ -20,7 +20,6 @@ import os.path import platform import re import shutil -import subprocess import sys import setuptools @@ -127,6 +126,9 @@ class TestAio(setuptools.Command): import tests loader = tests.Loader() loader.loadTestsFromNames(['tests_aio']) + # Even without dedicated threads, the framework will somehow spawn a + # new thread for tests to run upon. New thread doesn't have event loop + # attached by default, so initialization is needed. runner = tests.Runner(dedicated_threads=False) result = runner.run(loader.suite) if not result.wasSuccessful(): diff --git a/src/python/grpcio_tests/tests/_runner.py b/src/python/grpcio_tests/tests/_runner.py index 361e99e5f8b..6411ffa8981 100644 --- a/src/python/grpcio_tests/tests/_runner.py +++ b/src/python/grpcio_tests/tests/_runner.py @@ -119,7 +119,7 @@ class Runner(object): def __init__(self, dedicated_threads=True): """Constructs the Runner object. - + Args: dedicated_threads: A bool indicates whether to spawn each unit test in separate thread or not. From 70821ebe4ab23573c4eade8a8b6af95dc124469e Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 18:49:58 -0700 Subject: [PATCH 16/30] Supply the event loop to aio iomgr in test case level --- src/python/grpcio_tests/commands.py | 2 -- src/python/grpcio_tests/tests_aio/unit/BUILD.bazel | 7 +++++++ src/python/grpcio_tests/tests_aio/unit/channel_test.py | 9 +++++---- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/python/grpcio_tests/commands.py b/src/python/grpcio_tests/commands.py index 436ca4d3c7a..301a2bea23b 100644 --- a/src/python/grpcio_tests/commands.py +++ b/src/python/grpcio_tests/commands.py @@ -119,8 +119,6 @@ class TestAio(setuptools.Command): pass def run(self): - from grpc.experimental import aio - aio.init_grpc_aio() self._add_eggs_to_path() import tests diff --git a/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel b/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel index f635dc534be..82a12f9b7e4 100644 --- a/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel +++ b/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel @@ -20,6 +20,12 @@ package( GRPC_ASYNC_TESTS = glob(["*_test.py"]) +py_library( + name = "_test_base", + srcs_version = "PY3", + srcs = ["_test_base.py"], +) + py_library( name = "_test_server", srcs_version = "PY3", @@ -41,6 +47,7 @@ py_library( python_version="PY3", deps=[ ":_test_server", + ":_test_base", "//src/python/grpcio/grpc:grpcio", "//src/proto/grpc/testing:py_messages_proto", "//src/proto/grpc/testing:benchmark_service_py_pb2_grpc", diff --git a/src/python/grpcio_tests/tests_aio/unit/channel_test.py b/src/python/grpcio_tests/tests_aio/unit/channel_test.py index 3bacf3c01dd..4f95fcfb850 100644 --- a/src/python/grpcio_tests/tests_aio/unit/channel_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/channel_test.py @@ -22,9 +22,10 @@ from grpc.experimental import aio from src.proto.grpc.testing import messages_pb2 from tests.unit.framework.common import test_constants from tests_aio.unit._test_server import start_test_server +from tests_aio.unit._test_base import AioTestBase -class TestChannel(unittest.TestCase): +class TestChannel(AioTestBase): def test_async_context(self): @@ -40,7 +41,7 @@ class TestChannel(unittest.TestCase): ) await hi(messages_pb2.SimpleRequest()) - asyncio.get_event_loop().run_until_complete(coro()) + self._loop.run_until_complete(coro()) def test_unary_unary(self): @@ -58,7 +59,7 @@ class TestChannel(unittest.TestCase): await channel.close() - asyncio.get_event_loop().run_until_complete(coro()) + self._loop.run_until_complete(coro()) def test_unary_call_times_out(self): @@ -90,7 +91,7 @@ class TestChannel(unittest.TestCase): self.assertIsNotNone( exception_context.exception.trailing_metadata()) - asyncio.get_event_loop().run_until_complete(coro()) + self._loop.run_until_complete(coro()) if __name__ == '__main__': From 57a0173e5c62aae894909183a3fcedee2ef666a7 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 18:52:45 -0700 Subject: [PATCH 17/30] Make YAPF and PyLint happy --- tools/run_tests/run_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 61215a92336..8cf00dc5546 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -862,12 +862,12 @@ class PythonLanguage(object): name='pypy3', major='3', config_vars=config_vars) if args.iomgr_platform == 'asyncio': - if args.compiler not in ('default', 'python3.6', 'python3.7', 'python3.8'): + if args.compiler not in ('default', 'python3.6', 'python3.7', + 'python3.8'): raise Exception( 'Compiler %s not supported with IO Manager platform: %s' % (args.compiler, args.iomgr_platform)) - if args.compiler == 'default': if os.name == 'nt': return (python35_config,) From 1ebaace7f1cf18965c42ddd5ee3e915a47a3f9b4 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 19:02:26 -0700 Subject: [PATCH 18/30] Bring back the file that I accidentally rm -f --- .../grpcio_tests/tests_aio/unit/_test_base.py | 29 +++++++++++++++++++ .../grpcio_tests/tests_aio/unit/init_test.py | 5 ++-- .../tests_aio/unit/server_test.py | 6 ++-- 3 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 src/python/grpcio_tests/tests_aio/unit/_test_base.py diff --git a/src/python/grpcio_tests/tests_aio/unit/_test_base.py b/src/python/grpcio_tests/tests_aio/unit/_test_base.py new file mode 100644 index 00000000000..09b0ef7abf8 --- /dev/null +++ b/src/python/grpcio_tests/tests_aio/unit/_test_base.py @@ -0,0 +1,29 @@ +# 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. + +import asyncio +import unittest +from grpc.experimental import aio + +class AioTestBase(unittest.TestCase): + + def setUp(self): + self._loop = asyncio.new_event_loop() + asyncio.set_event_loop(self._loop) + aio.init_grpc_aio() + + @property + def loop(self): + return self._loop + diff --git a/src/python/grpcio_tests/tests_aio/unit/init_test.py b/src/python/grpcio_tests/tests_aio/unit/init_test.py index d575cfeb780..b2a07633028 100644 --- a/src/python/grpcio_tests/tests_aio/unit/init_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/init_test.py @@ -19,6 +19,7 @@ import unittest import grpc from grpc.experimental import aio from tests_aio.unit._test_server import start_test_server +from tests_aio.unit._test_base import AioTestBase class TestAioRpcError(unittest.TestCase): @@ -60,7 +61,7 @@ class TestAioRpcError(unittest.TestCase): second_aio_rpc_error.__class__) -class TestInsecureChannel(unittest.TestCase): +class TestInsecureChannel(AioTestBase): def test_insecure_channel(self): @@ -70,7 +71,7 @@ class TestInsecureChannel(unittest.TestCase): channel = aio.insecure_channel(server_target) self.assertIsInstance(channel, aio.Channel) - asyncio.get_event_loop().run_until_complete(coro()) + self.loop.run_until_complete(coro()) if __name__ == '__main__': diff --git a/src/python/grpcio_tests/tests_aio/unit/server_test.py b/src/python/grpcio_tests/tests_aio/unit/server_test.py index 2d543893176..ba1c53472e3 100644 --- a/src/python/grpcio_tests/tests_aio/unit/server_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/server_test.py @@ -20,6 +20,7 @@ import grpc from grpc.experimental import aio from src.proto.grpc.testing import messages_pb2 from src.proto.grpc.testing import benchmark_service_pb2_grpc +from tests_aio.unit._test_base import AioTestBase _TEST_METHOD_PATH = '' @@ -37,10 +38,9 @@ class GenericHandler(grpc.GenericRpcHandler): return grpc.unary_unary_rpc_method_handler(unary_unary) -class TestServer(unittest.TestCase): +class TestServer(AioTestBase): def test_unary_unary(self): - loop = asyncio.get_event_loop() async def test_unary_unary_body(): server = aio.server() @@ -53,7 +53,7 @@ class TestServer(unittest.TestCase): response = await unary_call(_REQUEST) self.assertEqual(response, _RESPONSE) - loop.run_until_complete(test_unary_unary_body()) + self.loop.run_until_complete(test_unary_unary_body()) if __name__ == '__main__': From fc82a522a13f61dbe7d72b125ce10607790705a6 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 19:03:38 -0700 Subject: [PATCH 19/30] Add .python-version to .gitignore --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index ed1669c4ccc..06e8706f9a8 100644 --- a/.gitignore +++ b/.gitignore @@ -147,3 +147,7 @@ cmake-build-debug/ # Benchmark outputs BenchmarkDotNet.Artifacts/ + +# pyenv config +.python-version + From b3b2168b603d2743d737bce844315d64317e3092 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 19:09:11 -0700 Subject: [PATCH 20/30] Make YAPF happy --- src/python/grpcio_tests/tests_aio/unit/_test_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio_tests/tests_aio/unit/_test_base.py b/src/python/grpcio_tests/tests_aio/unit/_test_base.py index 09b0ef7abf8..61602259043 100644 --- a/src/python/grpcio_tests/tests_aio/unit/_test_base.py +++ b/src/python/grpcio_tests/tests_aio/unit/_test_base.py @@ -16,6 +16,7 @@ import asyncio import unittest from grpc.experimental import aio + class AioTestBase(unittest.TestCase): def setUp(self): @@ -26,4 +27,3 @@ class AioTestBase(unittest.TestCase): @property def loop(self): return self._loop - From c66bdcdbe0c1d3f1b3c829368ae799b3ed79c2da Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 23 Oct 2019 19:09:41 -0700 Subject: [PATCH 21/30] Unblock the compilation speed up --- src/python/grpcio_tests/tests_aio/unit/_test_server.py | 2 +- tools/run_tests/helper_scripts/build_python.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/grpcio_tests/tests_aio/unit/_test_server.py b/src/python/grpcio_tests/tests_aio/unit/_test_server.py index 1afe3082ac1..dd2d7a98ed0 100644 --- a/src/python/grpcio_tests/tests_aio/unit/_test_server.py +++ b/src/python/grpcio_tests/tests_aio/unit/_test_server.py @@ -1,4 +1,4 @@ -# Copyright 2019 gRPC authors. +# 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. diff --git a/tools/run_tests/helper_scripts/build_python.sh b/tools/run_tests/helper_scripts/build_python.sh index 7cd1ef9d517..8c9f6fdeb33 100755 --- a/tools/run_tests/helper_scripts/build_python.sh +++ b/tools/run_tests/helper_scripts/build_python.sh @@ -122,7 +122,7 @@ export LANG=en_US.UTF-8 # Allow build_ext to build C/C++ files in parallel # by enabling a monkeypatch. It speeds up the build a lot. -export GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS=4 +export GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS=${GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS:-4} # If ccache is available on Linux, use it. if [ "$(is_linux)" ]; then From 322a171d3b62cd2d84087acb37fd309937c38f52 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 21 Oct 2019 09:16:31 -0700 Subject: [PATCH 22/30] Remove unused-parameter warnings, round 2 (6 of 19) --- .../ext/transport/chttp2/transport/parsing.cc | 8 ++++---- .../ext/transport/chttp2/transport/writing.cc | 2 +- .../ext/transport/inproc/inproc_transport.cc | 19 +++++++++--------- src/core/lib/avl/avl.cc | 2 +- src/core/lib/channel/channel_stack.cc | 2 +- src/core/lib/channel/channel_stack.h | 20 +++++++++++++++---- src/core/lib/channel/channelz.cc | 4 ++-- src/core/lib/channel/connected_channel.cc | 5 +++-- src/core/lib/compression/message_compress.cc | 5 +++-- .../stream_compression_identity.cc | 12 +++++------ 10 files changed, 46 insertions(+), 33 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/parsing.cc b/src/core/ext/transport/chttp2/transport/parsing.cc index e789006ed31..2cf2a7ee5a6 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.cc +++ b/src/core/ext/transport/chttp2/transport/parsing.cc @@ -312,13 +312,13 @@ static grpc_error* init_frame_parser(grpc_chttp2_transport* t) { } } -static grpc_error* skip_parser(void* parser, grpc_chttp2_transport* t, - grpc_chttp2_stream* s, const grpc_slice& slice, - int is_last) { +static grpc_error* skip_parser(void* /*parser*/, grpc_chttp2_transport* /*t*/, + grpc_chttp2_stream* /*s*/, + const grpc_slice& /*slice*/, int /*is_last*/) { return GRPC_ERROR_NONE; } -static grpc_error* skip_header(void* tp, grpc_mdelem md) { +static grpc_error* skip_header(void* /*tp*/, grpc_mdelem md) { GRPC_MDELEM_UNREF(md); return GRPC_ERROR_NONE; } diff --git a/src/core/ext/transport/chttp2/transport/writing.cc b/src/core/ext/transport/chttp2/transport/writing.cc index 5fb21c8c86b..312ef54e80e 100644 --- a/src/core/ext/transport/chttp2/transport/writing.cc +++ b/src/core/ext/transport/chttp2/transport/writing.cc @@ -176,7 +176,7 @@ static void report_stall(grpc_chttp2_transport* t, grpc_chttp2_stream* s, } /* How many bytes would we like to put on the wire during a single syscall */ -static uint32_t target_write_size(grpc_chttp2_transport* t) { +static uint32_t target_write_size(grpc_chttp2_transport* /*t*/) { return 1024 * 1024; } diff --git a/src/core/ext/transport/inproc/inproc_transport.cc b/src/core/ext/transport/inproc/inproc_transport.cc index a6d91ef1e05..9cb79f5cbe7 100644 --- a/src/core/ext/transport/inproc/inproc_transport.cc +++ b/src/core/ext/transport/inproc/inproc_transport.cc @@ -900,7 +900,7 @@ bool cancel_stream_locked(inproc_stream* s, grpc_error* error) { return ret; } -void do_nothing(void* arg, grpc_error* error) {} +void do_nothing(void* /*arg*/, grpc_error* /*error*/) {} void perform_stream_op(grpc_transport* gt, grpc_stream* gs, grpc_transport_stream_op_batch* op) { @@ -1140,7 +1140,7 @@ void perform_transport_op(grpc_transport* gt, grpc_transport_op* op) { gpr_mu_unlock(&t->mu->mu); } -void destroy_stream(grpc_transport* gt, grpc_stream* gs, +void destroy_stream(grpc_transport* /*gt*/, grpc_stream* gs, grpc_closure* then_schedule_closure) { INPROC_LOG(GPR_INFO, "destroy_stream %p %p", gs, then_schedule_closure); inproc_stream* s = reinterpret_cast(gs); @@ -1162,16 +1162,17 @@ void destroy_transport(grpc_transport* gt) { * INTEGRATION GLUE */ -void set_pollset(grpc_transport* gt, grpc_stream* gs, grpc_pollset* pollset) { +void set_pollset(grpc_transport* /*gt*/, grpc_stream* /*gs*/, + grpc_pollset* /*pollset*/) { // Nothing to do here } -void set_pollset_set(grpc_transport* gt, grpc_stream* gs, - grpc_pollset_set* pollset_set) { +void set_pollset_set(grpc_transport* /*gt*/, grpc_stream* /*gs*/, + grpc_pollset_set* /*pollset_set*/) { // Nothing to do here } -grpc_endpoint* get_endpoint(grpc_transport* t) { return nullptr; } +grpc_endpoint* get_endpoint(grpc_transport* /*t*/) { return nullptr; } const grpc_transport_vtable inproc_vtable = { sizeof(inproc_stream), "inproc", init_stream, @@ -1183,9 +1184,9 @@ const grpc_transport_vtable inproc_vtable = { * Main inproc transport functions */ void inproc_transports_create(grpc_transport** server_transport, - const grpc_channel_args* server_args, + const grpc_channel_args* /*server_args*/, grpc_transport** client_transport, - const grpc_channel_args* client_args) { + const grpc_channel_args* /*client_args*/) { INPROC_LOG(GPR_INFO, "inproc_transports_create"); shared_mu* mu = new (gpr_malloc(sizeof(*mu))) shared_mu(); inproc_transport* st = new (gpr_malloc(sizeof(*st))) @@ -1221,7 +1222,7 @@ void grpc_inproc_transport_init(void) { grpc_channel* grpc_inproc_channel_create(grpc_server* server, grpc_channel_args* args, - void* reserved) { + void* /*reserved*/) { GRPC_API_TRACE("grpc_inproc_channel_create(server=%p, args=%p)", 2, (server, args)); diff --git a/src/core/lib/avl/avl.cc b/src/core/lib/avl/avl.cc index ec106ddb1da..e04fe35b197 100644 --- a/src/core/lib/avl/avl.cc +++ b/src/core/lib/avl/avl.cc @@ -294,7 +294,7 @@ grpc_avl grpc_avl_remove(grpc_avl avl, void* key, void* user_data) { return avl; } -grpc_avl grpc_avl_ref(grpc_avl avl, void* user_data) { +grpc_avl grpc_avl_ref(grpc_avl avl, void* /*user_data*/) { ref_node(avl.root); return avl; } diff --git a/src/core/lib/channel/channel_stack.cc b/src/core/lib/channel/channel_stack.cc index df956c7176c..e9b348972f2 100644 --- a/src/core/lib/channel/channel_stack.cc +++ b/src/core/lib/channel/channel_stack.cc @@ -203,7 +203,7 @@ void grpc_call_stack_set_pollset_or_pollset_set(grpc_call_stack* call_stack, } void grpc_call_stack_ignore_set_pollset_or_pollset_set( - grpc_call_element* elem, grpc_polling_entity* pollent) {} + grpc_call_element* /*elem*/, grpc_polling_entity* /*pollent*/) {} void grpc_call_stack_destroy(grpc_call_stack* stack, const grpc_call_final_info* final_info, diff --git a/src/core/lib/channel/channel_stack.h b/src/core/lib/channel/channel_stack.h index f7be806b695..f9272871931 100644 --- a/src/core/lib/channel/channel_stack.h +++ b/src/core/lib/channel/channel_stack.h @@ -235,13 +235,25 @@ void grpc_call_stack_set_pollset_or_pollset_set(grpc_call_stack* call_stack, grpc_stream_unref(&(channel_stack)->refcount, reason) #else #define GRPC_CALL_STACK_REF(call_stack, reason) \ - grpc_stream_ref(&(call_stack)->refcount) + do { \ + grpc_stream_ref(&(call_stack)->refcount); \ + (void)(reason); \ + } while (0); #define GRPC_CALL_STACK_UNREF(call_stack, reason) \ - grpc_stream_unref(&(call_stack)->refcount) + do { \ + grpc_stream_unref(&(call_stack)->refcount); \ + (void)(reason); \ + } while (0); #define GRPC_CHANNEL_STACK_REF(channel_stack, reason) \ - grpc_stream_ref(&(channel_stack)->refcount) + do { \ + grpc_stream_ref(&(channel_stack)->refcount); \ + (void)(reason); \ + } while (0); #define GRPC_CHANNEL_STACK_UNREF(channel_stack, reason) \ - grpc_stream_unref(&(channel_stack)->refcount) + do { \ + grpc_stream_unref(&(channel_stack)->refcount); \ + (void)(reason); \ + } while (0); #endif /* Destroy a call stack */ diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index d8a0aa4d068..6300c230d4e 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -54,7 +54,7 @@ namespace channelz { namespace { void* parent_uuid_copy(void* p) { return p; } -void parent_uuid_destroy(void* p) {} +void parent_uuid_destroy(void* /*p*/) {} int parent_uuid_cmp(void* p1, void* p2) { return GPR_ICMP(p1, p2); } const grpc_arg_pointer_vtable parent_uuid_vtable = { parent_uuid_copy, parent_uuid_destroy, parent_uuid_cmp}; @@ -315,7 +315,7 @@ void ChannelNode::RemoveChildSubchannel(intptr_t child_uuid) { // ServerNode // -ServerNode::ServerNode(grpc_server* server, size_t channel_tracer_max_nodes) +ServerNode::ServerNode(grpc_server* /*server*/, size_t channel_tracer_max_nodes) : BaseNode(EntityType::kServer, /* name */ nullptr), trace_(channel_tracer_max_nodes) {} diff --git a/src/core/lib/channel/connected_channel.cc b/src/core/lib/channel/connected_channel.cc index c9c64b8cb33..23ff63bf316 100644 --- a/src/core/lib/channel/connected_channel.cc +++ b/src/core/lib/channel/connected_channel.cc @@ -167,7 +167,7 @@ static void set_pollset_or_pollset_set(grpc_call_element* elem, /* Destructor for call_data */ static void connected_channel_destroy_call_elem( - grpc_call_element* elem, const grpc_call_final_info* final_info, + grpc_call_element* elem, const grpc_call_final_info* /*final_info*/, grpc_closure* then_schedule_closure) { call_data* calld = static_cast(elem->call_data); channel_data* chand = static_cast(elem->channel_data); @@ -195,7 +195,8 @@ static void connected_channel_destroy_channel_elem(grpc_channel_element* elem) { /* No-op. */ static void connected_channel_get_channel_info( - grpc_channel_element* elem, const grpc_channel_info* channel_info) {} + grpc_channel_element* /*elem*/, const grpc_channel_info* /*channel_info*/) { +} const grpc_channel_filter grpc_connected_filter = { connected_channel_start_transport_stream_op_batch, diff --git a/src/core/lib/compression/message_compress.cc b/src/core/lib/compression/message_compress.cc index e06454f871c..7faa1dcd828 100644 --- a/src/core/lib/compression/message_compress.cc +++ b/src/core/lib/compression/message_compress.cc @@ -80,11 +80,12 @@ error: return 0; } -static void* zalloc_gpr(void* opaque, unsigned int items, unsigned int size) { +static void* zalloc_gpr(void* /*opaque*/, unsigned int items, + unsigned int size) { return gpr_malloc(items * size); } -static void zfree_gpr(void* opaque, void* address) { gpr_free(address); } +static void zfree_gpr(void* /*opaque*/, void* address) { gpr_free(address); } static int zlib_compress(grpc_slice_buffer* input, grpc_slice_buffer* output, int gzip) { diff --git a/src/core/lib/compression/stream_compression_identity.cc b/src/core/lib/compression/stream_compression_identity.cc index b7981394d5e..d5e3afff052 100644 --- a/src/core/lib/compression/stream_compression_identity.cc +++ b/src/core/lib/compression/stream_compression_identity.cc @@ -47,12 +47,10 @@ static void grpc_stream_compression_pass_through(grpc_slice_buffer* in, } } -static bool grpc_stream_compress_identity(grpc_stream_compression_context* ctx, - grpc_slice_buffer* in, - grpc_slice_buffer* out, - size_t* output_size, - size_t max_output_size, - grpc_stream_compression_flush flush) { +static bool grpc_stream_compress_identity( + grpc_stream_compression_context* ctx, grpc_slice_buffer* in, + grpc_slice_buffer* out, size_t* output_size, size_t max_output_size, + grpc_stream_compression_flush /*flush*/) { if (ctx == nullptr) { return false; } @@ -84,7 +82,7 @@ grpc_stream_compression_context_create_identity( } static void grpc_stream_compression_context_destroy_identity( - grpc_stream_compression_context* ctx) { + grpc_stream_compression_context* /*ctx*/) { return; } From 2477b966e2b96d2949466e5c48d1d4eef228841a Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 21 Oct 2019 09:16:35 -0700 Subject: [PATCH 23/30] Remove unused-parameter warnings, round 2 (17 of 19) --- test/core/iomgr/ev_epollex_linux_test.cc | 2 +- test/core/iomgr/tcp_client_uv_test.cc | 2 +- test/core/iomgr/tcp_server_uv_test.cc | 2 +- test/core/json/json_stream_error_test.cc | 6 ++++-- test/core/security/check_gcp_environment_linux_test.cc | 4 ++-- test/core/security/check_gcp_environment_windows_test.cc | 2 +- test/core/security/grpc_alts_credentials_options_test.cc | 2 +- test/core/security/oauth2_utils.cc | 2 +- test/core/transport/chttp2/bin_encoder_test.cc | 2 +- 9 files changed, 13 insertions(+), 11 deletions(-) diff --git a/test/core/iomgr/ev_epollex_linux_test.cc b/test/core/iomgr/ev_epollex_linux_test.cc index 08a28e0b515..5bc5935a95e 100644 --- a/test/core/iomgr/ev_epollex_linux_test.cc +++ b/test/core/iomgr/ev_epollex_linux_test.cc @@ -111,5 +111,5 @@ int main(int argc, char** argv) { return 0; } #else /* defined(GRPC_LINUX_EPOLL_CREATE1) && defined(GRPC_LINUX_EVENTFD) */ -int main(int argc, char** argv) { return 0; } +int main(int /*argc*/, char** /*argv*/) { return 0; } #endif diff --git a/test/core/iomgr/tcp_client_uv_test.cc b/test/core/iomgr/tcp_client_uv_test.cc index bde8c2f3539..90a1a432771 100644 --- a/test/core/iomgr/tcp_client_uv_test.cc +++ b/test/core/iomgr/tcp_client_uv_test.cc @@ -209,6 +209,6 @@ int main(int argc, char** argv) { #else /* GRPC_UV */ -int main(int argc, char** argv) { return 1; } +int main(int /*argc*/, char** /*argv*/) { return 1; } #endif /* GRPC_UV */ diff --git a/test/core/iomgr/tcp_server_uv_test.cc b/test/core/iomgr/tcp_server_uv_test.cc index 625a18c0cc7..a084cccbb27 100644 --- a/test/core/iomgr/tcp_server_uv_test.cc +++ b/test/core/iomgr/tcp_server_uv_test.cc @@ -314,6 +314,6 @@ int main(int argc, char** argv) { #else /* GRPC_UV */ -int main(int argc, char** argv) { return 1; } +int main(int /*argc*/, char** /*argv*/) { return 1; } #endif /* GRPC_UV */ diff --git a/test/core/json/json_stream_error_test.cc b/test/core/json/json_stream_error_test.cc index 53e20be2069..f31e9c95d86 100644 --- a/test/core/json/json_stream_error_test.cc +++ b/test/core/json/json_stream_error_test.cc @@ -28,12 +28,14 @@ static int g_string_clear_once = 0; -static void string_clear(void* userdata) { +static void string_clear(void* /*userdata*/) { GPR_ASSERT(!g_string_clear_once); g_string_clear_once = 1; } -static uint32_t read_char(void* userdata) { return GRPC_JSON_READ_CHAR_ERROR; } +static uint32_t read_char(void* /*userdata*/) { + return GRPC_JSON_READ_CHAR_ERROR; +} static grpc_json_reader_vtable reader_vtable = { string_clear, nullptr, nullptr, read_char, nullptr, nullptr, diff --git a/test/core/security/check_gcp_environment_linux_test.cc b/test/core/security/check_gcp_environment_linux_test.cc index b01471abd34..2f266cb4001 100644 --- a/test/core/security/check_gcp_environment_linux_test.cc +++ b/test/core/security/check_gcp_environment_linux_test.cc @@ -72,7 +72,7 @@ static void test_gcp_environment_check_failure() { GPR_ASSERT(!check_bios_data_linux_test("\n")); } -int main(int argc, char** argv) { +int main(int /*argc*/, char** /*argv*/) { /* Tests. */ test_gcp_environment_check_success(); test_gcp_environment_check_failure(); @@ -81,6 +81,6 @@ int main(int argc, char** argv) { #else // GPR_LINUX -int main(int argc, char** argv) { return 0; } +int main(int /*argc*/, char** /*argv*/) { return 0; } #endif // GPR_LINUX diff --git a/test/core/security/check_gcp_environment_windows_test.cc b/test/core/security/check_gcp_environment_windows_test.cc index 2b9407e8cad..0f08e7106ec 100644 --- a/test/core/security/check_gcp_environment_windows_test.cc +++ b/test/core/security/check_gcp_environment_windows_test.cc @@ -68,6 +68,6 @@ int main(int argc, char** argv) { } #else // GPR_WINDOWS -int main(int argc, char** argv) { return 0; } +int main(int /*argc*/, char** /*argv*/) { return 0; } #endif // GPR_WINDOWS diff --git a/test/core/security/grpc_alts_credentials_options_test.cc b/test/core/security/grpc_alts_credentials_options_test.cc index 623db48ebca..d33b59191d2 100644 --- a/test/core/security/grpc_alts_credentials_options_test.cc +++ b/test/core/security/grpc_alts_credentials_options_test.cc @@ -87,7 +87,7 @@ static void test_client_options_api_success() { grpc_alts_credentials_options_destroy(new_options); } -int main(int argc, char** argv) { +int main(int /*argc*/, char** /*argv*/) { /* Test. */ test_copy_client_options_failure(); test_client_options_api_success(); diff --git a/test/core/security/oauth2_utils.cc b/test/core/security/oauth2_utils.cc index 8b4c795a9ff..cab06f90ff1 100644 --- a/test/core/security/oauth2_utils.cc +++ b/test/core/security/oauth2_utils.cc @@ -63,7 +63,7 @@ static void on_oauth2_response(void* arg, grpc_error* error) { gpr_mu_unlock(request->mu); } -static void destroy_after_shutdown(void* pollset, grpc_error* error) { +static void destroy_after_shutdown(void* pollset, grpc_error* /*error*/) { grpc_pollset_destroy(reinterpret_cast(pollset)); gpr_free(pollset); } diff --git a/test/core/transport/chttp2/bin_encoder_test.cc b/test/core/transport/chttp2/bin_encoder_test.cc index bd62b0e479a..9702838c842 100644 --- a/test/core/transport/chttp2/bin_encoder_test.cc +++ b/test/core/transport/chttp2/bin_encoder_test.cc @@ -98,7 +98,7 @@ static void expect_binary_header(const char* hdr, int binary) { } } -int main(int argc, char** argv) { +int main(int /*argc*/, char** /*argv*/) { grpc_init(); /* Base64 test vectors from RFC 4648, with padding removed */ From 2fbd97217b1cc2052b67fbaecc5b655ef9e0b4c4 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Thu, 24 Oct 2019 15:10:28 -0700 Subject: [PATCH 24/30] Turn the single threaded runner on by default. --- src/python/grpcio_tests/tests/_runner.py | 6 +++++- src/python/grpcio_tests/tests_aio/unit/_test_server.py | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/python/grpcio_tests/tests/_runner.py b/src/python/grpcio_tests/tests/_runner.py index 6411ffa8981..804b1a6e331 100644 --- a/src/python/grpcio_tests/tests/_runner.py +++ b/src/python/grpcio_tests/tests/_runner.py @@ -117,7 +117,7 @@ class AugmentedCase(collections.namedtuple('AugmentedCase', ['case', 'id'])): class Runner(object): - def __init__(self, dedicated_threads=True): + def __init__(self, dedicated_threads=False): """Constructs the Runner object. Args: @@ -202,11 +202,13 @@ class Runner(object): augmented_case.case.id())) sys.stdout.flush() if self._dedicated_threads: + # (Deprecated) Spawns dedicated thread for each test case. case_thread = threading.Thread( target=augmented_case.case.run, args=(result,)) try: with stdout_pipe, stderr_pipe: case_thread.start() + # If the thread is exited unexpected, stop testing. while case_thread.is_alive(): check_kill_self() time.sleep(0) @@ -214,6 +216,7 @@ class Runner(object): except: # pylint: disable=try-except-raise # re-raise the exception after forcing the with-block to end raise + # Records the result of the test case run. result.set_output(augmented_case.case, stdout_pipe.output(), stderr_pipe.output()) sys.stdout.write(result_out.getvalue()) @@ -221,6 +224,7 @@ class Runner(object): result_out.truncate(0) check_kill_self() else: + # Donates current thread to test case execution. augmented_case.case.run(result) result.stopTestRun() stdout_pipe.close() diff --git a/src/python/grpcio_tests/tests_aio/unit/_test_server.py b/src/python/grpcio_tests/tests_aio/unit/_test_server.py index dd2d7a98ed0..5f3661f42cf 100644 --- a/src/python/grpcio_tests/tests_aio/unit/_test_server.py +++ b/src/python/grpcio_tests/tests_aio/unit/_test_server.py @@ -24,7 +24,7 @@ from src.proto.grpc.testing import test_pb2_grpc from tests.unit.framework.common import test_constants -class TestServiceServicer(test_pb2_grpc.TestServiceServicer): +class _TestServiceServicer(test_pb2_grpc.TestServiceServicer): async def UnaryCall(self, request, context): return messages_pb2.SimpleResponse() @@ -36,7 +36,7 @@ class TestServiceServicer(test_pb2_grpc.TestServiceServicer): async def start_test_server(): server = aio.server(options=(('grpc.so_reuseport', 0),)) - test_pb2_grpc.add_TestServiceServicer_to_server(TestServiceServicer(), + test_pb2_grpc.add_TestServiceServicer_to_server(_TestServiceServicer(), server) port = server.add_insecure_port('[::]:0') await server.start() From cad65769e21ef4c37e09f33c187b437a8deb2635 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Wed, 23 Oct 2019 00:39:35 -0700 Subject: [PATCH 25/30] Remove unused parameters altogether, not just names --- .../filters/client_channel/client_channel.cc | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index b94d7c7efcc..8812b2b1ad6 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -633,8 +633,8 @@ class CallData { // Sets *status and *server_pushback_md based on md_batch and error. // Only sets *server_pushback_md if server_pushback_md != nullptr. - void GetCallStatus(grpc_call_element* elem, grpc_metadata_batch* md_batch, - grpc_error* error, grpc_status_code* status, + void GetCallStatus(grpc_metadata_batch* md_batch, grpc_error* error, + grpc_status_code* status, grpc_mdelem** server_pushback_md); // Adds recv_trailing_metadata_ready closure to closures. void AddClosureForRecvTrailingMetadataReady( @@ -663,10 +663,10 @@ class CallData { // Adds the on_complete closure for the pending batch completed in // batch_data to closures. - void AddClosuresForCompletedPendingBatch( - grpc_call_element* elem, SubchannelCallBatchData* batch_data, - SubchannelCallRetryState* retry_state, grpc_error* error, - CallCombinerClosureList* closures); + void AddClosuresForCompletedPendingBatch(grpc_call_element* elem, + SubchannelCallBatchData* batch_data, + grpc_error* error, + CallCombinerClosureList* closures); // If there are any cached ops to replay or pending ops to start on the // subchannel call, adds a closure to closures to invoke @@ -2908,8 +2908,7 @@ void CallData::RecvMessageReady(void* arg, grpc_error* error) { // recv_trailing_metadata handling // -void CallData::GetCallStatus(grpc_call_element* /*elem*/, - grpc_metadata_batch* md_batch, grpc_error* error, +void CallData::GetCallStatus(grpc_metadata_batch* md_batch, grpc_error* error, grpc_status_code* status, grpc_mdelem** server_pushback_md) { if (error != GRPC_ERROR_NONE) { @@ -3078,7 +3077,7 @@ void CallData::RecvTrailingMetadataReady(void* arg, grpc_error* error) { grpc_mdelem* server_pushback_md = nullptr; grpc_metadata_batch* md_batch = batch_data->batch.payload->recv_trailing_metadata.recv_trailing_metadata; - calld->GetCallStatus(elem, md_batch, GRPC_ERROR_REF(error), &status, + calld->GetCallStatus(md_batch, GRPC_ERROR_REF(error), &status, &server_pushback_md); if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_call_trace)) { gpr_log(GPR_INFO, "chand=%p calld=%p: call finished, status=%s", chand, @@ -3111,8 +3110,7 @@ void CallData::RecvTrailingMetadataReady(void* arg, grpc_error* error) { void CallData::AddClosuresForCompletedPendingBatch( grpc_call_element* elem, SubchannelCallBatchData* batch_data, - SubchannelCallRetryState* /*retry_state*/, grpc_error* error, - CallCombinerClosureList* closures) { + grpc_error* error, CallCombinerClosureList* closures) { PendingBatch* pending = PendingBatchFind( elem, "completed", [batch_data](grpc_transport_stream_op_batch* batch) { // Match the pending batch with the same set of send ops as the @@ -3210,7 +3208,7 @@ void CallData::OnComplete(void* arg, grpc_error* error) { if (!retry_state->retry_dispatched) { // Add closure for the completed pending batch, if any. calld->AddClosuresForCompletedPendingBatch( - elem, batch_data, retry_state, GRPC_ERROR_REF(error), &closures); + elem, batch_data, GRPC_ERROR_REF(error), &closures); // If needed, add a callback to start any replay or pending send ops on // the subchannel call. if (!retry_state->completed_recv_trailing_metadata) { From fb74da86201352967b4ae272d8a4ac2f7b2c0930 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 25 Oct 2019 10:58:10 -0700 Subject: [PATCH 26/30] Adopt reviewer's advice --- src/python/grpcio_tests/tests_aio/unit/channel_test.py | 7 +++---- src/python/grpcio_tests/tests_aio/unit/init_test.py | 1 - src/python/grpcio_tests/tests_aio/unit/server_test.py | 1 - 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/python/grpcio_tests/tests_aio/unit/channel_test.py b/src/python/grpcio_tests/tests_aio/unit/channel_test.py index 4f95fcfb850..e18b6da6d39 100644 --- a/src/python/grpcio_tests/tests_aio/unit/channel_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/channel_test.py @@ -41,7 +41,7 @@ class TestChannel(AioTestBase): ) await hi(messages_pb2.SimpleRequest()) - self._loop.run_until_complete(coro()) + self.loop.run_until_complete(coro()) def test_unary_unary(self): @@ -59,7 +59,7 @@ class TestChannel(AioTestBase): await channel.close() - self._loop.run_until_complete(coro()) + self.loop.run_until_complete(coro()) def test_unary_call_times_out(self): @@ -91,10 +91,9 @@ class TestChannel(AioTestBase): self.assertIsNotNone( exception_context.exception.trailing_metadata()) - self._loop.run_until_complete(coro()) + self.loop.run_until_complete(coro()) if __name__ == '__main__': - aio.init_grpc_aio() logging.basicConfig() unittest.main(verbosity=2) diff --git a/src/python/grpcio_tests/tests_aio/unit/init_test.py b/src/python/grpcio_tests/tests_aio/unit/init_test.py index b2a07633028..297f178ee44 100644 --- a/src/python/grpcio_tests/tests_aio/unit/init_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/init_test.py @@ -75,6 +75,5 @@ class TestInsecureChannel(AioTestBase): if __name__ == '__main__': - aio.init_grpc_aio() logging.basicConfig() unittest.main(verbosity=2) diff --git a/src/python/grpcio_tests/tests_aio/unit/server_test.py b/src/python/grpcio_tests/tests_aio/unit/server_test.py index ba1c53472e3..15f4ff182d6 100644 --- a/src/python/grpcio_tests/tests_aio/unit/server_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/server_test.py @@ -57,6 +57,5 @@ class TestServer(AioTestBase): if __name__ == '__main__': - aio.init_grpc_aio() logging.basicConfig() unittest.main(verbosity=2) From ac6d942a5bca0d042c62856fe555a14fb077e55e Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 25 Oct 2019 11:26:19 -0700 Subject: [PATCH 27/30] Make the speed up of compilation automatic on Linux --- tools/run_tests/helper_scripts/build_python.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/helper_scripts/build_python.sh b/tools/run_tests/helper_scripts/build_python.sh index 8c9f6fdeb33..52f9729e260 100755 --- a/tools/run_tests/helper_scripts/build_python.sh +++ b/tools/run_tests/helper_scripts/build_python.sh @@ -122,7 +122,8 @@ export LANG=en_US.UTF-8 # Allow build_ext to build C/C++ files in parallel # by enabling a monkeypatch. It speeds up the build a lot. -export GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS=${GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS:-4} +DEFAULT_PARALLEL_JOBS=`nproc` || DEFAULT_PARALLEL_JOBS=4 +export GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS=${GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS:-$DEFAULT_PARALLEL_JOBS} # If ccache is available on Linux, use it. if [ "$(is_linux)" ]; then From a3689a8b6c0655df21f98e7e34df11aeb4eb22ee Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Fri, 25 Oct 2019 11:49:03 -0700 Subject: [PATCH 28/30] Properly handle race at thread creation --- src/cpp/thread_manager/thread_manager.cc | 20 ++++++++++++++++---- src/cpp/thread_manager/thread_manager.h | 4 ++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/cpp/thread_manager/thread_manager.cc b/src/cpp/thread_manager/thread_manager.cc index 2b65352f797..b633598660d 100644 --- a/src/cpp/thread_manager/thread_manager.cc +++ b/src/cpp/thread_manager/thread_manager.cc @@ -34,8 +34,10 @@ ThreadManager::WorkerThread::WorkerThread(ThreadManager* thd_mgr) thd_ = grpc_core::Thread( "grpcpp_sync_server", [](void* th) { static_cast(th)->Run(); }, - this); - thd_.Start(); + this, &created_); + if (!created_) { + gpr_log(GPR_ERROR, "Could not create grpc_sync_server worker-thread"); + } } void ThreadManager::WorkerThread::Run() { @@ -139,7 +141,9 @@ void ThreadManager::Initialize() { } for (int i = 0; i < min_pollers_; i++) { - new WorkerThread(this); + WorkerThread* worker = new WorkerThread(this); + GPR_ASSERT(worker->created()); // Must be able to create the minimum + worker->Start(); } } @@ -177,7 +181,15 @@ void ThreadManager::MainWorkLoop() { } // Drop lock before spawning thread to avoid contention lock.Unlock(); - new WorkerThread(this); + WorkerThread* worker = new WorkerThread(this); + if (worker->created()) { + worker->Start(); + } else { + num_pollers_--; + num_threads_--; + resource_exhausted = true; + delete worker; + } } else if (num_pollers_ > 0) { // There is still at least some thread polling, so we can go on // even though we are below the number of pollers that we would diff --git a/src/cpp/thread_manager/thread_manager.h b/src/cpp/thread_manager/thread_manager.h index 4c1b3aad2b4..f275e6ff9d3 100644 --- a/src/cpp/thread_manager/thread_manager.h +++ b/src/cpp/thread_manager/thread_manager.h @@ -124,6 +124,9 @@ class ThreadManager { WorkerThread(ThreadManager* thd_mgr); ~WorkerThread(); + bool created() const { return created_; } + void Start() { thd_.Start(); } + private: // Calls thd_mgr_->MainWorkLoop() and once that completes, calls // thd_mgr_>MarkAsCompleted(this) to mark the thread as completed @@ -131,6 +134,7 @@ class ThreadManager { ThreadManager* const thd_mgr_; grpc_core::Thread thd_; + bool created_; }; // The main function in ThreadManager From 52b5c5a57560acb6e59a7fc8b3dfe45364445c83 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 25 Oct 2019 13:05:05 -0700 Subject: [PATCH 29/30] Make shellcheck happy --- tools/run_tests/helper_scripts/build_python.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/helper_scripts/build_python.sh b/tools/run_tests/helper_scripts/build_python.sh index 52f9729e260..87c16a2ec4f 100755 --- a/tools/run_tests/helper_scripts/build_python.sh +++ b/tools/run_tests/helper_scripts/build_python.sh @@ -122,7 +122,7 @@ export LANG=en_US.UTF-8 # Allow build_ext to build C/C++ files in parallel # by enabling a monkeypatch. It speeds up the build a lot. -DEFAULT_PARALLEL_JOBS=`nproc` || DEFAULT_PARALLEL_JOBS=4 +DEFAULT_PARALLEL_JOBS=$(nproc) || DEFAULT_PARALLEL_JOBS=4 export GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS=${GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS:-$DEFAULT_PARALLEL_JOBS} # If ccache is available on Linux, use it. From 1784f2e449789f742bd13d101f71d2084dfd4371 Mon Sep 17 00:00:00 2001 From: Hope Casey-Allen Date: Fri, 25 Oct 2019 13:26:40 -0700 Subject: [PATCH 30/30] Update contributing doc with copyright message info --- CONTRIBUTING.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1241f9162ac..c524257a5a3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -72,6 +72,10 @@ How to get your contributions merged smoothly and quickly. - Don't fix code style and formatting unless you are already changing that line to address an issue. PRs with irrelevant changes won't be merged. If you do want to fix formatting or style, do that in a separate PR. + +- If you are adding a new file, make sure it has the copyright message template + at the top as a comment. You can copy over the message from an existing file + and update the year. - Unless your PR is trivial, you should expect there will be reviewer comments that you'll need to address before merging. We expect you to be reasonably