diff --git a/Release/src/CMakeLists.txt b/Release/src/CMakeLists.txt index eceaad7437..cea7048c7d 100644 --- a/Release/src/CMakeLists.txt +++ b/Release/src/CMakeLists.txt @@ -16,12 +16,13 @@ set(SOURCES ${HEADERS_DETAILS} pch/stdafx.h http/client/http_client.cpp - http/client/http_client_msg.cpp http/client/http_client_impl.h - http/common/internal_http_helpers.h + http/client/http_client_msg.cpp + http/common/connection_pool_helpers.h + http/common/http_compression.cpp http/common/http_helpers.cpp http/common/http_msg.cpp - http/common/http_compression.cpp + http/common/internal_http_helpers.h http/listener/http_listener.cpp http/listener/http_listener_msg.cpp http/listener/http_server_api.cpp diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index 645b45466c..ada8e9494d 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -17,6 +17,7 @@ #include #include "../common/internal_http_helpers.h" +#include "../common/connection_pool_helpers.h" #include "cpprest/asyncrt_utils.h" #if defined(__clang__) @@ -345,48 +346,6 @@ class asio_connection bool m_closed; }; -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 - { - const size_t oldConnectionsSize = m_connections.size(); - if (m_highWater > oldConnectionsSize) - { - m_highWater = oldConnectionsSize; - } - - if (oldConnectionsSize == 0) - { - return nullptr; - } - - 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) - { - m_connections.push_back(std::move(released)); - } - - bool free_stale_connections() CPPREST_NOEXCEPT - { - 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_highWater = 0; - std::vector> m_connections; -}; - /// Implements a connection pool with adaptive connection removal /// /// Every 30 seconds, the lambda in `start_epoch_interval` fires, triggering the @@ -501,7 +460,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; }; diff --git a/Release/src/http/common/connection_pool_helpers.h b/Release/src/http/common/connection_pool_helpers.h new file mode 100644 index 0000000000..580b82af23 --- /dev/null +++ b/Release/src/http/common/connection_pool_helpers.h @@ -0,0 +1,66 @@ +#pragma once + +#include "cpprest/details/cpprest_compat.h" +#include +#include +#include + +namespace web +{ +namespace http +{ +namespace client +{ +namespace details +{ + +template +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 + { + const size_t oldConnectionsSize = m_connections.size(); + if (oldConnectionsSize == 0) + { + m_staleBefore = 0; + return nullptr; + } + + auto result = std::move(m_connections.back()); + m_connections.pop_back(); + const size_t newConnectionsSize = m_connections.size(); + if (m_staleBefore > newConnectionsSize) + { + m_staleBefore = newConnectionsSize; + } + + return result; + } + + // releases `released` back to the connection pool + void release(std::shared_ptr&& released) + { + m_connections.push_back(std::move(released)); + } + + bool free_stale_connections() CPPREST_NOEXCEPT + { + assert(m_staleBefore <= m_connections.size()); + m_connections.erase(m_connections.begin(), m_connections.begin() + m_staleBefore); + const size_t connectionsSize = m_connections.size(); + m_staleBefore = connectionsSize; + return (connectionsSize != 0); + } + +private: + std::vector> m_connections; + size_t m_staleBefore = 0; +}; + +} // details +} // client +} // http +} // web diff --git a/Release/tests/functional/http/client/CMakeLists.txt b/Release/tests/functional/http/client/CMakeLists.txt index d92b477481..45f0d9af02 100644 --- a/Release/tests/functional/http/client/CMakeLists.txt +++ b/Release/tests/functional/http/client/CMakeLists.txt @@ -2,8 +2,11 @@ set(SOURCES authentication_tests.cpp building_request_tests.cpp client_construction.cpp + compression_tests.cpp + connection_pool_tests.cpp connections_and_errors.cpp header_tests.cpp + http_client_fuzz_tests.cpp http_client_tests.cpp http_methods_tests.cpp multiple_requests.cpp @@ -20,8 +23,6 @@ set(SOURCES response_stream_tests.cpp status_code_reason_phrase_tests.cpp to_string_tests.cpp - http_client_fuzz_tests.cpp - compression_tests.cpp ) add_casablanca_test(httpclient_test SOURCES) diff --git a/Release/tests/functional/http/client/connection_pool_tests.cpp b/Release/tests/functional/http/client/connection_pool_tests.cpp new file mode 100644 index 0000000000..037ed69d88 --- /dev/null +++ b/Release/tests/functional/http/client/connection_pool_tests.cpp @@ -0,0 +1,45 @@ +#include "stdafx.h" +#include +#include "../../../src/http/common/connection_pool_helpers.h" + +using namespace web::http::client::details; + +SUITE(connection_pooling) { + TEST(empty_returns_nullptr) { + connection_pool_stack connectionStack; + VERIFY_ARE_EQUAL(connectionStack.try_acquire(), std::shared_ptr{}); + } + + static int noisyCount = 0; + struct noisy { + noisy() = delete; + noisy(int) { ++noisyCount; } + noisy(const noisy&) = delete; + noisy(noisy&&) { ++noisyCount; } + noisy& operator=(const noisy&) = delete; + noisy& operator=(noisy&&) = delete; + ~noisy() { --noisyCount; } + }; + + TEST(cycled_connections_survive) { + connection_pool_stack connectionStack; + VERIFY_ARE_EQUAL(0, noisyCount); + connectionStack.release(std::make_shared(42)); + connectionStack.release(std::make_shared(42)); + connectionStack.release(std::make_shared(42)); + VERIFY_ARE_EQUAL(3, noisyCount); + VERIFY_IS_TRUE(connectionStack.free_stale_connections()); + auto tmp = connectionStack.try_acquire(); + VERIFY_ARE_NOT_EQUAL(tmp, std::shared_ptr{}); + connectionStack.release(std::move(tmp)); + VERIFY_ARE_EQUAL(tmp, std::shared_ptr{}); + tmp = connectionStack.try_acquire(); + VERIFY_ARE_NOT_EQUAL(tmp, std::shared_ptr{}); + connectionStack.release(std::move(tmp)); + VERIFY_IS_TRUE(connectionStack.free_stale_connections()); + VERIFY_ARE_EQUAL(1, noisyCount); + VERIFY_IS_FALSE(connectionStack.free_stale_connections()); + VERIFY_ARE_EQUAL(0, noisyCount); + VERIFY_IS_FALSE(connectionStack.free_stale_connections()); + } +};