diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index ec6aa995b64a3..733eac06e09fc 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.13.6 (September 29, 2020) =========================== diff --git a/source/common/common/utility.cc b/source/common/common/utility.cc index 072825cfb91f2..7ad15aa0da67e 100644 --- a/source/common/common/utility.cc +++ b/source/common/common/utility.cc @@ -277,7 +277,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 6d160f98c1e5a..ed0c51bd70ef8 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -159,7 +159,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 93134d0b779a2..1fc97faefb82f 100644 --- a/source/common/router/retry_state_impl.cc +++ b/source/common/router/retry_state_impl.cc @@ -113,7 +113,7 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, Http::HeaderMap& } 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)) { @@ -128,7 +128,7 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, Http::HeaderMap& // 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))); @@ -152,7 +152,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, ",")) { + for (const auto& retry_on : StringUtil::splitToken(config, ",")) { if (retry_on == Http::Headers::get().EnvoyRetryOnValues._5xx) { ret |= RetryPolicy::RETRY_ON_5XX; } else if (retry_on == Http::Headers::get().EnvoyRetryOnValues.GatewayError) { @@ -180,7 +180,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, ",")) { + for (const auto& retry_on : StringUtil::splitToken(retry_grpc_on_header, ",")) { 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 ba8912edb6362..886967de36c46 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -414,7 +414,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/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 3e0b457204e33..c16f029d197db 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 9f0c188bf1621..a64269bf6278c 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -330,6 +330,18 @@ void ConnectionHandlerImpl::ActiveTcpListener::onAcceptWorker( } } +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) { // Find matching filter chain. diff --git a/source/server/connection_handler_impl.h b/source/server/connection_handler_impl.h index 407734fc48fc8..0b210f0576200 100644 --- a/source/server/connection_handler_impl.h +++ b/source/server/connection_handler_impl.h @@ -130,6 +130,9 @@ class ConnectionHandlerImpl : public Network::ConnectionHandler, // ActiveListenerImplBase Network::Listener* listener() override { return listener_.get(); } void destroy() override { listener_.reset(); } + void pauseListening() override; + void resumeListening() override; + void shutdownListener() override { listener_.reset(); } // Network::BalancedConnectionHandler uint64_t numConnections() const override { return num_listener_connections_; } diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index dafbcd5bb2f1a..8f0690497208f 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -357,6 +357,22 @@ TEST_F(ConnectionHandlerTest, AddDisabledListener) { handler_->addListener(*test_listener); } +TEST_F(ConnectionHandlerTest, DisableListenerAfterStop) { + InSequence s; + + Network::TcpListenerCallbacks* 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(absl::nullopt, *test_listener); + + EXPECT_CALL(*listener, onDestroy()); + + handler_->stopListeners(); + handler_->disableListeners(); +} + TEST_F(ConnectionHandlerTest, DestroyCloseConnections) { InSequence s;