diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 8445c5551b905..d7cd3fe1a0509 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -5,6 +5,7 @@ Version history ================ Changes ------- +* listener: fix crash when disabling or re-enabling listeners due to overload while processing LDS updates. 1.14.5 (September 29, 2020) =========================== diff --git a/source/common/common/utility.cc b/source/common/common/utility.cc index 7a3656f47ba72..d53679d6bd2b5 100644 --- a/source/common/common/utility.cc +++ b/source/common/common/utility.cc @@ -283,7 +283,7 @@ bool StringUtil::findToken(absl::string_view source, absl::string_view delimiter absl::string_view key_token, bool trim_whitespace) { const auto tokens = splitToken(source, delimiters, trim_whitespace); if (trim_whitespace) { - for (const auto token : tokens) { + for (const auto& token : tokens) { if (key_token == trim(token)) { return true; } diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index fe9393574a327..4c59a4f34e0c8 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -310,7 +310,7 @@ std::string Utility::parseCookieValue(const HeaderMap& headers, const std::strin if (header.key() == Http::Headers::get().Cookie.get()) { // Split the cookie header into individual cookies. - for (const auto s : StringUtil::splitToken(header.value().getStringView(), ";")) { + for (const auto& s : StringUtil::splitToken(header.value().getStringView(), ";")) { // Find the key part of the cookie (i.e. the name of the cookie). size_t first_non_space = s.find_first_not_of(" "); size_t equals_index = s.find('='); diff --git a/source/common/router/retry_state_impl.cc b/source/common/router/retry_state_impl.cc index 2952b7a341573..1dd8b36c1583c 100644 --- a/source/common/router/retry_state_impl.cc +++ b/source/common/router/retry_state_impl.cc @@ -114,7 +114,7 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, } if (request_headers.EnvoyRetriableStatusCodes()) { - for (const auto code : StringUtil::splitToken( + for (const auto& code : StringUtil::splitToken( request_headers.EnvoyRetriableStatusCodes()->value().getStringView(), ",")) { unsigned int out; if (absl::SimpleAtoi(code, &out)) { @@ -129,7 +129,7 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, // to provide HeaderMatcher serialized into a string. To avoid this extra // complexity we only support name-only header matchers via request // header. Anything more sophisticated needs to be provided via config. - for (const auto header_name : StringUtil::splitToken( + for (const auto& header_name : StringUtil::splitToken( request_headers.EnvoyRetriableHeaderNames()->value().getStringView(), ",")) { envoy::config::route::v3::HeaderMatcher header_matcher; header_matcher.set_name(std::string(absl::StripAsciiWhitespace(header_name))); @@ -153,7 +153,7 @@ void RetryStateImpl::enableBackoffTimer() { std::pair RetryStateImpl::parseRetryOn(absl::string_view config) { uint32_t ret = 0; bool all_fields_valid = true; - for (const auto retry_on : StringUtil::splitToken(config, ",", false, true)) { + for (const auto& retry_on : StringUtil::splitToken(config, ",", false, true)) { if (retry_on == Http::Headers::get().EnvoyRetryOnValues._5xx) { ret |= RetryPolicy::RETRY_ON_5XX; } else if (retry_on == Http::Headers::get().EnvoyRetryOnValues.GatewayError) { @@ -181,7 +181,7 @@ std::pair RetryStateImpl::parseRetryOn(absl::string_view config) std::pair RetryStateImpl::parseRetryGrpcOn(absl::string_view retry_grpc_on_header) { uint32_t ret = 0; bool all_fields_valid = true; - for (const auto retry_on : StringUtil::splitToken(retry_grpc_on_header, ",", false, true)) { + for (const auto& retry_on : StringUtil::splitToken(retry_grpc_on_header, ",", false, true)) { if (retry_on == Http::Headers::get().EnvoyRetryOnGrpcValues.Cancelled) { ret |= RetryPolicy::RETRY_ON_GRPC_CANCELLED; } else if (retry_on == Http::Headers::get().EnvoyRetryOnGrpcValues.DeadlineExceeded) { diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index 1ed9faf2c4322..3294d3f669fa9 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -406,7 +406,7 @@ void DiskLayer::walkDirectory(const std::string& path, const std::string& prefix // Comments are useful for placeholder files with no value. const std::string text_file{api.fileSystem().fileReadToEnd(full_path)}; const auto lines = StringUtil::splitToken(text_file, "\n"); - for (const auto line : lines) { + for (const auto& line : lines) { if (!line.empty() && line.front() == '#') { continue; } diff --git a/source/extensions/filters/http/common/compressor/compressor.cc b/source/extensions/filters/http/common/compressor/compressor.cc index 5242ee31f4197..fd05db285686b 100644 --- a/source/extensions/filters/http/common/compressor/compressor.cc +++ b/source/extensions/filters/http/common/compressor/compressor.cc @@ -202,7 +202,7 @@ CompressorFilter::chooseEncoding(const Http::ResponseHeaderMap& headers) const { } // Find all encodings accepted by the user agent and adjust the list of allowed compressors. - for (const auto token : StringUtil::splitToken(*accept_encoding_, ",", false /* keep_empty */)) { + for (const auto& token : StringUtil::splitToken(*accept_encoding_, ",", false /* keep_empty */)) { EncPair pair = std::make_pair(StringUtil::trim(StringUtil::cropRight(token, ";")), static_cast(1)); const auto params = StringUtil::cropLeft(token, ";"); @@ -242,7 +242,7 @@ CompressorFilter::chooseEncoding(const Http::ResponseHeaderMap& headers) const { // Find intersection of encodings accepted by the user agent and provided // by the allowed compressors and choose the one with the highest q-value. EncPair choice{Http::Headers::get().AcceptEncodingValues.Identity, static_cast(0)}; - for (const auto pair : pairs) { + for (const auto& pair : pairs) { if ((pair.second > choice.second) && (allowed_compressors.count(std::string(pair.first)) || pair.first == Http::Headers::get().AcceptEncodingValues.Identity || diff --git a/source/extensions/filters/network/dubbo_proxy/hessian_utils.cc b/source/extensions/filters/network/dubbo_proxy/hessian_utils.cc index 337afc66059d8..c06ca93dd2c40 100644 --- a/source/extensions/filters/network/dubbo_proxy/hessian_utils.cc +++ b/source/extensions/filters/network/dubbo_proxy/hessian_utils.cc @@ -29,7 +29,7 @@ typename std::enable_if::value, T>::type leftShift(T left, uin inline void addByte(Buffer::Instance& buffer, const uint8_t value) { buffer.add(&value, 1); } void addSeq(Buffer::Instance& buffer, const std::initializer_list& values) { - for (const int8_t& value : values) { + for (const uint8_t& value : values) { buffer.add(&value, 1); } } diff --git a/source/extensions/stat_sinks/hystrix/hystrix.cc b/source/extensions/stat_sinks/hystrix/hystrix.cc index e04fa22ae0d1e..0596dd4cda41f 100644 --- a/source/extensions/stat_sinks/hystrix/hystrix.cc +++ b/source/extensions/stat_sinks/hystrix/hystrix.cc @@ -50,7 +50,7 @@ void HystrixSink::addHistogramToStream(const QuantileLatencyMap& latency_map, ab // TODO: Consider if we better use join here ss << ", \"" << key << "\": {"; bool is_first = true; - for (const std::pair& element : latency_map) { + for (const auto& element : latency_map) { const std::string quantile = fmt::sprintf("%g", element.first * 100); HystrixSink::addDoubleToStream(quantile, element.second, ss, is_first); is_first = false; diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 454e742207ccb..3464a2fd3cbd0 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -339,6 +339,18 @@ void emitLogs(Network::ListenerConfig& config, StreamInfo::StreamInfo& stream_in } } // namespace +void ConnectionHandlerImpl::ActiveTcpListener::pauseListening() { + if (listener_ != nullptr) { + listener_->disable(); + } +} + +void ConnectionHandlerImpl::ActiveTcpListener::resumeListening() { + if (listener_ != nullptr) { + listener_->enable(); + } +} + void ConnectionHandlerImpl::ActiveTcpListener::newConnection( Network::ConnectionSocketPtr&& socket) { auto stream_info = std::make_unique(parent_.dispatcher_.timeSource()); diff --git a/source/server/connection_handler_impl.h b/source/server/connection_handler_impl.h index ba9b7609d8cbe..1c452a5caec48 100644 --- a/source/server/connection_handler_impl.h +++ b/source/server/connection_handler_impl.h @@ -130,8 +130,8 @@ class ConnectionHandlerImpl : public Network::ConnectionHandler, // ActiveListenerImplBase Network::Listener* listener() override { return listener_.get(); } - void pauseListening() override { listener_->disable(); } - void resumeListening() override { listener_->enable(); } + void pauseListening() override; + void resumeListening() override; void shutdownListener() override { listener_.reset(); } // Network::BalancedConnectionHandler diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index c2c38cba1b5ab..e3fe75972e2ad 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -363,6 +363,22 @@ TEST_F(ConnectionHandlerTest, AddDisabledListener) { handler_->addListener(*test_listener); } +TEST_F(ConnectionHandlerTest, DisableListenerAfterStop) { + InSequence s; + + Network::ListenerCallbacks* listener_callbacks; + auto listener = new NiceMock(); + TestListener* test_listener = + addListener(1, false, false, "test_listener", listener, &listener_callbacks); + EXPECT_CALL(*socket_factory_, localAddress()).WillOnce(ReturnRef(local_address_)); + handler_->addListener(*test_listener); + + EXPECT_CALL(*listener, onDestroy()); + + handler_->stopListeners(); + handler_->disableListeners(); +} + TEST_F(ConnectionHandlerTest, DestroyCloseConnections) { InSequence s;