diff options
23 files changed, 346 insertions, 41 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt index 3b41ec6c6e..700fa48abc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -665,6 +665,7 @@ add_dependencies(buildtests_cxx server_crash_test) endif() add_dependencies(buildtests_cxx server_crash_test_client) add_dependencies(buildtests_cxx server_early_return_test) +add_dependencies(buildtests_cxx server_interceptors_end2end_test) add_dependencies(buildtests_cxx server_request_call_test) add_dependencies(buildtests_cxx shutdown_test) add_dependencies(buildtests_cxx slice_hash_table_test) @@ -15342,6 +15343,47 @@ target_link_libraries(server_early_return_test endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) +add_executable(server_interceptors_end2end_test + test/cpp/end2end/server_interceptors_end2end_test.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + + +target_include_directories(server_interceptors_end2end_test + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include + PRIVATE ${_gRPC_SSL_INCLUDE_DIR} + PRIVATE ${_gRPC_PROTOBUF_INCLUDE_DIR} + PRIVATE ${_gRPC_ZLIB_INCLUDE_DIR} + PRIVATE ${_gRPC_BENCHMARK_INCLUDE_DIR} + PRIVATE ${_gRPC_CARES_INCLUDE_DIR} + PRIVATE ${_gRPC_GFLAGS_INCLUDE_DIR} + PRIVATE ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} + PRIVATE ${_gRPC_NANOPB_INCLUDE_DIR} + PRIVATE third_party/googletest/googletest/include + PRIVATE third_party/googletest/googletest + PRIVATE third_party/googletest/googlemock/include + PRIVATE third_party/googletest/googlemock + PRIVATE ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(server_interceptors_end2end_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc++_test_util + grpc_test_util + grpc++ + grpc + gpr_test_util + gpr + ${_gRPC_GFLAGS_LIBRARIES} +) + + +endif (gRPC_BUILD_TESTS) +if (gRPC_BUILD_TESTS) + add_executable(server_request_call_test ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.pb.cc ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.grpc.pb.cc @@ -1230,6 +1230,7 @@ server_context_test_spouse_test: $(BINDIR)/$(CONFIG)/server_context_test_spouse_ server_crash_test: $(BINDIR)/$(CONFIG)/server_crash_test server_crash_test_client: $(BINDIR)/$(CONFIG)/server_crash_test_client server_early_return_test: $(BINDIR)/$(CONFIG)/server_early_return_test +server_interceptors_end2end_test: $(BINDIR)/$(CONFIG)/server_interceptors_end2end_test server_request_call_test: $(BINDIR)/$(CONFIG)/server_request_call_test shutdown_test: $(BINDIR)/$(CONFIG)/shutdown_test slice_hash_table_test: $(BINDIR)/$(CONFIG)/slice_hash_table_test @@ -1727,6 +1728,7 @@ buildtests_cxx: privatelibs_cxx \ $(BINDIR)/$(CONFIG)/server_crash_test \ $(BINDIR)/$(CONFIG)/server_crash_test_client \ $(BINDIR)/$(CONFIG)/server_early_return_test \ + $(BINDIR)/$(CONFIG)/server_interceptors_end2end_test \ $(BINDIR)/$(CONFIG)/server_request_call_test \ $(BINDIR)/$(CONFIG)/shutdown_test \ $(BINDIR)/$(CONFIG)/slice_hash_table_test \ @@ -1909,6 +1911,7 @@ buildtests_cxx: privatelibs_cxx \ $(BINDIR)/$(CONFIG)/server_crash_test \ $(BINDIR)/$(CONFIG)/server_crash_test_client \ $(BINDIR)/$(CONFIG)/server_early_return_test \ + $(BINDIR)/$(CONFIG)/server_interceptors_end2end_test \ $(BINDIR)/$(CONFIG)/server_request_call_test \ $(BINDIR)/$(CONFIG)/shutdown_test \ $(BINDIR)/$(CONFIG)/slice_hash_table_test \ @@ -2401,6 +2404,8 @@ test_cxx: buildtests_cxx $(Q) $(BINDIR)/$(CONFIG)/server_crash_test || ( echo test server_crash_test failed ; exit 1 ) $(E) "[RUN] Testing server_early_return_test" $(Q) $(BINDIR)/$(CONFIG)/server_early_return_test || ( echo test server_early_return_test failed ; exit 1 ) + $(E) "[RUN] Testing server_interceptors_end2end_test" + $(Q) $(BINDIR)/$(CONFIG)/server_interceptors_end2end_test || ( echo test server_interceptors_end2end_test failed ; exit 1 ) $(E) "[RUN] Testing server_request_call_test" $(Q) $(BINDIR)/$(CONFIG)/server_request_call_test || ( echo test server_request_call_test failed ; exit 1 ) $(E) "[RUN] Testing shutdown_test" @@ -20137,6 +20142,49 @@ endif endif +SERVER_INTERCEPTORS_END2END_TEST_SRC = \ + test/cpp/end2end/server_interceptors_end2end_test.cc \ + +SERVER_INTERCEPTORS_END2END_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(SERVER_INTERCEPTORS_END2END_TEST_SRC)))) +ifeq ($(NO_SECURE),true) + +# You can't build secure targets if you don't have OpenSSL. + +$(BINDIR)/$(CONFIG)/server_interceptors_end2end_test: openssl_dep_error + +else + + + + +ifeq ($(NO_PROTOBUF),true) + +# You can't build the protoc plugins or protobuf-enabled targets if you don't have protobuf 3.5.0+. + +$(BINDIR)/$(CONFIG)/server_interceptors_end2end_test: protobuf_dep_error + +else + +$(BINDIR)/$(CONFIG)/server_interceptors_end2end_test: $(PROTOBUF_DEP) $(SERVER_INTERCEPTORS_END2END_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + $(E) "[LD] Linking $@" + $(Q) mkdir -p `dirname $@` + $(Q) $(LDXX) $(LDFLAGS) $(SERVER_INTERCEPTORS_END2END_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBSXX) $(LDLIBS_PROTOBUF) $(LDLIBS) $(LDLIBS_SECURE) $(GTEST_LIB) -o $(BINDIR)/$(CONFIG)/server_interceptors_end2end_test + +endif + +endif + +$(OBJDIR)/$(CONFIG)/test/cpp/end2end/server_interceptors_end2end_test.o: $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + +deps_server_interceptors_end2end_test: $(SERVER_INTERCEPTORS_END2END_TEST_OBJS:.o=.dep) + +ifneq ($(NO_SECURE),true) +ifneq ($(NO_DEPS),true) +-include $(SERVER_INTERCEPTORS_END2END_TEST_OBJS:.o=.dep) +endif +endif + + SERVER_REQUEST_CALL_TEST_SRC = \ $(GENDIR)/src/proto/grpc/testing/echo_messages.pb.cc $(GENDIR)/src/proto/grpc/testing/echo_messages.grpc.pb.cc \ $(GENDIR)/src/proto/grpc/testing/echo.pb.cc $(GENDIR)/src/proto/grpc/testing/echo.grpc.pb.cc \ diff --git a/build.yaml b/build.yaml index e5955007d4..e7e92d280d 100644 --- a/build.yaml +++ b/build.yaml @@ -5456,6 +5456,22 @@ targets: - grpc - gpr_test_util - gpr +- name: server_interceptors_end2end_test + gtest: true + cpu_cost: 0.5 + build: test + language: c++ + headers: + - test/cpp/end2end/interceptors_util.h + src: + - test/cpp/end2end/server_interceptors_end2end_test.cc + deps: + - grpc++_test_util + - grpc_test_util + - grpc++ + - grpc + - gpr_test_util + - gpr - name: server_request_call_test gtest: true build: test diff --git a/include/grpcpp/impl/codegen/channel_interface.h b/include/grpcpp/impl/codegen/channel_interface.h index 6fd1dd1d9b..e957ea6aab 100644 --- a/include/grpcpp/impl/codegen/channel_interface.h +++ b/include/grpcpp/impl/codegen/channel_interface.h @@ -21,6 +21,7 @@ #include <grpc/impl/codegen/connectivity_state.h> #include <grpcpp/impl/codegen/call.h> +#include <grpcpp/impl/codegen/client_context.h> #include <grpcpp/impl/codegen/status.h> #include <grpcpp/impl/codegen/time.h> diff --git a/include/grpcpp/impl/codegen/client_interceptor.h b/include/grpcpp/impl/codegen/client_interceptor.h index 00113f04aa..0e08a7ce01 100644 --- a/include/grpcpp/impl/codegen/client_interceptor.h +++ b/include/grpcpp/impl/codegen/client_interceptor.h @@ -21,7 +21,6 @@ #include <vector> -#include <grpc/impl/codegen/log.h> #include <grpcpp/impl/codegen/interceptor.h> #include <grpcpp/impl/codegen/string_ref.h> diff --git a/include/grpcpp/impl/codegen/interceptor.h b/include/grpcpp/impl/codegen/interceptor.h index cdd34b80d1..15cab711e5 100644 --- a/include/grpcpp/impl/codegen/interceptor.h +++ b/include/grpcpp/impl/codegen/interceptor.h @@ -21,13 +21,13 @@ #include <grpc/impl/codegen/grpc_types.h> #include <grpcpp/impl/codegen/byte_buffer.h> -#include <grpcpp/impl/codegen/channel_interface.h> #include <grpcpp/impl/codegen/config.h> #include <grpcpp/impl/codegen/core_codegen_interface.h> #include <grpcpp/impl/codegen/metadata_map.h> namespace grpc { +class ChannelInterface; class Status; namespace experimental { diff --git a/include/grpcpp/impl/codegen/method_handler_impl.h b/include/grpcpp/impl/codegen/method_handler_impl.h index 4f02e3e39b..dd53f975f6 100644 --- a/include/grpcpp/impl/codegen/method_handler_impl.h +++ b/include/grpcpp/impl/codegen/method_handler_impl.h @@ -66,7 +66,7 @@ class RpcMethodHandler : public MethodHandler { return func_(service_, param.server_context, static_cast<RequestType*>(param.request), &rsp); }); - delete static_cast<RequestType*>(param.request); + static_cast<RequestType*>(param.request)->~RequestType(); } GPR_CODEGEN_ASSERT(!param.server_context->sent_initial_metadata_); @@ -86,16 +86,18 @@ class RpcMethodHandler : public MethodHandler { param.call->cq()->Pluck(&ops); } - void* Deserialize(grpc_byte_buffer* req, Status* status) final { + void* Deserialize(grpc_call* call, grpc_byte_buffer* req, + Status* status) final { ByteBuffer buf; buf.set_buffer(req); - auto* request = new RequestType(); + auto* request = new (g_core_codegen_interface->grpc_call_arena_alloc( + call, sizeof(RequestType))) RequestType(); *status = SerializationTraits<RequestType>::Deserialize(&buf, request); buf.Release(); if (status->ok()) { return request; } - delete request; + request->~RequestType(); return nullptr; } @@ -170,7 +172,7 @@ class ServerStreamingHandler : public MethodHandler { return func_(service_, param.server_context, static_cast<RequestType*>(param.request), &writer); }); - delete static_cast<RequestType*>(param.request); + static_cast<RequestType*>(param.request)->~RequestType(); } CallOpSet<CallOpSendInitialMetadata, CallOpServerSendStatus> ops; @@ -189,16 +191,18 @@ class ServerStreamingHandler : public MethodHandler { param.call->cq()->Pluck(&ops); } - void* Deserialize(grpc_byte_buffer* req, Status* status) final { + void* Deserialize(grpc_call* call, grpc_byte_buffer* req, + Status* status) final { ByteBuffer buf; buf.set_buffer(req); - auto* request = new RequestType(); + auto* request = new (g_core_codegen_interface->grpc_call_arena_alloc( + call, sizeof(RequestType))) RequestType(); *status = SerializationTraits<RequestType>::Deserialize(&buf, request); buf.Release(); if (status->ok()) { return request; } - delete request; + request->~RequestType(); return nullptr; } @@ -323,7 +327,8 @@ class ErrorMethodHandler : public MethodHandler { param.call->cq()->Pluck(&ops); } - void* Deserialize(grpc_byte_buffer* req, Status* status) final { + void* Deserialize(grpc_call* call, grpc_byte_buffer* req, + Status* status) final { // We have to destroy any request payload if (req != nullptr) { g_core_codegen_interface->grpc_byte_buffer_destroy(req); diff --git a/include/grpcpp/impl/codegen/rpc_service_method.h b/include/grpcpp/impl/codegen/rpc_service_method.h index 44da2bd768..e77f4046a3 100644 --- a/include/grpcpp/impl/codegen/rpc_service_method.h +++ b/include/grpcpp/impl/codegen/rpc_service_method.h @@ -56,7 +56,8 @@ class MethodHandler { a HandlerParameter and passed to RunHandler. It is illegal to access the pointer after calling RunHandler. Ownership of the deserialized request is retained by the handler. Returns nullptr if deserialization failed. */ - virtual void* Deserialize(grpc_byte_buffer* req, Status* status) { + virtual void* Deserialize(grpc_call* call, grpc_byte_buffer* req, + Status* status) { GPR_CODEGEN_ASSERT(req == nullptr); return nullptr; } diff --git a/include/grpcpp/impl/codegen/server_interceptor.h b/include/grpcpp/impl/codegen/server_interceptor.h index c39e9a988d..5fb5df28b7 100644 --- a/include/grpcpp/impl/codegen/server_interceptor.h +++ b/include/grpcpp/impl/codegen/server_interceptor.h @@ -22,7 +22,6 @@ #include <atomic> #include <vector> -#include <grpc/impl/codegen/log.h> #include <grpcpp/impl/codegen/interceptor.h> #include <grpcpp/impl/codegen/string_ref.h> diff --git a/include/grpcpp/impl/codegen/server_interface.h b/include/grpcpp/impl/codegen/server_interface.h index 92c87a5f7e..3967e96cfe 100644 --- a/include/grpcpp/impl/codegen/server_interface.h +++ b/include/grpcpp/impl/codegen/server_interface.h @@ -333,7 +333,12 @@ class ServerInterface : public internal::CallHook { } private: - virtual const std::vector< + // EXPERIMENTAL + // Getter method for the vector of interceptor factory objects. + // Returns a nullptr (rather than being pure) since this is a new method and + // adding a new pure method to an interface would be a breaking change (even + // though this is private and non-API) + virtual std::vector< std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>* interceptor_creators() { return nullptr; diff --git a/include/grpcpp/server.h b/include/grpcpp/server.h index 2b89ffd317..82d60b0218 100644 --- a/include/grpcpp/server.h +++ b/include/grpcpp/server.h @@ -191,8 +191,7 @@ class Server : public ServerInterface, private GrpcLibraryCodegen { grpc_server* server() override { return server_; }; private: - const std::vector< - std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>* + std::vector<std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>* interceptor_creators() override { return &interceptor_creators_; } @@ -226,6 +225,14 @@ class Server : public ServerInterface, private GrpcLibraryCodegen { ServerInitializer* initializer(); + // A vector of interceptor factory objects. + // This should be destroyed after health_check_service_ and this requirement + // is satisfied by declaring interceptor_creators_ before + // health_check_service_. (C++ mandates that member objects be destroyed in + // the reverse order of initialization.) + std::vector<std::unique_ptr<experimental::ServerInterceptorFactoryInterface>> + interceptor_creators_; + const int max_receive_message_size_; /// The following completion queues are ONLY used in case of Sync API @@ -261,9 +268,6 @@ class Server : public ServerInterface, private GrpcLibraryCodegen { // A special handler for resource exhausted in sync case std::unique_ptr<internal::MethodHandler> resource_exhausted_handler_; - - std::vector<std::unique_ptr<experimental::ServerInterceptorFactoryInterface>> - interceptor_creators_; }; } // namespace grpc diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index daf1b89b09..91894689c3 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -2951,6 +2951,27 @@ static void apply_service_config_to_call_locked(grpc_call_element* elem) { } } +// If the channel is in TRANSIENT_FAILURE and the call is not +// wait_for_ready=true, fails the call and returns true. +static bool fail_call_if_in_transient_failure(grpc_call_element* elem) { + channel_data* chand = static_cast<channel_data*>(elem->channel_data); + call_data* calld = static_cast<call_data*>(elem->call_data); + grpc_transport_stream_op_batch* batch = calld->pending_batches[0].batch; + if (grpc_connectivity_state_check(&chand->state_tracker) == + GRPC_CHANNEL_TRANSIENT_FAILURE && + (batch->payload->send_initial_metadata.send_initial_metadata_flags & + GRPC_INITIAL_METADATA_WAIT_FOR_READY) == 0) { + pending_batches_fail( + elem, + grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "channel is in state TRANSIENT_FAILURE"), + GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE), + true /* yield_call_combiner */); + return true; + } + return false; +} + // Invoked once resolver results are available. static void process_service_config_and_start_lb_pick_locked( grpc_call_element* elem) { @@ -2958,6 +2979,9 @@ static void process_service_config_and_start_lb_pick_locked( // Only get service config data on the first attempt. if (GPR_LIKELY(calld->num_attempts_completed == 0)) { apply_service_config_to_call_locked(elem); + // Check this after applying service config, since it may have + // affected the call's wait_for_ready value. + if (fail_call_if_in_transient_failure(elem)) return; } // Start LB pick. grpc_core::LbPicker::StartLocked(elem); @@ -3127,6 +3151,16 @@ static void start_pick_locked(void* arg, grpc_error* ignored) { // We do not yet have an LB policy, so wait for a resolver result. if (GPR_UNLIKELY(!chand->started_resolving)) { start_resolving_locked(chand); + } else { + // Normally, we want to do this check in + // process_service_config_and_start_lb_pick_locked(), so that we + // can honor the wait_for_ready setting in the service config. + // However, if the channel is in TRANSIENT_FAILURE at this point, that + // means that the resolver has returned a failure, so we're not going + // to get a service config right away. In that case, we fail the + // call now based on the wait_for_ready value passed in from the + // application. + if (fail_call_if_in_transient_failure(elem)) return; } // Create a new waiter, which will delete itself when done. grpc_core::New<grpc_core::ResolverResultWaiter>(elem); diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index 6855246fa4..287bc0454e 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -37,6 +37,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/handshaker.h" #include "src/core/lib/channel/handshaker_registry.h" +#include "src/core/lib/gpr/host_port.h" #include "src/core/lib/iomgr/endpoint.h" #include "src/core/lib/iomgr/resolve_address.h" #include "src/core/lib/iomgr/resource_quota.h" @@ -366,8 +367,14 @@ grpc_error* grpc_chttp2_server_add_port(grpc_server* server, const char* addr, arg = grpc_channel_args_find(args, GRPC_ARG_ENABLE_CHANNELZ); if (grpc_channel_arg_get_bool(arg, false)) { + char* host; + char* port; + gpr_split_host_port(addr, &host, &port); + // allocated host's ownership is passed to ListenSocketNode. state->channelz_listen_socket = - grpc_core::MakeRefCounted<grpc_core::channelz::ListenSocketNode>(); + grpc_core::MakeRefCounted<grpc_core::channelz::ListenSocketNode>( + grpc_core::UniquePtr<char>(host), *port_num); + gpr_free(port); socket_uuid = state->channelz_listen_socket->uuid(); } diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index 33577d890a..032654b861 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -374,7 +374,8 @@ grpc_json* SocketNode::RenderJson() { return top_level_json; } -ListenSocketNode::ListenSocketNode() : BaseNode(EntityType::kSocket) {} +ListenSocketNode::ListenSocketNode(UniquePtr<char> host, int port) + : BaseNode(EntityType::kSocket), host_(std::move(host)), port_(port) {} grpc_json* ListenSocketNode::RenderJson() { // We need to track these three json objects to build our object @@ -388,6 +389,21 @@ grpc_json* ListenSocketNode::RenderJson() { json_iterator = nullptr; json_iterator = grpc_json_add_number_string_child(json, json_iterator, "socketId", uuid()); + json = top_level_json; + json_iterator = nullptr; + json_iterator = grpc_json_create_child(json_iterator, json, "local", nullptr, + GRPC_JSON_OBJECT, false); + json = json_iterator; + json_iterator = nullptr; + json_iterator = grpc_json_create_child(json_iterator, json, "tcpip_address", + nullptr, GRPC_JSON_OBJECT, false); + json = json_iterator; + json_iterator = nullptr; + json_iterator = + grpc_json_add_number_string_child(json, json_iterator, "port", port_); + json_iterator = grpc_json_create_child(json_iterator, json, "ip_address", + host_.get(), GRPC_JSON_STRING, false); + return top_level_json; } diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index 88551befc8..8e66623142 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -268,10 +268,15 @@ class SocketNode : public BaseNode { // Handles channelz bookkeeping for listen sockets class ListenSocketNode : public BaseNode { public: - ListenSocketNode(); + // ListenSocketNode takes ownership of host. + ListenSocketNode(UniquePtr<char> host, int port); ~ListenSocketNode() override {} grpc_json* RenderJson() override; + + private: + UniquePtr<char> host_; + int port_; }; // Creation functions diff --git a/src/cpp/server/server_cc.cc b/src/cpp/server/server_cc.cc index 59a531e272..6f911e1392 100644 --- a/src/cpp/server/server_cc.cc +++ b/src/cpp/server/server_cc.cc @@ -256,7 +256,8 @@ class Server::SyncRequest final : public internal::CompletionQueueTag { // Set interception point for RECV MESSAGE auto* handler = resources_ ? method_->handler() : server_->resource_exhausted_handler_.get(); - request_ = handler->Deserialize(request_payload_, &request_status_); + request_ = handler->Deserialize(call_.call(), request_payload_, + &request_status_); request_payload_ = nullptr; interceptor_methods_.AddInterceptionHookPoint( @@ -446,7 +447,8 @@ Server::Server( std::vector< std::unique_ptr<experimental::ServerInterceptorFactoryInterface>> interceptor_creators) - : max_receive_message_size_(max_receive_message_size), + : interceptor_creators_(std::move(interceptor_creators)), + max_receive_message_size_(max_receive_message_size), sync_server_cqs_(std::move(sync_server_cqs)), started_(false), shutdown_(false), @@ -454,8 +456,7 @@ Server::Server( has_generic_service_(false), server_(nullptr), server_initializer_(new ServerInitializer(this)), - health_check_service_disabled_(false), - interceptor_creators_(std::move(interceptor_creators)) { + health_check_service_disabled_(false) { g_gli_initializer.summon(); gpr_once_init(&g_once_init_callbacks, InitGlobalCallbacks); global_callbacks_ = g_callbacks; diff --git a/test/core/memory_usage/BUILD b/test/core/memory_usage/BUILD new file mode 100644 index 0000000000..2f4e60d177 --- /dev/null +++ b/test/core/memory_usage/BUILD @@ -0,0 +1,64 @@ +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_cc_test", "grpc_cc_binary", "grpc_package") + +grpc_package(name = "test/core/memory_usage") + +licenses(["notice"]) # Apache v2 + +load("//test/core/util:grpc_fuzzer.bzl", "grpc_fuzzer") + +grpc_cc_binary( + name = "client", + testonly = 1, + srcs = ["client.cc"], + language = "C++", + deps = [ + "//:gpr", + "//:grpc", + "//test/core/util:gpr_test_util", + "//test/core/util:grpc_test_util", + ], +) + +grpc_cc_binary( + name = "server", + testonly = 1, + srcs = ["server.cc"], + language = "C++", + deps = [ + "//:gpr", + "//:grpc", + "//test/core/util:gpr_test_util", + "//test/core/util:grpc_test_util", + "//test/core/end2end:ssl_test_data" + ], +) + +grpc_cc_test( + name = "memory_usage_test", + srcs = ["memory_usage_test.cc"], + language = "C++", + data = [ + ":client", + ":server", + ], + deps = [ + "//:gpr", + "//:grpc", + "//test/core/util:gpr_test_util", + "//test/core/util:grpc_test_util", + ], +) diff --git a/test/cpp/end2end/client_interceptors_end2end_test.cc b/test/cpp/end2end/client_interceptors_end2end_test.cc index 5720e87478..e8ffd46344 100644 --- a/test/cpp/end2end/client_interceptors_end2end_test.cc +++ b/test/cpp/end2end/client_interceptors_end2end_test.cc @@ -152,7 +152,9 @@ class HijackingInterceptor : public experimental::Interceptor { EchoRequest req; auto* buffer = methods->GetSendMessage(); auto copied_buffer = *buffer; - SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req); + EXPECT_TRUE( + SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req) + .ok()); EXPECT_EQ(req.message(), "Hello"); } if (methods->QueryInterceptionHookPoint( @@ -255,7 +257,9 @@ class HijackingInterceptorMakesAnotherCall : public experimental::Interceptor { EchoRequest req; auto* buffer = methods->GetSendMessage(); auto copied_buffer = *buffer; - SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req); + EXPECT_TRUE( + SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req) + .ok()); EXPECT_EQ(req.message(), "Hello"); req_ = req; stub_ = grpc::testing::EchoTestService::NewStub( @@ -367,7 +371,9 @@ class LoggingInterceptor : public experimental::Interceptor { EchoRequest req; auto* buffer = methods->GetSendMessage(); auto copied_buffer = *buffer; - SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req); + EXPECT_TRUE( + SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req) + .ok()); EXPECT_TRUE(req.message().find("Hello") == 0); } if (methods->QueryInterceptionHookPoint( diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index b05c60cc72..9218c85717 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/test/cpp/end2end/client_lb_end2end_test.cc @@ -218,13 +218,14 @@ class ClientLbEnd2endTest : public ::testing::Test { bool SendRpc( const std::unique_ptr<grpc::testing::EchoTestService::Stub>& stub, EchoResponse* response = nullptr, int timeout_ms = 1000, - Status* result = nullptr) { + Status* result = nullptr, bool wait_for_ready = false) { const bool local_response = (response == nullptr); if (local_response) response = new EchoResponse; EchoRequest request; request.set_message(kRequestMessage_); ClientContext context; context.set_deadline(grpc_timeout_milliseconds_to_deadline(timeout_ms)); + if (wait_for_ready) context.set_wait_for_ready(true); Status status = stub->Echo(&context, request, response); if (result != nullptr) *result = status; if (local_response) delete response; @@ -233,10 +234,11 @@ class ClientLbEnd2endTest : public ::testing::Test { void CheckRpcSendOk( const std::unique_ptr<grpc::testing::EchoTestService::Stub>& stub, - const grpc_core::DebugLocation& location) { + const grpc_core::DebugLocation& location, bool wait_for_ready = false) { EchoResponse response; Status status; - const bool success = SendRpc(stub, &response, 2000, &status); + const bool success = + SendRpc(stub, &response, 2000, &status, wait_for_ready); ASSERT_TRUE(success) << "From " << location.file() << ":" << location.line() << "\n" << "Error: " << status.error_message() << " " @@ -312,7 +314,7 @@ class ClientLbEnd2endTest : public ::testing::Test { if (ignore_failure) { SendRpc(stub); } else { - CheckRpcSendOk(stub, location); + CheckRpcSendOk(stub, location, true); } } while (servers_[server_idx]->service_.request_count() == 0); ResetCounters(); @@ -524,7 +526,7 @@ TEST_F(ClientLbEnd2endTest, PickFirstUpdates) { do { channel_state = channel->GetState(true /* try to connect */); } while (channel_state == GRPC_CHANNEL_READY); - GPR_ASSERT(channel_state != GRPC_CHANNEL_READY); + ASSERT_NE(channel_state, GRPC_CHANNEL_READY); servers_[0]->service_.ResetCounters(); // Next update introduces servers_[1], making the channel recover. @@ -841,7 +843,7 @@ TEST_F(ClientLbEnd2endTest, RoundRobinUpdates) { do { channel_state = channel->GetState(true /* try to connect */); } while (channel_state == GRPC_CHANNEL_READY); - GPR_ASSERT(channel_state != GRPC_CHANNEL_READY); + ASSERT_NE(channel_state, GRPC_CHANNEL_READY); servers_[0]->service_.ResetCounters(); // Next update introduces servers_[1], making the channel recover. @@ -850,7 +852,7 @@ TEST_F(ClientLbEnd2endTest, RoundRobinUpdates) { SetNextResolution(ports); WaitForServer(stub, 1, DEBUG_LOCATION); channel_state = channel->GetState(false /* try to connect */); - GPR_ASSERT(channel_state == GRPC_CHANNEL_READY); + ASSERT_EQ(channel_state, GRPC_CHANNEL_READY); // Check LB policy name for the channel. EXPECT_EQ("round_robin", channel->GetLoadBalancingPolicyName()); @@ -960,7 +962,7 @@ TEST_F(ClientLbEnd2endTest, RoundRobinReresolve) { if (SendRpc(stub)) break; now = gpr_now(GPR_CLOCK_MONOTONIC); } - GPR_ASSERT(gpr_time_cmp(deadline, now) > 0); + ASSERT_GT(gpr_time_cmp(deadline, now), 0); } TEST_F(ClientLbEnd2endTest, RoundRobinSingleReconnect) { diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc index b69b861fcf..6ce0696114 100644 --- a/test/cpp/end2end/grpclb_end2end_test.cc +++ b/test/cpp/end2end/grpclb_end2end_test.cc @@ -539,13 +539,15 @@ class GrpclbEnd2endTest : public ::testing::Test { balancers_.at(i)->add_response(response, delay_ms); } - Status SendRpc(EchoResponse* response = nullptr, int timeout_ms = 1000) { + Status SendRpc(EchoResponse* response = nullptr, int timeout_ms = 1000, + bool wait_for_ready = false) { const bool local_response = (response == nullptr); if (local_response) response = new EchoResponse; EchoRequest request; request.set_message(kRequestMessage_); ClientContext context; context.set_deadline(grpc_timeout_milliseconds_to_deadline(timeout_ms)); + if (wait_for_ready) context.set_wait_for_ready(true); Status status = stub_->Echo(&context, request, response); if (local_response) delete response; return status; @@ -1366,7 +1368,7 @@ TEST_F(SingleBalancerTest, DropAllFirst) { {}, {{"rate_limiting", num_of_drop_by_rate_limiting_addresses}, {"load_balancing", num_of_drop_by_load_balancing_addresses}}), 0); - const Status status = SendRpc(); + const Status status = SendRpc(nullptr, 1000, true); EXPECT_FALSE(status.ok()); EXPECT_EQ(status.error_message(), "Call dropped by load balancing policy"); } @@ -1391,7 +1393,7 @@ TEST_F(SingleBalancerTest, DropAll) { // fail. Status status; do { - status = SendRpc(); + status = SendRpc(nullptr, 1000, true); } while (status.ok()); EXPECT_FALSE(status.ok()); EXPECT_EQ(status.error_message(), "Call dropped by load balancing policy"); diff --git a/test/cpp/end2end/server_interceptors_end2end_test.cc b/test/cpp/end2end/server_interceptors_end2end_test.cc index 44ba2a6009..e08a4493d3 100644 --- a/test/cpp/end2end/server_interceptors_end2end_test.cc +++ b/test/cpp/end2end/server_interceptors_end2end_test.cc @@ -103,7 +103,9 @@ class LoggingInterceptor : public experimental::Interceptor { EchoRequest req; auto* buffer = methods->GetSendMessage(); auto copied_buffer = *buffer; - SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req); + EXPECT_TRUE( + SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req) + .ok()); EXPECT_TRUE(req.message().find("Hello") == 0); } if (methods->QueryInterceptionHookPoint( diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 9d7d487e0d..042856146d 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -4724,6 +4724,28 @@ "deps": [ "gpr", "gpr_test_util", + "grpc", + "grpc++", + "grpc++_test_util", + "grpc_test_util" + ], + "headers": [ + "test/cpp/end2end/interceptors_util.h" + ], + "is_filegroup": false, + "language": "c++", + "name": "server_interceptors_end2end_test", + "src": [ + "test/cpp/end2end/interceptors_util.h", + "test/cpp/end2end/server_interceptors_end2end_test.cc" + ], + "third_party": false, + "type": "target" + }, + { + "deps": [ + "gpr", + "gpr_test_util", "grpc++_test_util_unsecure", "grpc++_unsecure", "grpc_test_util_unsecure", diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 1052666634..ef34cd6556 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -5150,6 +5150,30 @@ "posix", "windows" ], + "cpu_cost": 0.5, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "server_interceptors_end2end_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": true + }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], "cpu_cost": 1.0, "exclude_configs": [], "exclude_iomgrs": [], |