From e64c2779be14a1ce4592a98695ad88b5e5ad6312 Mon Sep 17 00:00:00 2001 From: Feng Li Date: Mon, 13 Nov 2017 13:26:44 -0800 Subject: [PATCH 1/2] Remove the client liveness detect timer when the downstream request/connection has been finalized. Fix memory leaks in proto/stream body decoder. Support clean shutdown for grpc lib and protobuf lib. --- net/grpc/gateway/backend/grpc_backend.cc | 10 +++++++--- net/grpc/gateway/codec/proto_decoder.cc | 4 ++-- net/grpc/gateway/codec/stream_body_decoder.cc | 2 +- net/grpc/gateway/frontend/nginx_http_frontend.cc | 6 ++++-- net/grpc/gateway/frontend/nginx_http_frontend.h | 4 ++-- net/grpc/gateway/runtime/runtime.cc | 3 +++ 6 files changed, 19 insertions(+), 10 deletions(-) diff --git a/net/grpc/gateway/backend/grpc_backend.cc b/net/grpc/gateway/backend/grpc_backend.cc index 92b2c79..7daf91b 100644 --- a/net/grpc/gateway/backend/grpc_backend.cc +++ b/net/grpc/gateway/backend/grpc_backend.cc @@ -48,6 +48,10 @@ GrpcBackend::GrpcBackend() GrpcBackend::~GrpcBackend() { BACKEND_DEBUG("Deleting GRPC backend proxy."); + for (auto& m : request_initial_metadata_) { + grpc_slice_unref(m.key); + grpc_slice_unref(m.value); + } grpc_metadata_array_destroy(&response_initial_metadata_); grpc_metadata_array_destroy(&response_trailing_metadata_); if (request_buffer_ != nullptr) { @@ -171,9 +175,9 @@ void GrpcBackend::OnResponseMessage(bool result) { grpc_slice slice; while (grpc_byte_buffer_reader_next(&reader, &slice)) { message->push_back(Slice(slice, Slice::STEAL_REF)); - grpc_slice_unref(slice); } grpc_byte_buffer_reader_destroy(&reader); + grpc_byte_buffer_destroy(response_buffer_); response->set_message(std::move(message)); frontend()->Send(std::move(response)); @@ -234,8 +238,8 @@ void GrpcBackend::Send(std::unique_ptr request, Tag* on_done) { continue; } grpc_metadata initial_metadata; - initial_metadata.key = grpc_slice_intern( - grpc_slice_from_copied_string(header.first.c_str())); + initial_metadata.key = + grpc_slice_from_copied_string(header.first.c_str()); initial_metadata.value = grpc_slice_from_copied_buffer( header.second.data(), header.second.size()); initial_metadata.flags = 0; diff --git a/net/grpc/gateway/codec/proto_decoder.cc b/net/grpc/gateway/codec/proto_decoder.cc index 5ff7107..4395f1a 100644 --- a/net/grpc/gateway/codec/proto_decoder.cc +++ b/net/grpc/gateway/codec/proto_decoder.cc @@ -12,8 +12,8 @@ ProtoDecoder::~ProtoDecoder() {} Status ProtoDecoder::Decode() { if (inputs()->empty()) { - Slice* slice = new Slice(grpc_empty_slice(), Slice::STEAL_REF); - ByteBuffer* buffer = new ByteBuffer(slice, 1); + Slice slice(grpc_empty_slice(), Slice::STEAL_REF); + ByteBuffer* buffer = new ByteBuffer(&slice, 1); results()->push_back(std::unique_ptr(buffer)); } else { ByteBuffer* buffer = new ByteBuffer(inputs()->data(), inputs()->size()); diff --git a/net/grpc/gateway/codec/stream_body_decoder.cc b/net/grpc/gateway/codec/stream_body_decoder.cc index 4759d25..946cdf7 100644 --- a/net/grpc/gateway/codec/stream_body_decoder.cc +++ b/net/grpc/gateway/codec/stream_body_decoder.cc @@ -49,7 +49,7 @@ Status StreamBodyDecoder::Decode() { if (varint_value_ == 0) { buffer_.reset(new Slice(grpc_empty_slice(), Slice::STEAL_REF)); results()->push_back(std::unique_ptr( - new ByteBuffer(buffer_.release(), 1))); + new ByteBuffer(buffer_.get(), 1))); state_ = EXPECTING_MESSAGE_KEY_TYPE; } else { grpc_slice slice = grpc_slice_malloc(varint_value_); diff --git a/net/grpc/gateway/frontend/nginx_http_frontend.cc b/net/grpc/gateway/frontend/nginx_http_frontend.cc index 2d8de7d..06dfe28 100644 --- a/net/grpc/gateway/frontend/nginx_http_frontend.cc +++ b/net/grpc/gateway/frontend/nginx_http_frontend.cc @@ -43,8 +43,10 @@ grpc::gateway::Frontend *get_frontend(ngx_http_request_t *r) { void grpc_gateway_request_cleanup_handler(void *data) { grpc_gateway_request_context *context = static_cast(data); - static_cast(context->frontend.get()) - ->set_http_request(nullptr); + auto *frontend = + static_cast(context->frontend.get()); + frontend->StopClientLivenessDetection(); + frontend->set_http_request(nullptr); context->frontend.reset(); } diff --git a/net/grpc/gateway/frontend/nginx_http_frontend.h b/net/grpc/gateway/frontend/nginx_http_frontend.h index 010d5b0..9a697a4 100644 --- a/net/grpc/gateway/frontend/nginx_http_frontend.h +++ b/net/grpc/gateway/frontend/nginx_http_frontend.h @@ -81,6 +81,8 @@ class NginxHttpFrontend : public Frontend { void OnClientLivenessDetectionEvent(ngx_event_t *event); + void StopClientLivenessDetection(); + private: friend void ::continue_read_request_body(ngx_http_request_t *r); @@ -116,8 +118,6 @@ class NginxHttpFrontend : public Frontend { void WriteToNginxResponse(uint8_t *data, size_t size); - void StopClientLivenessDetection(); - ngx_http_request_t *http_request_; std::unique_ptr decoder_; std::unique_ptr encoder_; diff --git a/net/grpc/gateway/runtime/runtime.cc b/net/grpc/gateway/runtime/runtime.cc index bd2490d..f4921b8 100644 --- a/net/grpc/gateway/runtime/runtime.cc +++ b/net/grpc/gateway/runtime/runtime.cc @@ -23,6 +23,7 @@ #include "net/grpc/gateway/runtime/constants.h" #include "third_party/grpc/include/grpc++/support/config.h" #include "third_party/grpc/include/grpc/grpc.h" +#include "third_party/protobuf/include/google/protobuf/stubs/common.h" namespace grpc { namespace gateway { @@ -78,6 +79,8 @@ void Runtime::Shutdown() { grpc_channel_destroy(entry.second); } grpc_backend_channels_.clear(); + grpc_shutdown(); + google::protobuf::ShutdownProtobufLibrary(); } std::shared_ptr Runtime::CreateNginxFrontend( From 24ff395bb4c376ef700fd154e23a36ff43388a13 Mon Sep 17 00:00:00 2001 From: Feng Li Date: Mon, 13 Nov 2017 14:16:04 -0800 Subject: [PATCH 2/2] Change gRPC-Web make file to add the protobuf include directory. --- Makefile | 5 +++-- net/grpc/gateway/runtime/runtime.cc | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index ec1250d..2c2cc5d 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,7 @@ OS := $(shell uname) CC := g++ ROOT_DIR := $(shell pwd) GRPC_GATEWAY_PROTOS := $(ROOT_DIR)/net/grpc/gateway/protos +PROTO_INC := $(ROOT_DIR)/third_party/grpc/third_party/protobuf/include PROTO_SRC := $(ROOT_DIR)/third_party/grpc/third_party/protobuf/src PROTO_LIB := $(PROTO_SRC)/.libs PROTOC := $(PROTO_SRC)/protoc @@ -41,7 +42,7 @@ nginx_config: auto/configure \ --with-http_ssl_module \ --with-http_v2_module \ - --with-cc-opt="-I /usr/local/include -I $(ROOT_DIR) -I $(PROTO_SRC) \ + --with-cc-opt="-I /usr/local/include -I $(ROOT_DIR) -I $(PROTO_INC) -I $(PROTO_SRC) \ -I $(GRPC_INC) -I $(GRPC_SRC)" \ --with-ld-opt="$(NGINX_LD_OPT)" \ --with-openssl="$(ROOT_DIR)/third_party/openssl" \ @@ -52,7 +53,7 @@ nginx_config_static: auto/configure \ --with-http_ssl_module \ --with-http_v2_module \ - --with-cc-opt="-I /usr/local/include -I $(ROOT_DIR) -I $(PROTO_SRC) \ + --with-cc-opt="-I /usr/local/include -I $(ROOT_DIR) -I $(PROTO_INC) -I $(PROTO_SRC) \ -I $(GRPC_INC) -I $(GRPC_SRC)" \ --with-ld-opt="$(NGINX_STATIC_LD_OPT)" \ --with-openssl="$(ROOT_DIR)/third_party/openssl" \ diff --git a/net/grpc/gateway/runtime/runtime.cc b/net/grpc/gateway/runtime/runtime.cc index f4921b8..e843b1d 100644 --- a/net/grpc/gateway/runtime/runtime.cc +++ b/net/grpc/gateway/runtime/runtime.cc @@ -4,6 +4,7 @@ #include #include +#include "google/protobuf/stubs/common.h" #include "net/grpc/gateway/backend/grpc_backend.h" #include "net/grpc/gateway/codec/b64_proto_decoder.h" #include "net/grpc/gateway/codec/b64_proto_encoder.h" @@ -23,7 +24,6 @@ #include "net/grpc/gateway/runtime/constants.h" #include "third_party/grpc/include/grpc++/support/config.h" #include "third_party/grpc/include/grpc/grpc.h" -#include "third_party/protobuf/include/google/protobuf/stubs/common.h" namespace grpc { namespace gateway {