From 1c2332b0c1edfdcd5746939feaabc0904475d7ea Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 24 Apr 2017 09:13:35 -0700 Subject: [PATCH 1/4] Fix potential memory leak, cleanup some code --- .../message_compress_filter.c | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/core/ext/filters/http/message_compress/message_compress_filter.c b/src/core/ext/filters/http/message_compress/message_compress_filter.c index 4f5f41e9b04..1da8cf69cbf 100644 --- a/src/core/ext/filters/http/message_compress/message_compress_filter.c +++ b/src/core/ext/filters/http/message_compress/message_compress_filter.c @@ -277,13 +277,10 @@ static void compress_start_transport_stream_op_batch( GPR_TIMER_BEGIN("compress_start_transport_stream_op_batch", 0); if (op->cancel_stream) { - gpr_atm cur; GRPC_ERROR_REF(op->payload->cancel_stream.cancel_error); - do { - cur = gpr_atm_acq_load(&calld->send_initial_metadata_state); - } while (!gpr_atm_rel_cas( - &calld->send_initial_metadata_state, cur, - CANCELLED_BIT | (gpr_atm)op->payload->cancel_stream.cancel_error)); + gpr_atm cur = gpr_atm_full_xchg( + &calld->send_initial_metadata_state, + CANCELLED_BIT | (gpr_atm)op->payload->cancel_stream.cancel_error); switch (cur) { case HAS_COMPRESSION_ALGORITHM: case NO_COMPRESSION_ALGORITHM: @@ -311,13 +308,18 @@ static void compress_start_transport_stream_op_batch( grpc_transport_stream_op_batch_finish_with_failure(exec_ctx, op, error); return; } - gpr_atm cur = gpr_atm_acq_load(&calld->send_initial_metadata_state); + gpr_atm cur; + retry_send_im: + cur = gpr_atm_acq_load(&calld->send_initial_metadata_state); GPR_ASSERT(cur != HAS_COMPRESSION_ALGORITHM && cur != NO_COMPRESSION_ALGORITHM); if ((cur & CANCELLED_BIT) == 0) { - gpr_atm_rel_store(&calld->send_initial_metadata_state, - has_compression_algorithm ? HAS_COMPRESSION_ALGORITHM - : NO_COMPRESSION_ALGORITHM); + if (!gpr_atm_rel_cas(&calld->send_initial_metadata_state, cur, + has_compression_algorithm + ? HAS_COMPRESSION_ALGORITHM + : NO_COMPRESSION_ALGORITHM)) { + goto retry_send_im; + } if (cur != INITIAL_METADATA_UNSEEN) { grpc_call_next_op(exec_ctx, elem, (grpc_transport_stream_op_batch *)cur); From 35ee7e7967d390c269ae82367a3667b10bfa8d9b Mon Sep 17 00:00:00 2001 From: Yong Ni Date: Wed, 26 Apr 2017 17:45:45 -0700 Subject: [PATCH 2/4] Added verbose option to run_interop_test to ease tracing the commands invoked --- tools/run_tests/python_utils/jobset.py | 5 +++++ tools/run_tests/run_interop_tests.py | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/tools/run_tests/python_utils/jobset.py b/tools/run_tests/python_utils/jobset.py index 460f359cf35..d01e82a76b1 100755 --- a/tools/run_tests/python_utils/jobset.py +++ b/tools/run_tests/python_utils/jobset.py @@ -208,6 +208,11 @@ class JobSpec(object): def __repr__(self): return 'JobSpec(shortname=%s, cmdline=%s)' % (self.shortname, self.cmdline) + def __str__(self): + return '%s: %s %s' % (self.shortname, + ' '.join('%s=%s' % kv for kv in self.environ.items()), + ' '.join(self.cmdline)) + class JobResult(object): def __init__(self): diff --git a/tools/run_tests/run_interop_tests.py b/tools/run_tests/run_interop_tests.py index 44e93eb9cc3..867d9e6f7bf 100755 --- a/tools/run_tests/run_interop_tests.py +++ b/tools/run_tests/run_interop_tests.py @@ -888,6 +888,10 @@ argp.add_argument('-t', '--travis', default=False, action='store_const', const=True) +argp.add_argument('-v', '--verbose', + default=False, + action='store_const', + const=True) argp.add_argument('--use_docker', default=False, action='store_const', @@ -989,6 +993,9 @@ if args.use_docker: if build_jobs: jobset.message('START', 'Building interop docker images.', do_newline=True) + if args.verbose: + print('Jobs to run: \n%s\n' % '\n'.join(str(j) for j in build_jobs)) + num_failures, _ = jobset.run( build_jobs, newline_on_success=True, maxjobs=args.jobs) if num_failures == 0: @@ -1164,6 +1171,9 @@ try: if args.manual_run: print('All tests will skipped --manual_run option is active.') + if args.verbose: + print('Jobs to run: \n%s\n' % '\n'.join(str(job) for job in jobs)) + num_failures, resultset = jobset.run(jobs, newline_on_success=True, maxjobs=args.jobs, skip_jobs=args.manual_run) From 75d2496ba81271c9bb989c5561851f46fb18d2f0 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 27 Apr 2017 19:05:33 -0700 Subject: [PATCH 3/4] Add gflags to Mac build from source instructions Note that gflags is already listed as something to install under Linux before build from source and running tests but was not included on Mac. --- INSTALL.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/INSTALL.md b/INSTALL.md index 29f0060c81a..6cfa1b6cbaf 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -46,6 +46,11 @@ packages, which you can get from [Homebrew](https://brew.sh): $ brew install autoconf automake libtool shtool ``` +If you plan to build from source and run tests, install the following as well: +```sh + $ brew install gflags +``` + ## Protoc By default gRPC uses [protocol buffers](https://github.com/google/protobuf), From 97106a7ef2f6c7ce0754bd71778aeb779343ae4c Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Fri, 28 Apr 2017 13:11:11 -0700 Subject: [PATCH 4/4] Removed 3rd party dep on thrift --- .gitmodules | 3 - .../grpc++/impl/codegen/thrift_serializer.h | 217 ------------------ include/grpc++/impl/codegen/thrift_utils.h | 83 ------- third_party/thrift | 1 - 4 files changed, 304 deletions(-) delete mode 100644 include/grpc++/impl/codegen/thrift_serializer.h delete mode 100644 include/grpc++/impl/codegen/thrift_utils.h delete mode 160000 third_party/thrift diff --git a/.gitmodules b/.gitmodules index 0f003693e43..144fd080ac9 100644 --- a/.gitmodules +++ b/.gitmodules @@ -14,9 +14,6 @@ [submodule "third_party/boringssl"] path = third_party/boringssl url = https://github.com/google/boringssl.git -[submodule "third_party/thrift"] - path = third_party/thrift - url = https://github.com/apache/thrift.git [submodule "third_party/benchmark"] path = third_party/benchmark url = https://github.com/google/benchmark diff --git a/include/grpc++/impl/codegen/thrift_serializer.h b/include/grpc++/impl/codegen/thrift_serializer.h deleted file mode 100644 index 86bc7105c0e..00000000000 --- a/include/grpc++/impl/codegen/thrift_serializer.h +++ /dev/null @@ -1,217 +0,0 @@ -/* - * - * Copyright 2016, Google Inc. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - */ - -#ifndef GRPCXX_IMPL_CODEGEN_THRIFT_SERIALIZER_H -#define GRPCXX_IMPL_CODEGEN_THRIFT_SERIALIZER_H - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace apache { -namespace thrift { -namespace util { - -using apache::thrift::protocol::TBinaryProtocolT; -using apache::thrift::protocol::TCompactProtocolT; -using apache::thrift::protocol::TMessageType; -using apache::thrift::protocol::TNetworkBigEndian; -using apache::thrift::transport::TMemoryBuffer; -using apache::thrift::transport::TBufferBase; -using apache::thrift::transport::TTransport; - -template -class ThriftSerializer { - public: - ThriftSerializer() - : prepared_(false), - last_deserialized_(false), - serialize_version_(false) {} - - virtual ~ThriftSerializer() {} - - // Serialize the passed type into the internal buffer - // and returns a pointer to internal buffer and its size - template - void Serialize(const T& fields, const uint8_t** serialized_buffer, - size_t* serialized_len) { - // prepare or reset buffer - if (!prepared_ || last_deserialized_) { - prepare(); - } else { - buffer_->resetBuffer(); - } - last_deserialized_ = false; - - // if required serialize protocol version - if (serialize_version_) { - protocol_->writeMessageBegin("", TMessageType(0), 0); - } - - // serialize fields into buffer - fields.write(protocol_.get()); - - // write the end of message - if (serialize_version_) { - protocol_->writeMessageEnd(); - } - - uint8_t* byte_buffer; - uint32_t byte_buffer_size; - buffer_->getBuffer(&byte_buffer, &byte_buffer_size); - *serialized_buffer = byte_buffer; - *serialized_len = byte_buffer_size; - } - - // Serialize the passed type into the byte buffer - template - void Serialize(const T& fields, grpc_byte_buffer** bp) { - const uint8_t* byte_buffer; - size_t byte_buffer_size; - - Serialize(fields, &byte_buffer, &byte_buffer_size); - - grpc_slice slice = grpc_slice_from_copied_buffer( - reinterpret_cast(byte_buffer), byte_buffer_size); - - *bp = grpc_raw_byte_buffer_create(&slice, 1); - - grpc_slice_unref(slice); - } - - // Deserialize the passed char array into the passed type, returns the number - // of bytes that have been consumed from the passed string. - template - uint32_t Deserialize(uint8_t* serialized_buffer, size_t length, T* fields) { - // prepare buffer if necessary - if (!prepared_) { - prepare(); - } - last_deserialized_ = true; - - // reset buffer transport - buffer_->resetBuffer(serialized_buffer, length); - - // read the protocol version if necessary - if (serialize_version_) { - std::string name = ""; - TMessageType mt = static_cast(0); - int32_t seq_id = 0; - protocol_->readMessageBegin(name, mt, seq_id); - } - - // deserialize buffer into fields - uint32_t len = fields->read(protocol_.get()); - - // read the end of message - if (serialize_version_) { - protocol_->readMessageEnd(); - } - - return len; - } - - // Deserialize the passed byte buffer to passed type, returns the number - // of bytes consumed from byte buffer - template - uint32_t Deserialize(grpc_byte_buffer* buffer, T* msg) { - grpc_byte_buffer_reader reader; - grpc_byte_buffer_reader_init(&reader, buffer); - - grpc_slice slice = grpc_byte_buffer_reader_readall(&reader); - - uint32_t len = - Deserialize(GRPC_SLICE_START_PTR(slice), GRPC_SLICE_LENGTH(slice), msg); - - grpc_slice_unref(slice); - - grpc_byte_buffer_reader_destroy(&reader); - - return len; - } - - // set serialization version flag - void SetSerializeVersion(bool value) { serialize_version_ = value; } - - // Set the container size limit to deserialize - // This function should be called after buffer_ is initialized - void SetContainerSizeLimit(int32_t container_limit) { - if (!prepared_) { - prepare(); - } - protocol_->setContainerSizeLimit(container_limit); - } - - // Set the string size limit to deserialize - // This function should be called after buffer_ is initialized - void SetStringSizeLimit(int32_t string_limit) { - if (!prepared_) { - prepare(); - } - protocol_->setStringSizeLimit(string_limit); - } - - private: - bool prepared_; - bool last_deserialized_; - boost::shared_ptr buffer_; - std::shared_ptr protocol_; - bool serialize_version_; - - void prepare() { - buffer_ = boost::make_shared(); - // create a protocol for the memory buffer transport - protocol_ = std::make_shared(buffer_); - prepared_ = true; - } - -}; // ThriftSerializer - -typedef ThriftSerializer> - ThriftSerializerBinary; -typedef ThriftSerializer> - ThriftSerializerCompact; - -} // namespace util -} // namespace thrift -} // namespace apache - -#endif // GRPCXX_IMPL_CODEGEN_THRIFT_SERIALIZER_H diff --git a/include/grpc++/impl/codegen/thrift_utils.h b/include/grpc++/impl/codegen/thrift_utils.h deleted file mode 100644 index 742d7397035..00000000000 --- a/include/grpc++/impl/codegen/thrift_utils.h +++ /dev/null @@ -1,83 +0,0 @@ -/* - * - * Copyright 2016, Google Inc. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - */ - -#ifndef GRPCXX_IMPL_CODEGEN_THRIFT_UTILS_H -#define GRPCXX_IMPL_CODEGEN_THRIFT_UTILS_H - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace grpc { - -using apache::thrift::util::ThriftSerializerCompact; - -template -class SerializationTraits::value>::type> { - public: - static Status Serialize(const T& msg, grpc_byte_buffer** bp, - bool* own_buffer) { - *own_buffer = true; - - ThriftSerializerCompact serializer; - serializer.Serialize(msg, bp); - - return Status(StatusCode::OK, "ok"); - } - - static Status Deserialize(grpc_byte_buffer* buffer, T* msg, - int max_receive_message_size) { - if (!buffer) { - return Status(StatusCode::INTERNAL, "No payload"); - } - - ThriftSerializerCompact deserializer; - deserializer.Deserialize(buffer, msg); - - grpc_byte_buffer_destroy(buffer); - - return Status(StatusCode::OK, "ok"); - } -}; - -} // namespace grpc - -#endif // GRPCXX_IMPL_CODEGEN_THRIFT_UTILS_H diff --git a/third_party/thrift b/third_party/thrift deleted file mode 160000 index bcad91771b7..00000000000 --- a/third_party/thrift +++ /dev/null @@ -1 +0,0 @@ -Subproject commit bcad91771b7f0bff28a1cac1981d7ef2b9bcef3c