From cb65504b14d7cab25e092137b190daec7dc73c0f Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 6 Aug 2018 13:02:28 -0700 Subject: [PATCH 1/6] Implement host override of CN checking in the ASIO backend Add new function calc_cn_host which applies our host header override policy. Add the CN hostname used to asio_connection. Change the protocol such that the SSL upgrade request needs to come with the target hostname to use. Replace std::deque in connection pooling with a version that has constant amortized time per insertion. (std::deque is only constant amortized time for insertions in terms of element operations, not overall). Factor out the replacement policy from the overall pool machinery. Change release() to take shared_ptr&& to make sure the caller has let go. Enable verify_cert_chain_platform_specific on Windows; otherwise OpenSSL says that the root CAs are untrusted and all SSL tests fail on Windows. This accidentially repairs these test cases on Windows: **** outside_tests:outside_wikipedia_compressed_http_response FAILED **** **** outside_tests:multiple_https_requests FAILED **** **** outside_tests:outside_ssl_json FAILED **** --- Release/src/http/client/http_client_asio.cpp | 278 ++++++++++++------ Release/src/http/common/x509_cert_utilities.h | 62 ++-- .../functional/http/client/outside_tests.cpp | 10 +- 3 files changed, 241 insertions(+), 109 deletions(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index d3002dfc09..0fdce47673 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -81,7 +81,35 @@ using utility::conversions::details::to_string; using std::to_string; #endif -static const std::string CRLF("\r\n"); +namespace +{ +const std::string CRLF("\r\n"); + +std::string calc_cn_host(const bool secure, + const web::http::uri& baseUri, + const web::http::http_headers& requestHeaders) +{ + std::string result; + if (secure) + { + const utility::string_t* encResult; + const auto hostHeader = requestHeaders.find(_XPLATSTR("Host")); + if (hostHeader == requestHeaders.end()) + { + encResult = &baseUri.host(); + } + else + { + encResult = &hostHeader->second; + } + + result = utility::conversions::to_utf8string(*encResult); + utility::details::inplace_tolower(result); + } + + return result; +} +} // namespace namespace web { @@ -119,14 +147,21 @@ class asio_connection public: asio_connection(boost::asio::io_service& io_service) - : m_socket(io_service), m_is_reused(false), m_keep_alive(true), m_closed(false) + : m_socket_lock() + , m_socket(io_service) + , m_ssl_stream() + , m_cn_hostname() + , m_is_reused(false) + , m_keep_alive(true) + , m_closed(false) { } ~asio_connection() { close(); } // This simply instantiates the internal state to support ssl. It does not perform the handshake. - void upgrade_to_ssl(const std::function& ssl_context_callback) + void upgrade_to_ssl(std::string&& cn_hostname, + const std::function& ssl_context_callback) { std::lock_guard lock(m_socket_lock); assert(!is_ssl()); @@ -139,6 +174,7 @@ class asio_connection } m_ssl_stream = utility::details::make_unique>( m_socket, ssl_context); + m_cn_hostname = cn_hostname; } void close() @@ -166,6 +202,7 @@ class asio_connection void set_keep_alive(bool keep_alive) { m_keep_alive = keep_alive; } bool keep_alive() const { return m_keep_alive; } bool is_ssl() const { return m_ssl_stream ? true : false; } + const std::string& cn_hostname() const { return m_cn_hostname; } // Check if the error code indicates that the connection was closed by the // server: this is used to detect if a connection in the pool was closed during @@ -223,7 +260,6 @@ class asio_connection template void async_handshake(boost::asio::ssl::stream_base::handshake_type type, const http_client_config& config, - const std::string& host_name, const HandshakeHandler& handshake_handler, const CertificateHandler& cert_handler) { @@ -244,7 +280,7 @@ class asio_connection // Check to set host name for Server Name Indication (SNI) if (config.is_tlsext_sni_enabled()) { - SSL_set_tlsext_host_name(m_ssl_stream->native_handle(), const_cast(host_name.data())); + SSL_set_tlsext_host_name(m_ssl_stream->native_handle(), &m_cn_hostname[0]); } m_ssl_stream->async_handshake(type, handshake_handler); @@ -301,22 +337,84 @@ class asio_connection std::mutex m_socket_lock; tcp::socket m_socket; std::unique_ptr> m_ssl_stream; + std::string m_cn_hostname; bool m_is_reused; bool m_keep_alive; bool m_closed; }; +class connection_pool_deque +{ +public: + // attempts to acquire a connection from the deque; returns nullptr if no connection is + // available + std::shared_ptr try_acquire() CPPREST_NOEXCEPT + { + if (m_availableConnections.empty()) + { + if (m_returnedConnections.empty()) + { + return nullptr; + } + + m_availableConnections.swap(m_returnedConnections); + std::reverse(m_availableConnections.begin(), m_availableConnections.end()); + } + + if (m_coldConnections) + { + --m_coldConnections; + } + + auto result = std::move(m_availableConnections.back()); + m_availableConnections.pop_back(); + return result; + } + + // releases `released` back to the connection pool + void release(std::shared_ptr&& released) CPPREST_NOEXCEPT + { + try + { + m_returnedConnections.push_back(std::move(released)); + } + catch (...) + { + // if we didn't have enough memory to pool the connection, give up and drop it on the + // floor + auto dropped = std::move(released); + (void)dropped; + } + } + + bool free_stale_connections() CPPREST_NOEXCEPT + { + const auto availableSz = m_availableConnections.size(); + const auto returnedSz = m_returnedConnections.size(); + auto remaining = m_coldConnections; + const auto removeFromAvailable = std::min(availableSz, remaining); + remaining -= removeFromAvailable; + const auto removeFromReturned = std::min(returnedSz, remaining); + const auto newAvailable = availableSz - removeFromAvailable; + const auto newReturned = returnedSz - removeFromReturned; + m_availableConnections.resize(newAvailable); + m_returnedConnections.resize(newReturned); + m_coldConnections = newAvailable + newReturned; + return m_coldConnections != 0; + } + +private: + size_t m_coldConnections = 0; + std::vector> m_availableConnections; // oldest connection at the back + std::vector> m_returnedConnections; // oldest connection at the front +}; + /// Implements a connection pool with adaptive connection removal /// -/// The timeout mechanism is based on the `uint64_t m_epoch` member. Every 30 seconds, -/// the lambda in `start_epoch_interval` fires, triggering the cleanup of any -/// connections that have resided in the pool since the last cleanup phase's epoch. -/// -/// This works because the `m_connections` member functions is used in LIFO order. -/// LIFO usage guarantees that the elements remain sorted based on epoch number, -/// since the highest epoch is always removed and on insertion the next monotonically -/// increasing epoch is used. +/// Every 30 seconds, the lambda in `start_epoch_interval` fires, triggering the +/// cleanup of any connections that have resided in the pool since the last +/// cleanup phase. /// /// During the cleanup phase, connections are removed starting with the oldest. This /// ensures that if a high intensity workload is followed by a low intensity workload, @@ -327,93 +425,100 @@ class asio_connection /// /// while(1) /// { -/// auto conn = pool.acquire(); +/// auto conn = pool.try_acquire(); /// if (!conn) conn = new_conn(); -/// pool.release(conn); +/// pool.release(std::move(conn)); /// } /// /// class asio_connection_pool final : public std::enable_shared_from_this { public: - asio_connection_pool() : m_pool_epoch_timer(crossplat::threadpool::shared_instance().service()) {} + asio_connection_pool() + : m_lock() + , m_connections() + , m_is_timer_running(false) + , m_pool_epoch_timer(crossplat::threadpool::shared_instance().service()) + { + } + + asio_connection_pool(const asio_connection_pool&) = delete; + asio_connection_pool& operator=(const asio_connection_pool&) = delete; - std::shared_ptr acquire() + std::shared_ptr try_acquire(const std::string& cn_hostname) { std::lock_guard lock(m_lock); + if (m_connections.empty()) + { + return nullptr; + } - if (m_connections.empty()) return nullptr; + auto conn = m_connections[cn_hostname].try_acquire(); + if (conn) + { + conn->start_reuse(); + } - auto conn = std::move(m_connections.back().second); - m_connections.pop_back(); - conn->start_reuse(); return conn; } - void release(const std::shared_ptr& connection) + void release(std::shared_ptr&& connection) { connection->cancel(); - - if (!connection->keep_alive()) return; - - std::lock_guard lock(m_lock); - if (!is_timer_running) + if (!connection->keep_alive()) { - start_epoch_interval(shared_from_this()); - is_timer_running = true; + return; } - m_epoch++; - m_connections.emplace_back(m_epoch, std::move(connection)); + std::lock_guard lock(m_lock); + m_connections[connection->cn_hostname()].release(std::move(connection)); } private: // Note: must be called under m_lock static void start_epoch_interval(const std::shared_ptr& pool) { - _ASSERTE(pool.get() != nullptr); - auto& self = *pool; std::weak_ptr weak_pool = pool; - self.m_prev_epoch = self.m_epoch; - pool->m_pool_epoch_timer.expires_from_now(boost::posix_time::seconds(30)); - pool->m_pool_epoch_timer.async_wait([weak_pool](const boost::system::error_code& ec) { - if (ec) return; + self.m_pool_epoch_timer.expires_from_now(boost::posix_time::seconds(30)); + self.m_pool_epoch_timer.async_wait([weak_pool](const boost::system::error_code& ec) { + if (ec) + { + return; + } auto pool = weak_pool.lock(); - if (!pool) return; - auto& self = *pool; - auto& connections = self.m_connections; + if (!pool) + { + return; + } + auto& self = *pool; std::lock_guard lock(self.m_lock); - if (self.m_prev_epoch == self.m_epoch) + bool restartTimer = false; + for (auto& entry : self.m_connections) { - connections.clear(); - self.is_timer_running = false; - return; + if (entry.second.free_stale_connections()) + { + restartTimer = true; + } } - else + + if (restartTimer) { - auto prev_epoch = self.m_prev_epoch; - auto erase_end = std::find_if(connections.begin(), - connections.end(), - [prev_epoch](std::pair>& p) { - return p.first > prev_epoch; - }); - - connections.erase(connections.begin(), erase_end); start_epoch_interval(pool); } + else + { + self.m_is_timer_running = false; + } }); } std::mutex m_lock; - std::deque>> m_connections; - - uint64_t m_epoch = 0; - uint64_t m_prev_epoch = 0; - bool is_timer_running = false; + std::map m_connections; + bool m_is_timer_running; boost::asio::deadline_timer m_pool_epoch_timer; }; @@ -430,16 +535,20 @@ class asio_client final : public _http_client_communicator virtual void send_request(const std::shared_ptr& request_ctx) override; - void release_connection(std::shared_ptr& conn) { m_pool->release(conn); } - std::shared_ptr obtain_connection() - { - std::shared_ptr conn = m_pool->acquire(); + void release_connection(std::shared_ptr&& conn) { m_pool->release(std::move(conn)); } + std::shared_ptr obtain_connection(const http_request& req) + { + std::string cn_host = calc_cn_host(m_start_with_ssl, base_uri(), req.headers()); + std::shared_ptr conn = m_pool->try_acquire(cn_host); if (conn == nullptr) { // Pool was empty. Create a new connection conn = std::make_shared(crossplat::threadpool::shared_instance().service()); - if (m_start_with_ssl) conn->upgrade_to_ssl(this->client_config().get_ssl_context_callback()); + if (m_start_with_ssl) + { + conn->upgrade_to_ssl(std::move(cn_host), this->client_config().get_ssl_context_callback()); + } } return conn; @@ -447,7 +556,8 @@ class asio_client final : public _http_client_communicator virtual pplx::task propagate(http_request request) override; -public: + bool start_with_ssl() const CPPREST_NOEXCEPT { return m_start_with_ssl; } + tcp::resolver m_resolver; private: @@ -470,7 +580,7 @@ class asio_context final : public request_context, public std::enable_shared_fro , m_connection(connection) #ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE , m_openssl_failed(false) -#endif +#endif // CPPREST_USE_PLATFORM_VERIFICATION { } @@ -478,14 +588,14 @@ class asio_context final : public request_context, public std::enable_shared_fro { m_timer.stop(); // Release connection back to the pool. If connection was not closed, it will be put to the pool for reuse. - std::static_pointer_cast(m_http_client)->release_connection(m_connection); + std::static_pointer_cast(m_http_client)->release_connection(std::move(m_connection)); } static std::shared_ptr create_request_context(std::shared_ptr<_http_client_communicator>& client, http_request& request) { auto client_cast(std::static_pointer_cast(client)); - auto connection(client_cast->obtain_connection()); + auto connection(client_cast->obtain_connection(request)); auto ctx = std::make_shared(client, request, connection); ctx->m_timer.set_ctx(std::weak_ptr(ctx)); return ctx; @@ -578,7 +688,7 @@ class asio_context final : public request_context, public std::enable_shared_fro m_context->m_timer.reset(); //// Replace the connection. This causes old connection object to go out of scope. auto client = std::static_pointer_cast(m_context->m_http_client); - m_context->m_connection = client->obtain_connection(); + m_context->m_connection = client->obtain_connection(m_context->m_request); auto endpoint = *endpoints; m_context->m_connection->async_connect(endpoint, @@ -869,7 +979,12 @@ class asio_context final : public request_context, public std::enable_shared_fro } private: - void upgrade_to_ssl() { m_connection->upgrade_to_ssl(m_http_client->client_config().get_ssl_context_callback()); } + void upgrade_to_ssl() + { + auto& client = static_cast(*m_http_client); + m_connection->upgrade_to_ssl(calc_cn_host(client.start_with_ssl(), client.base_uri(), m_request.headers()), + client.client_config().get_ssl_context_callback()); + } std::string generate_basic_auth_header() { @@ -951,7 +1066,7 @@ class asio_context final : public request_context, public std::enable_shared_fro { // Replace the connection. This causes old connection object to go out of scope. auto client = std::static_pointer_cast(m_http_client); - m_connection = client->obtain_connection(); + m_connection = client->obtain_connection(m_request); auto endpoint = *endpoints; m_connection->async_connect( @@ -987,11 +1102,11 @@ class asio_context final : public request_context, public std::enable_shared_fro m_connection->async_handshake( boost::asio::ssl::stream_base::client, m_http_client->client_config(), - utility::conversions::to_utf8string(m_http_client->base_uri().host()), boost::bind(&asio_context::handle_handshake, shared_from_this(), boost::asio::placeholders::error), - // Use a weak_ptr since the verify_callback is stored until the connection is destroyed. - // This avoids creating a circular reference since we pool connection objects. + // Use a weak_ptr since the verify_callback is stored until the connection is + // destroyed. This avoids creating a circular reference since we pool connection + // objects. [weakCtx](bool preverified, boost::asio::ssl::verify_context& verify_context) { auto this_request = weakCtx.lock(); if (this_request) @@ -1031,23 +1146,22 @@ class asio_context final : public request_context, public std::enable_shared_fro // certificate chain, the rest are optional intermediate certificates, followed // finally by the root CA self signed certificate. - const auto& host = utility::conversions::to_utf8string(m_http_client->base_uri().host()); #ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE - // Attempt to use platform certificate validation when it is available: - // If OpenSSL fails we will doing verification at the end using the whole certificate chain, - // so wait until the 'leaf' cert. For now return true so OpenSSL continues down the certificate - // chain. + // If OpenSSL fails we will doing verification at the end using the whole certificate + // chain so wait until the 'leaf' cert. For now return true so OpenSSL continues down + // the certificate chain. if (!preverified) { m_openssl_failed = true; } + if (m_openssl_failed) { - return verify_cert_chain_platform_specific(verifyCtx, host); + return verify_cert_chain_platform_specific(verifyCtx, m_connection->cn_hostname()); } -#endif +#endif // CPPREST_USE_PLATFORM_VERIFICATION - boost::asio::ssl::rfc2818_verification rfc2818(host); + boost::asio::ssl::rfc2818_verification rfc2818(m_connection->cn_hostname()); return rfc2818(preverified, verifyCtx); } @@ -1759,7 +1873,7 @@ class asio_context final : public request_context, public std::enable_shared_fro #ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE bool m_openssl_failed; -#endif +#endif // CPPREST_USE_PLATFORM_VERIFICATION }; std::shared_ptr<_http_client_communicator> create_platform_final_pipeline_stage(uri&& base_uri, diff --git a/Release/src/http/common/x509_cert_utilities.h b/Release/src/http/common/x509_cert_utilities.h index e19af0498a..581a5189c4 100644 --- a/Release/src/http/common/x509_cert_utilities.h +++ b/Release/src/http/common/x509_cert_utilities.h @@ -1,23 +1,29 @@ /*** -* Copyright (C) Microsoft. All rights reserved. -* Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. -* -* =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ -* -* Contains utility functions for helping to verify server certificates in OS X/iOS and Android. -* -* For the latest on this and related APIs, please see: https://github.com/Microsoft/cpprestsdk -* -* =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- -****/ + * Copyright (C) Microsoft. All rights reserved. + * Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. + * + * =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ + * + * Contains utility functions for helping to verify server certificates in OS X/iOS and Android. + * + * For the latest on this and related APIs, please see: https://github.com/Microsoft/cpprestsdk + * + * =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- + ****/ #pragma once #if defined(_WIN32) #include -namespace web { namespace http { namespace client { namespace details { - +namespace web +{ +namespace http +{ +namespace client +{ +namespace details +{ struct winhttp_cert_context { PCCERT_CONTEXT raw; @@ -49,12 +55,16 @@ struct winhttp_cert_chain_context } } }; - -}}}} // namespaces +} +} +} +} // namespaces #endif // _WIN32 -#if defined(__APPLE__) || (defined(ANDROID) || defined(__ANDROID__)) || (defined(_WIN32) && !defined(__cplusplus_winrt) && !defined(_M_ARM) && !defined(CPPREST_EXCLUDE_WEBSOCKETS)) - #define CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE +#if defined(__APPLE__) || (defined(ANDROID) || defined(__ANDROID__)) || \ + (defined(_WIN32) && defined(CPPREST_FORCE_HTTP_CLIENT_ASIO)) || \ + (defined(_WIN32) && !defined(__cplusplus_winrt) && !defined(_M_ARM) && !defined(CPPREST_EXCLUDE_WEBSOCKETS)) +#define CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE #endif #ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE @@ -76,8 +86,14 @@ struct winhttp_cert_chain_context #pragma warning(pop) #endif -namespace web { namespace http { namespace client { namespace details { - +namespace web +{ +namespace http +{ +namespace client +{ +namespace details +{ /// /// Using platform specific APIs verifies server certificate. /// Currently implemented to work on Windows, iOS, Android, and OS X. @@ -85,8 +101,10 @@ namespace web { namespace http { namespace client { namespace details { /// Boost.ASIO context to get certificate chain from. /// Host name from the URI. /// True if verification passed and server can be trusted, false otherwise. -bool verify_cert_chain_platform_specific(boost::asio::ssl::verify_context &verifyCtx, const std::string &hostName); - -}}}} +bool verify_cert_chain_platform_specific(boost::asio::ssl::verify_context& verifyCtx, const std::string& hostName); +} +} +} +} #endif // CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE diff --git a/Release/tests/functional/http/client/outside_tests.cpp b/Release/tests/functional/http/client/outside_tests.cpp index f6ea1396cf..7c0438d48c 100644 --- a/Release/tests/functional/http/client/outside_tests.cpp +++ b/Release/tests/functional/http/client/outside_tests.cpp @@ -197,10 +197,7 @@ TEST(server_hostname_mismatch) } #if !defined(__cplusplus_winrt) -TEST(server_hostname_host_override, - "Ignore:Android", "229", - "Ignore:Apple", "229", - "Ignore:Linux", "229") +TEST(server_hostname_host_override) { handle_timeout([] { @@ -223,7 +220,10 @@ TEST(server_hostname_host_override_after_upgrade) http_request req(methods::GET); req.headers().add(U("Host"), U("en.wikipedia.org")); auto response = client.request(req).get(); - VERIFY_ARE_EQUAL(status_codes::OK, response.status_code()); + // WinHTTP will transparently follow the HTTP 301 upgrade request redirect, + // ASIO does not and will return the 301 directly. + const auto statusCode = response.status_code(); + CHECK(statusCode == status_codes::OK || statusCode == status_codes::MovedPermanently); } #endif // !defined(__cplusplus_winrt) From 639900517d835e234bb48a75bc58cd8026fd5bc7 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Sat, 11 Aug 2018 03:16:04 -0700 Subject: [PATCH 2/6] Add missing std::move. --- Release/src/http/client/http_client_asio.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index 0fdce47673..7ba60ead03 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -174,7 +174,7 @@ class asio_connection } m_ssl_stream = utility::details::make_unique>( m_socket, ssl_context); - m_cn_hostname = cn_hostname; + m_cn_hostname = std::move(cn_hostname); } void close() From b589de0d292770fe9a91d5c410f1f7f910809615 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Sat, 11 Aug 2018 03:16:40 -0700 Subject: [PATCH 3/6] Use hot connections instead of cold connections in connection pool. --- Release/src/http/client/http_client_asio.cpp | 58 ++++++-------------- 1 file changed, 17 insertions(+), 41 deletions(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index 7ba60ead03..23dc3e76e9 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -344,70 +344,46 @@ class asio_connection bool m_closed; }; -class connection_pool_deque +class connection_pool_stack { public: // attempts to acquire a connection from the deque; returns nullptr if no connection is // available std::shared_ptr try_acquire() CPPREST_NOEXCEPT { - if (m_availableConnections.empty()) + const size_t oldConnectionsSize = m_connections.size(); + if (m_highWater > oldConnectionsSize) { - if (m_returnedConnections.empty()) - { - return nullptr; - } - - m_availableConnections.swap(m_returnedConnections); - std::reverse(m_availableConnections.begin(), m_availableConnections.end()); + m_highWater = oldConnectionsSize; } - if (m_coldConnections) + if (oldConnectionsSize == 0) { - --m_coldConnections; + return nullptr; } - auto result = std::move(m_availableConnections.back()); - m_availableConnections.pop_back(); + auto result = std::move(m_connections.back()); + m_connections.pop_back(); return result; } // releases `released` back to the connection pool - void release(std::shared_ptr&& released) CPPREST_NOEXCEPT + void release(std::shared_ptr&& released) { - try - { - m_returnedConnections.push_back(std::move(released)); - } - catch (...) - { - // if we didn't have enough memory to pool the connection, give up and drop it on the - // floor - auto dropped = std::move(released); - (void)dropped; - } + m_connections.push_back(std::move(released)); } bool free_stale_connections() CPPREST_NOEXCEPT { - const auto availableSz = m_availableConnections.size(); - const auto returnedSz = m_returnedConnections.size(); - auto remaining = m_coldConnections; - const auto removeFromAvailable = std::min(availableSz, remaining); - remaining -= removeFromAvailable; - const auto removeFromReturned = std::min(returnedSz, remaining); - const auto newAvailable = availableSz - removeFromAvailable; - const auto newReturned = returnedSz - removeFromReturned; - m_availableConnections.resize(newAvailable); - m_returnedConnections.resize(newReturned); - m_coldConnections = newAvailable + newReturned; - return m_coldConnections != 0; + m_connections.erase(m_connections.begin(), m_connections.begin() + m_highWater); + const size_t connectionsSize = m_connections.size(); + m_highWater = connectionsSize; + return (connectionsSize != 0); } private: - size_t m_coldConnections = 0; - std::vector> m_availableConnections; // oldest connection at the back - std::vector> m_returnedConnections; // oldest connection at the front + size_t m_highWater = 0; + std::vector> m_connections; }; /// Implements a connection pool with adaptive connection removal @@ -517,7 +493,7 @@ class asio_connection_pool final : public std::enable_shared_from_this m_connections; + std::map m_connections; bool m_is_timer_running; boost::asio::deadline_timer m_pool_epoch_timer; }; From 1b9ef2a70a30b188c588aa390a045631f78b5b22 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Sat, 11 Aug 2018 03:17:18 -0700 Subject: [PATCH 4/6] Put timer start code back. --- Release/src/http/client/http_client_asio.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index 23dc3e76e9..6a3b5349ca 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -447,6 +447,12 @@ class asio_connection_pool final : public std::enable_shared_from_this lock(m_lock); + if (!m_is_timer_running) + { + start_epoch_interval(shared_from_this()); + m_is_timer_running = true; + } + m_connections[connection->cn_hostname()].release(std::move(connection)); } From 99b893f1d2998377516ec9d7619c90fab6a99645 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Sat, 11 Aug 2018 03:18:47 -0700 Subject: [PATCH 5/6] Reset the shared_ptr when it is being dropped. --- Release/src/http/client/http_client_asio.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index 6a3b5349ca..6f05f3a056 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -443,6 +443,7 @@ class asio_connection_pool final : public std::enable_shared_from_thiscancel(); if (!connection->keep_alive()) { + connection.reset(); return; } From 3f8eaa20da9b18e44af0a7f03d540355fb99c434 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Sat, 11 Aug 2018 03:19:05 -0700 Subject: [PATCH 6/6] Fix end comment broken by merge hell. --- Release/src/http/client/http_client_asio.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index 6f05f3a056..e3ea15c9ff 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -563,7 +563,7 @@ class asio_context final : public request_context, public std::enable_shared_fro , m_connection(connection) #ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE , m_openssl_failed(false) -#endif // CPPREST_USE_PLATFORM_VERIFICATION +#endif // CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE { } @@ -1142,7 +1142,7 @@ class asio_context final : public request_context, public std::enable_shared_fro { return verify_cert_chain_platform_specific(verifyCtx, m_connection->cn_hostname()); } -#endif // CPPREST_USE_PLATFORM_VERIFICATION +#endif // CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE boost::asio::ssl::rfc2818_verification rfc2818(m_connection->cn_hostname()); return rfc2818(preverified, verifyCtx); @@ -1856,7 +1856,7 @@ class asio_context final : public request_context, public std::enable_shared_fro #ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE bool m_openssl_failed; -#endif // CPPREST_USE_PLATFORM_VERIFICATION +#endif // CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE }; std::shared_ptr<_http_client_communicator> create_platform_final_pipeline_stage(uri&& base_uri,