From 3e1af936d9118223d4b05a50d9a174f3cbf8ad1f Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Sat, 17 Apr 2021 21:36:24 -0700 Subject: [PATCH] Remove thread detach (#2105) --- sdk/core/azure-core/src/http/curl/curl.cpp | 221 +++++---- .../curl/curl_connection_pool_private.hpp | 101 ++-- .../src/http/curl/curl_connection_private.hpp | 9 +- .../src/http/curl/curl_session_private.hpp | 2 +- sdk/core/azure-core/test/ut/CMakeLists.txt | 4 + .../test/ut/azure_libcurl_core_main.cpp | 10 +- .../test/ut/curl_connection_pool.cpp | 454 ++++++++++++++---- sdk/core/azure-core/test/ut/curl_options.cpp | 16 +- sdk/core/azure-core/test/ut/curl_session.hpp | 2 +- .../azure-core/test/ut/curl_session_test.cpp | 23 +- 10 files changed, 559 insertions(+), 283 deletions(-) diff --git a/sdk/core/azure-core/src/http/curl/curl.cpp b/sdk/core/azure-core/src/http/curl/curl.cpp index d1239f475..9142a4f59 100644 --- a/sdk/core/azure-core/src/http/curl/curl.cpp +++ b/sdk/core/azure-core/src/http/curl/curl.cpp @@ -8,7 +8,7 @@ #include "azure/core/internal/diagnostics/log.hpp" #include "azure/core/platform.hpp" -// Private incude +// Private include #include "curl_connection_pool_private.hpp" #include "curl_connection_private.hpp" #include "curl_session_private.hpp" @@ -31,7 +31,7 @@ std::string const LogMsgPrefix = "[CURL Transport Adapter]: "; template #if defined(_MSC_VER) #pragma warning(push) -// C26812: The enum type 'CURLoption' is unscoped. Prefer 'enum class' over 'enum' (Enum.3) +// C26812: The enum type 'CURLoption' is un-scoped. Prefer 'enum class' over 'enum' (Enum.3) #pragma warning(disable : 26812) #endif inline bool SetLibcurlOption(CURL* handle, CURLoption option, T value, CURLcode* outError) @@ -180,6 +180,78 @@ static inline std::string GetHTTPMessagePreBody(Azure::Core::Http::Request const return httpRequest; } + +static void CleanupThread() +{ + using namespace Azure::Core::Http::_detail; + for (;;) + { + Log::Write(Logger::Level::Verbose, "Clean pool check now..."); + // Won't continue until the ConnectionPoolMutex is released from MoveConnectionBackToPool + std::unique_lock lockForPoolCleaning( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + Log::Write(Logger::Level::Verbose, "Clean pool sleep"); + // Wait for the default time OR to the signal from the conditional variable. + // wait_for releases the mutex lock when it goes to sleep and it takes the lock again when it + // wakes up (or it's cancelled). + if (CurlConnectionPool::g_curlConnectionPool.ConditionalVariableForCleanThread.wait_for( + lockForPoolCleaning, + std::chrono::milliseconds(DefaultCleanerIntervalMilliseconds), + []() { + return CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.size() == 0; + })) + { + // Cancelled by another thead or no connections on wakeup + Log::Write( + Logger::Level::Verbose, + "Clean pool - no connections on wake - return *************************"); + CurlConnectionPool::g_curlConnectionPool.IsCleanThreadRunning = false; + return; + } + + Log::Write(Logger::Level::Verbose, "Clean pool - inspect pool"); + // loop the connection pool index - Note: lock is re-taken for the mutex + // Notes: The size of each host-index is always expected to be greater than 0 because the + // host-index is removed anytime it becomes empty. + for (auto index = CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.begin(); + index != CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.end();) + { + // Each pool index behaves as a Last-in-First-out (connections are added to the pool with + // push_front). The last connection moved to the pool will be the first to be re-used. Because + // of this, the oldest connection in the pool can be found at the end of the list. Looping the + // connection pool backwards until a connection that is not expired is found or until all + // connections are removed. + for (auto connection = --(index->second.end()); + index->second.size() > 0 && connection->get()->IsExpired(); + connection = index->second.size() > 0 ? --connection : connection) + { + // remove connection from the pool and update the connection to the next one + // which is going to be list.end() + Log::Write(Logger::Level::Verbose, "Clean pool - remove connection"); + connection = index->second.erase(connection); + } + + if (index->second.size() == 0) + { + Log::Write(Logger::Level::Verbose, "Clean pool - remove index " + index->first); + index = CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.erase(index); + } + else + { + index = ++index; + } + } + + if (CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.size() == 0) + { + Log::Write( + Logger::Level::Verbose, + "Clean pool - all connections removed. Return**********************"); + CurlConnectionPool::g_curlConnectionPool.IsCleanThreadRunning = false; + return; + } + } +} } // namespace using Azure::Core::Context; @@ -194,6 +266,9 @@ using Azure::Core::Http::Request; using Azure::Core::Http::TransportException; using Azure::Core::Http::_detail::CurlConnectionPool; +Azure::Core::Http::_detail::CurlConnectionPool + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool; + std::unique_ptr CurlTransport::Send(Request& request, Context const& context) { // Create CurlSession to perform request @@ -201,7 +276,7 @@ std::unique_ptr CurlTransport::Send(Request& request, Context const auto session = std::make_unique( request, - CurlConnectionPool::ExtractOrCreateCurlConnection(request, m_options), + CurlConnectionPool::g_curlConnectionPool.ExtractOrCreateCurlConnection(request, m_options), m_options.HttpKeepAlive); CURLcode performing; @@ -226,7 +301,7 @@ std::unique_ptr CurlTransport::Send(Request& request, Context const // won't be no longer valid. session = std::make_unique( request, - CurlConnectionPool::ExtractOrCreateCurlConnection( + CurlConnectionPool::g_curlConnectionPool.ExtractOrCreateCurlConnection( request, m_options, getConnectionOpenIntent + 1 >= _detail::RequestPoolResetAfterConnectionFailed), @@ -1106,12 +1181,6 @@ int64_t CurlSession::ResponseBufferParser::BuildHeader( return indexOfEndOfStatusLine + 1 - buffer; } -std::mutex CurlConnectionPool::ConnectionPoolMutex; -std::map>> - CurlConnectionPool::ConnectionPoolIndex; -uint64_t CurlConnectionPool::g_connectionCounter = 0; -std::atomic CurlConnectionPool::g_isCleanConnectionsRunning(false); - namespace { inline std::string GetConnectionKey(std::string const& host, CurlTransportOptions const& options) { @@ -1168,15 +1237,16 @@ std::unique_ptr CurlConnectionPool::ExtractOrCreateCurlCo std::lock_guard lock(CurlConnectionPool::ConnectionPoolMutex); // get a ref to the pool from the map of pools - auto hostPoolIndex = CurlConnectionPool::ConnectionPoolIndex.find(connectionKey); + auto hostPoolIndex = g_curlConnectionPool.ConnectionPoolIndex.find(connectionKey); - if (hostPoolIndex != CurlConnectionPool::ConnectionPoolIndex.end() + if (hostPoolIndex != g_curlConnectionPool.ConnectionPoolIndex.end() && hostPoolIndex->second.size() > 0) { if (resetPool) { - // Remove all connections for the connection Key and move to spawn new connection below - CurlConnectionPool::g_connectionCounter -= hostPoolIndex->second.size(); + // clean the pool-index as requested in the call. Typically to force a new connection to be + // created and to discard all current connections in the pool for the host-index. A caller + // might request this after getting broken/closed connections multiple-times. hostPoolIndex->second.clear(); } else @@ -1187,13 +1257,11 @@ std::unique_ptr CurlConnectionPool::ExtractOrCreateCurlCo auto connection = std::move(*fistConnectionIterator); // Remove the connection ref from list hostPoolIndex->second.erase(fistConnectionIterator); - // reduce number of connections on the pool - CurlConnectionPool::g_connectionCounter -= 1; // Remove index if there are no more connections if (hostPoolIndex->second.size() == 0) { - CurlConnectionPool::ConnectionPoolIndex.erase(hostPoolIndex); + g_curlConnectionPool.ConnectionPoolIndex.erase(hostPoolIndex); } // return connection ref @@ -1324,96 +1392,41 @@ void CurlConnectionPool::MoveConnectionBackToPool( return; } + Log::Write(Logger::Level::Verbose, "Moving connection to pool..."); + // Lock mutex to access connection pool. mutex is unlock as soon as lock is out of scope std::lock_guard lock(CurlConnectionPool::ConnectionPoolMutex); auto& poolId = connection->GetConnectionKey(); - auto& hostPool = CurlConnectionPool::ConnectionPoolIndex[poolId]; - // update the time when connection was moved back to pool - connection->updateLastUsageTime(); - hostPool.push_front(std::move(connection)); - CurlConnectionPool::g_connectionCounter += 1; - // Check if there's no cleaner running and started - if (!CurlConnectionPool::g_isCleanConnectionsRunning) + auto& hostPool = g_curlConnectionPool.ConnectionPoolIndex[poolId]; + + if (hostPool.size() >= _detail::MaxConnectionsPerIndex) { - CurlConnectionPool::g_isCleanConnectionsRunning = true; - CurlConnectionPool::CleanUp(); + // Remove the last connection from the pool to insert this one. + auto lastConnection = --hostPool.end(); + hostPool.erase(lastConnection); + } + + // update the time when connection was moved back to pool + connection->UpdateLastUsageTime(); + hostPool.push_front(std::move(connection)); + + if (m_cleanThread.joinable() && !IsCleanThreadRunning) + { + // Clean thread was running before but it's finished, join it to finalize + m_cleanThread.join(); + } + + // Cleanup will start a background thread which will close abandoned connections from the pool. + // This will free-up resources from the app + // This is the only call to cleanup. + if (!m_cleanThread.joinable()) + { + Log::Write(Logger::Level::Verbose, "Start clean thread"); + IsCleanThreadRunning = true; + m_cleanThread = std::thread(CleanupThread); + } + else + { + Log::Write(Logger::Level::Verbose, "Clean thread running. Won't start a new one."); } } - -// spawn a thread for cleaning old connections. -// Thread will keep running while there are at least one connection in the pool -void CurlConnectionPool::CleanUp() -{ - std::thread backgroundCleanerThread([]() { - for (;;) - { - // wait before trying to clean - std::this_thread::sleep_for( - std::chrono::milliseconds(_detail::DefaultCleanerIntervalMilliseconds)); - - // while sleeping, it is allowed to explicitly prevent the cleaner to run and stop it, for - // example, when the application exits, and the cleaner is sleeping, we don't want it to wake - // up and try to access de-allocated memory. - if (!CurlConnectionPool::g_isCleanConnectionsRunning) - { - return; - } - - { - // take mutex for reading the pool - std::lock_guard lock(CurlConnectionPool::ConnectionPoolMutex); - - if (CurlConnectionPool::g_connectionCounter == 0) - { - // stop the cleaner since there are no connections - CurlConnectionPool::g_isCleanConnectionsRunning = false; - return; - } - - // loop the connection pool index - for (auto index = CurlConnectionPool::ConnectionPoolIndex.begin(); - index != CurlConnectionPool::ConnectionPoolIndex.end(); - index++) - { - if (index->second.size() == 0) - { - // Move the next pool index - continue; - } - - // Pool index with waiting connections. Loop the connection pool backwards until - // a connection that is not expired is found or until all connections are removed. - for (auto connection = index->second.end();;) - { - // loop starts at end(), go back to previous possition. We know the list is - // size() > 0 so we are safe to go end() - 1 and find the last element in the - // list - connection--; - if (connection->get()->IsExpired()) - { - // remove connection from the pool and update the connection to the next one - // which is going to be list.end() - connection = index->second.erase(connection); - CurlConnectionPool::g_connectionCounter -= 1; - - // Connection removed, break if there are no more connections to check - if (index->second.size() == 0) - { - break; - } - } - else - { - // Got a non-expired connection, all connections before this one are not - // expired. Break the loop and continue looping the Pool index - break; - } - } - } - } - } - }); - - // let thread run independent. It will be done once ther is not connections in the pool - backgroundCleanerThread.detach(); -} diff --git a/sdk/core/azure-core/src/http/curl/curl_connection_pool_private.hpp b/sdk/core/azure-core/src/http/curl/curl_connection_pool_private.hpp index 3bc9f22e9..197560bdb 100644 --- a/sdk/core/azure-core/src/http/curl/curl_connection_pool_private.hpp +++ b/sdk/core/azure-core/src/http/curl/curl_connection_pool_private.hpp @@ -15,11 +15,13 @@ #include "curl_connection_private.hpp" #include +#include #include #include #include #include #include +#include #if defined(TESTING_BUILD) // Define the class name that reads from ConnectionPool private members @@ -31,9 +33,6 @@ namespace Azure { namespace Core { namespace Test { namespace Azure { namespace Core { namespace Http { namespace _detail { - // In charge of calling the libcurl global functions for the Azure SDK - struct CurlGlobalStateForAzureSdk; - /** * @brief CURL HTTP connection pool makes it possible to re-use one curl connection to perform * more than one request. Use this component when connections are not re-used by default. @@ -47,27 +46,25 @@ namespace Azure { namespace Core { namespace Http { namespace _detail { friend class Azure::Core::Test::CurlConnectionPool_connectionPoolTest_Test; friend class Azure::Core::Test::CurlConnectionPool_uniquePort_Test; #endif - private: - // The cttor and dttor of this member makes sure of calling the libcurl global init and cleanup - AZ_CORE_DLLEXPORT static CurlGlobalStateForAzureSdk CurlGlobalState; public: - /** - * @brief Mutex for accessing connection pool for thread-safe reading and writing. - */ - AZ_CORE_DLLEXPORT static std::mutex ConnectionPoolMutex; - - /** - * @brief Keeps an unique key for each host and creates a connection pool for each key. - * - * @details This way getting a connection for a specific host can be done in O(1) instead of - * looping a single connection list to find the first connection for the required host. - * - * @remark There might be multiple connections for each host. - */ - AZ_CORE_DLLEXPORT static std:: - map>> - ConnectionPoolIndex; + ~CurlConnectionPool() + { + using namespace Azure::Core::Http::_detail; + if (m_cleanThread.joinable()) + { + { + std::unique_lock lock(ConnectionPoolMutex); + // Remove all connections + g_curlConnectionPool.ConnectionPoolIndex.clear(); + } + // Signal clean thread to wake up + ConditionalVariableForCleanThread.notify_one(); + // join thread + m_cleanThread.join(); + } + curl_global_cleanup(); + } /** * @brief Finds a connection to be re-used from the connection pool. @@ -81,7 +78,7 @@ namespace Azure { namespace Core { namespace Http { namespace _detail { * * @return #Azure::Core::Http::CurlNetworkConnection to use. */ - static std::unique_ptr ExtractOrCreateCurlConnection( + std::unique_ptr ExtractOrCreateCurlConnection( Request& request, CurlTransportOptions const& options, bool resetPool = false); @@ -92,51 +89,39 @@ namespace Azure { namespace Core { namespace Http { namespace _detail { * @param connection CURL HTTP connection to add to the pool. * @param lastStatusCode The most recent HTTP status code received from the \p connection. */ - static void MoveConnectionBackToPool( + void MoveConnectionBackToPool( std::unique_ptr connection, HttpStatusCode lastStatusCode); - // Class can't have instances. - CurlConnectionPool() = delete; + /** + * @brief Keeps a unique key for each host and creates a connection pool for each key. + * + * @details This way getting a connection for a specific host can be done in O(1) instead of + * looping a single connection list to find the first connection for the required host. + * + * @remark There might be multiple connections for each host. + */ + std::map>> ConnectionPoolIndex; - static void StopCleaner() { g_isCleanConnectionsRunning = false; } + std::mutex ConnectionPoolMutex; + + // This is used to put the cleaning pool thread to sleep and yet to be able to wake it if the + // application finishes. + std::condition_variable ConditionalVariableForCleanThread; + + AZ_CORE_DLLEXPORT static Azure::Core::Http::_detail::CurlConnectionPool g_curlConnectionPool; + + bool IsCleanThreadRunning = false; private: - /** - * Review all connections in the pool and removes old connections that might be already - * expired and closed its connection on server side. - */ - static void CleanUp(); - - AZ_CORE_DLLEXPORT static uint64_t g_connectionCounter; - AZ_CORE_DLLEXPORT static std::atomic g_isCleanConnectionsRunning; - // Removes all connections and indexes - static void ClearIndex() { CurlConnectionPool::ConnectionPoolIndex.clear(); } + // private constructor to keep this as singleton. + CurlConnectionPool() { curl_global_init(CURL_GLOBAL_ALL); } // Makes possible to know the number of current connections in the connection pool for an // index - static int64_t ConnectionsOnPool(std::string const& host) - { - auto& pool = CurlConnectionPool::ConnectionPoolIndex[host]; - return pool.size(); - }; + int64_t ConnectionsOnPool(std::string const& host) { return ConnectionPoolIndex[host].size(); }; - // Makes possible to know the number indexes in the pool - static int64_t ConnectionsIndexOnPool() - { - return CurlConnectionPool::ConnectionPoolIndex.size(); - }; - }; - - struct CurlGlobalStateForAzureSdk - { - CurlGlobalStateForAzureSdk() { curl_global_init(CURL_GLOBAL_ALL); } - - ~CurlGlobalStateForAzureSdk() - { - CurlConnectionPool::StopCleaner(); - curl_global_cleanup(); - } + std::thread m_cleanThread; }; }}}} // namespace Azure::Core::Http::_detail diff --git a/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp b/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp index 0c11cec64..27463cf8e 100644 --- a/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp +++ b/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp @@ -33,6 +33,9 @@ namespace Azure { namespace Core { namespace Http { constexpr static int DefaultCleanerIntervalMilliseconds = 1000 * 90; // 60 sec -> expired connection is when it waits for 60 sec or more and it's not re-used constexpr static int DefaultConnectionExpiredMilliseconds = 1000 * 60; + // Define the maximun allowed connections per host-index in the pool. If this number is reached + // for the host-index, next connections trying to be added to the pool will be ignored. + constexpr static size_t MaxConnectionsPerIndex = 1024; } // namespace _detail /** @@ -62,7 +65,7 @@ namespace Azure { namespace Core { namespace Http { /** * @brief Update last usage time for the connection. */ - virtual void updateLastUsageTime() = 0; + virtual void UpdateLastUsageTime() = 0; /** * @brief Checks whether this CURL connection is expired. @@ -124,7 +127,7 @@ namespace Azure { namespace Core { namespace Http { // into wire #if defined(_MSC_VER) #pragma warning(push) -// C26812: The enum type 'CURLcode' is unscoped. Prefer 'enum class' over 'enum' (Enum.3) +// C26812: The enum type 'CURLcode' is un-scoped. Prefer 'enum class' over 'enum' (Enum.3) #pragma warning(disable : 26812) #endif auto result = curl_easy_getinfo(m_handle, CURLINFO_ACTIVESOCKET, &m_curlSocket); @@ -150,7 +153,7 @@ namespace Azure { namespace Core { namespace Http { /** * @brief Update last usage time for the connection. */ - void updateLastUsageTime() override + void UpdateLastUsageTime() override { this->m_lastUseTime = std::chrono::steady_clock::now(); } diff --git a/sdk/core/azure-core/src/http/curl/curl_session_private.hpp b/sdk/core/azure-core/src/http/curl/curl_session_private.hpp index 86553a0dc..4f61918d3 100644 --- a/sdk/core/azure-core/src/http/curl/curl_session_private.hpp +++ b/sdk/core/azure-core/src/http/curl/curl_session_private.hpp @@ -385,7 +385,7 @@ namespace Azure { namespace Core { namespace Http { // IsEOF will also handle a connection that fail to complete an upload request. if (IsEOF() && m_keepAlive) { - _detail::CurlConnectionPool::MoveConnectionBackToPool( + _detail::CurlConnectionPool::g_curlConnectionPool.MoveConnectionBackToPool( std::move(m_connection), m_lastStatusCode); } } diff --git a/sdk/core/azure-core/test/ut/CMakeLists.txt b/sdk/core/azure-core/test/ut/CMakeLists.txt index c560b85dc..56fc089d9 100644 --- a/sdk/core/azure-core/test/ut/CMakeLists.txt +++ b/sdk/core/azure-core/test/ut/CMakeLists.txt @@ -25,6 +25,10 @@ if(BUILD_TRANSPORT_CURL) SET(CURL_CONNECTION_POOL_TESTS curl_connection_pool.cpp) endif() +if(RUN_LONG_UNIT_TESTS) + add_compile_definitions(RUN_LONG_UNIT_TESTS) +endif() + include(GoogleTest) add_executable ( diff --git a/sdk/core/azure-core/test/ut/azure_libcurl_core_main.cpp b/sdk/core/azure-core/test/ut/azure_libcurl_core_main.cpp index 7131415e8..a7baf2f2a 100644 --- a/sdk/core/azure-core/test/ut/azure_libcurl_core_main.cpp +++ b/sdk/core/azure-core/test/ut/azure_libcurl_core_main.cpp @@ -40,9 +40,8 @@ namespace Azure { namespace Core { namespace Test { { // Creating a new connection with default options Azure::Core::Http::CurlTransportOptions options; - auto connection - = Azure::Core::Http::_detail::CurlConnectionPool::ExtractOrCreateCurlConnection( - req, options); + auto connection = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ExtractOrCreateCurlConnection(req, options); auto session = std::make_unique( req, std::move(connection), options.HttpKeepAlive); @@ -51,7 +50,10 @@ namespace Azure { namespace Core { namespace Test { session->ReadToEnd(Azure::Core::Context::GetApplicationContext()); } // Check that after the connection is gone, it is moved back to the pool - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 1); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 1); } }}} // namespace Azure::Core::Test diff --git a/sdk/core/azure-core/test/ut/curl_connection_pool.cpp b/sdk/core/azure-core/test/ut/curl_connection_pool.cpp index 7af20cb45..23eee1397 100644 --- a/sdk/core/azure-core/test/ut/curl_connection_pool.cpp +++ b/sdk/core/azure-core/test/ut/curl_connection_pool.cpp @@ -21,7 +21,11 @@ #include #include +#include "curl_session.hpp" + using testing::ValuesIn; +using namespace Azure::Core::Http::_detail; +using namespace std::chrono_literals; namespace Azure { namespace Core { namespace Test { @@ -30,9 +34,13 @@ namespace Azure { namespace Core { namespace Test { TEST(CurlConnectionPool, connectionPoolTest) { - Azure::Core::Http::_detail::CurlConnectionPool::ClearIndex(); - // Make sure there are nothing in the pool - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 0); + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.clear(); + // Make sure there are nothing in the pool + EXPECT_EQ(CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.size(), 0); + } // Use the same request for all connections. Azure::Core::Http::Request req( @@ -44,8 +52,7 @@ namespace Azure { namespace Core { namespace Test { // Creating a new connection with default options Azure::Core::Http::CurlTransportOptions options; auto connection - = Azure::Core::Http::_detail::CurlConnectionPool::ExtractOrCreateCurlConnection( - req, options); + = CurlConnectionPool::g_curlConnectionPool.ExtractOrCreateCurlConnection(req, options); EXPECT_EQ(connection->GetConnectionKey(), expectedConnectionKey); @@ -56,23 +63,33 @@ namespace Azure { namespace Core { namespace Test { session->m_sessionState = Azure::Core::Http::CurlSession::SessionState::STREAMING; } // Check that after the connection is gone, it is moved back to the pool - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 1); - auto connectionFromPool - = Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.begin() - ->second.begin() - ->get(); - EXPECT_EQ(connectionFromPool->GetConnectionKey(), expectedConnectionKey); + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 1); + auto connectionFromPool + = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.begin() + ->second.begin() + ->get(); + EXPECT_EQ(connectionFromPool->GetConnectionKey(), expectedConnectionKey); + } // Test that asking a connection with same config will re-use the same connection { // Creating a new connection with default options Azure::Core::Http::CurlTransportOptions options; auto connection - = Azure::Core::Http::_detail::CurlConnectionPool::ExtractOrCreateCurlConnection( - req, options); + = CurlConnectionPool::g_curlConnectionPool.ExtractOrCreateCurlConnection(req, options); // There was just one connection in the pool, it should be empty now - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 0); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 0); // And the connection key for the connection we got is the expected EXPECT_EQ(connection->GetConnectionKey(), expectedConnectionKey); @@ -82,11 +99,19 @@ namespace Azure { namespace Core { namespace Test { session->m_lastStatusCode = Azure::Core::Http::HttpStatusCode::Ok; session->m_sessionState = Azure::Core::Http::CurlSession::SessionState::STREAMING; } - // Check that after the connection is gone, it is moved back to the pool - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 1); - auto values = Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.begin(); - EXPECT_EQ(values->second.size(), 1); - EXPECT_EQ(values->second.begin()->get()->GetConnectionKey(), expectedConnectionKey); + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + // Check that after the connection is gone, it is moved back to the pool + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 1); + auto values = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.begin(); + EXPECT_EQ(values->second.size(), 1); + EXPECT_EQ(values->second.begin()->get()->GetConnectionKey(), expectedConnectionKey); + } // Now test that using a different connection config won't re-use the same connection std::string const secondExpectedKey @@ -96,14 +121,17 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Http::CurlTransportOptions options; options.SslVerifyPeer = false; auto connection - = Azure::Core::Http::_detail::CurlConnectionPool::ExtractOrCreateCurlConnection( - req, options); + = CurlConnectionPool::g_curlConnectionPool.ExtractOrCreateCurlConnection(req, options); EXPECT_EQ(connection->GetConnectionKey(), secondExpectedKey); // One connection still in the pool after getting a new connection and with first expected // key - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 1); EXPECT_EQ( - Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.begin() + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 1); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .begin() ->second.begin() ->get() ->GetConnectionKey(), @@ -117,27 +145,38 @@ namespace Azure { namespace Core { namespace Test { } // Now there should be 2 index wit one connection each - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 2); - values = Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.begin(); - EXPECT_EQ(values->second.size(), 1); - EXPECT_EQ(values->second.begin()->get()->GetConnectionKey(), secondExpectedKey); - values++; - EXPECT_EQ(values->second.size(), 1); - EXPECT_EQ(values->second.begin()->get()->GetConnectionKey(), expectedConnectionKey); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 2); + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + auto values = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.begin(); + EXPECT_EQ(values->second.size(), 1); + EXPECT_EQ(values->second.begin()->get()->GetConnectionKey(), secondExpectedKey); + values++; + EXPECT_EQ(values->second.size(), 1); + EXPECT_EQ(values->second.begin()->get()->GetConnectionKey(), expectedConnectionKey); + } // Test re-using same custom config { // Creating a new connection with default options Azure::Core::Http::CurlTransportOptions options; auto connection - = Azure::Core::Http::_detail::CurlConnectionPool::ExtractOrCreateCurlConnection( - req, options); + = CurlConnectionPool::g_curlConnectionPool.ExtractOrCreateCurlConnection(req, options); EXPECT_EQ(connection->GetConnectionKey(), expectedConnectionKey); // One connection still in the pool after getting a new connection and with first expected // key - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 1); EXPECT_EQ( - Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.begin() + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 1); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .begin() ->second.begin() ->get() ->GetConnectionKey(), @@ -150,44 +189,206 @@ namespace Azure { namespace Core { namespace Test { session->m_sessionState = Azure::Core::Http::CurlSession::SessionState::STREAMING; } // Now there should be 2 index wit one connection each - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 2); - values = Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.begin(); - EXPECT_EQ(values->second.size(), 1); - EXPECT_EQ(values->second.begin()->get()->GetConnectionKey(), secondExpectedKey); - values++; - EXPECT_EQ(values->second.size(), 1); - EXPECT_EQ(values->second.begin()->get()->GetConnectionKey(), expectedConnectionKey); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 2); + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + auto values = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.begin(); + EXPECT_EQ(values->second.size(), 1); + EXPECT_EQ(values->second.begin()->get()->GetConnectionKey(), secondExpectedKey); + values++; + EXPECT_EQ(values->second.size(), 1); + EXPECT_EQ(values->second.begin()->get()->GetConnectionKey(), expectedConnectionKey); + } + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + // clean the pool + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.clear(); + } #ifdef RUN_LONG_UNIT_TESTS { - // Test pool clean routine - std::cout << "Running Connection Pool Cleaner Test. This test can take up to 2 minutes to " - "complete." - << std::endl - << "Add compiler option -DRUN_LONG_UNIT_TESTS=OFF when building if you want to " - "skip this test." - << std::endl; - - // Wait for 100 secs to make sure connections are removed. - // Connection need to be in the pool for more than 60 sec to consider it expired. - // Clean routine runs every 90 secs. - std::this_thread::sleep_for(std::chrono::milliseconds(1000 * 100)); - - // Ensure connections are removed but indexes are still there - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 2); - values = Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.begin(); - EXPECT_EQ(values->second.size(), 0); - values++; - EXPECT_EQ(values->second.size(), 0); + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + // clean the pool + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.clear(); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 0); } + + // Test pool clean routine. + std::cout << "Running Connection Pool Cleaner Test. This test can take up to 2 minutes to " + "complete." + << std::endl + << "Add compiler option -DRUN_LONG_UNIT_TESTS=OFF when building if you want to " + "skip this test." + << std::endl; + { + // Make sure the clean pool thread is started by adding 5 connections to the pool + std::vector> connections; + for (int count = 0; count < 5; count++) + { + connections.emplace_back( + CurlConnectionPool::g_curlConnectionPool.ExtractOrCreateCurlConnection(req, {})); + } + for (int count = 0; count < 5; count++) + { + CurlConnectionPool::g_curlConnectionPool.MoveConnectionBackToPool( + std::move(connections[count]), Http::HttpStatusCode::Ok); + } + } + + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 1); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex[expectedConnectionKey] + .size(), + 5); + } + + // Wait for 60 secs (default time to expire a connection) + std::this_thread::sleep_for(60ms); + + { + // Now check the pool until the clean thread until finishes removing the connections or + // fail after 5 minutes (indicates a problem with the clean routine) + + auto timeOut = Context::GetApplicationContext().WithDeadline( + std::chrono::system_clock::now() + 5min); + bool poolIsEmpty = false; + while (!poolIsEmpty && !timeOut.IsCancelled()) + { + std::this_thread::sleep_for(10ms); + // If test wakes while clean pool is running, it will wait until lock is released by + // the clean pool thread. + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + poolIsEmpty = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.size() + == 0; + } + EXPECT_TRUE(poolIsEmpty); + } + #endif + // Test max connections in pool. Try to add 2k connections to the pool. + // Using fake connections to avoid opening real http connections :) + // { + // using ::testing::_; + // using ::testing::Return; + // using ::testing::ReturnRef; + + // { + // std::lock_guard lock( + // CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + // // clean the pool + // CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.clear(); + // } + + // std::string hostKey("key"); + // for (uint64_t count = 0; count < 2000; count++) + // { + // MockCurlNetworkConnection* curlMock = new MockCurlNetworkConnection(); + // EXPECT_CALL(*curlMock, GetConnectionKey()).WillRepeatedly(ReturnRef(hostKey)); + // EXPECT_CALL(*curlMock, UpdateLastUsageTime()).WillRepeatedly(Return()); + // EXPECT_CALL(*curlMock, IsExpired()).WillRepeatedly(Return(false)); + // EXPECT_CALL(*curlMock, ReadFromSocket(_, _, _)).WillRepeatedly(Return(count)); + // EXPECT_CALL(*curlMock, DestructObj()); + + // CurlConnectionPool::g_curlConnectionPool.MoveConnectionBackToPool( + // std::unique_ptr(curlMock), + // Azure::Core::Http::HttpStatusCode::Ok); + // } + // // No need to take look here because connections are mocked to never be expired. + // EXPECT_EQ( + // Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + // .size(), + // 1); + // EXPECT_EQ( + // Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + // .ConnectionPoolIndex[hostKey] + // .size(), + // Azure::Core::Http::_detail::MaxConnectionsPerIndex); + // // Test the first and last connection. Each connection should remove the last and + // oldest auto connectionIt = + // Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + // .ConnectionPoolIndex[hostKey] + // .begin(); + // EXPECT_EQ( + // connectionIt->get()->ReadFromSocket(nullptr, 0, Context::GetApplicationContext()), + // 2000 - 1); // starting from zero + // connectionIt = --(Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + // .ConnectionPoolIndex[hostKey] + // .end()); + // EXPECT_EQ( + // connectionIt->get()->ReadFromSocket(nullptr, 0, Context::GetApplicationContext()), + // 2000 - 1024); + + // // Check the pool will take other host-key + // { + // std::string otherKey("otherHostKey"); + // MockCurlNetworkConnection* curlMock = new MockCurlNetworkConnection(); + // EXPECT_CALL(*curlMock, GetConnectionKey()).WillRepeatedly(ReturnRef(otherKey)); + // EXPECT_CALL(*curlMock, UpdateLastUsageTime()).WillRepeatedly(Return()); + // EXPECT_CALL(*curlMock, IsExpired()).WillRepeatedly(Return(false)); + // EXPECT_CALL(*curlMock, DestructObj()); + + // CurlConnectionPool::g_curlConnectionPool.MoveConnectionBackToPool( + // std::unique_ptr(curlMock), + // Azure::Core::Http::HttpStatusCode::Ok); + + // EXPECT_EQ( + // Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + // .ConnectionPoolIndex.size(), + // 2); + // EXPECT_EQ( + // Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + // .ConnectionPoolIndex[otherKey] + // .size(), + // 1); + // // No changes to the full pool + // EXPECT_EQ( + // Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + // .ConnectionPoolIndex[hostKey] + // .size(), + // Azure::Core::Http::_detail::MaxConnectionsPerIndex); + // } + // { + // std::lock_guard lock( + // CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + // // clean the pool + // CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex.clear(); + // } + // } } TEST(CurlConnectionPool, uniquePort) { - Azure::Core::Http::_detail::CurlConnectionPool::ClearIndex(); - // Make sure there is nothing in the pool - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 0); + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .clear(); + // Make sure there is nothing in the pool + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 0); + } + { // Request with no port std::string const authority(AzureSdkHttpbinServer::Get()); @@ -197,18 +398,32 @@ namespace Azure { namespace Core { namespace Test { = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "0011"; // Creating a new connection with default options - auto connection - = Azure::Core::Http::_detail::CurlConnectionPool::ExtractOrCreateCurlConnection( - req, {}); + auto connection = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ExtractOrCreateCurlConnection(req, {}); - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 0); - EXPECT_EQ(connection->GetConnectionKey(), expectedConnectionKey); + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.size(), + 0); + EXPECT_EQ(connection->GetConnectionKey(), expectedConnectionKey); + } // move connection back to the pool - Azure::Core::Http::_detail::CurlConnectionPool::MoveConnectionBackToPool( - std::move(connection), Azure::Core::Http::HttpStatusCode::Ok); + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .MoveConnectionBackToPool(std::move(connection), Azure::Core::Http::HttpStatusCode::Ok); + } + + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + // Test connection was moved to the pool + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 1); } - // Test connection was moved to the pool - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 1); { // Request with port @@ -219,19 +434,32 @@ namespace Azure { namespace Core { namespace Test { = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "4430011"; // Creating a new connection with default options - auto connection - = Azure::Core::Http::_detail::CurlConnectionPool::ExtractOrCreateCurlConnection( - req, {}); + auto connection = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ExtractOrCreateCurlConnection(req, {}); EXPECT_EQ(connection->GetConnectionKey(), expectedConnectionKey); - // Check connection in pool is not re-used because the port is different - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 1); + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + // Check connection in pool is not re-used because the port is different + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.size(), + 1); + } // move connection back to the pool - Azure::Core::Http::_detail::CurlConnectionPool::MoveConnectionBackToPool( - std::move(connection), Azure::Core::Http::HttpStatusCode::Ok); + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .MoveConnectionBackToPool(std::move(connection), Azure::Core::Http::HttpStatusCode::Ok); + } + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + // Check 2 connections in the pool + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 2); } - // Check 2 connections in the pool - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 2); // Re-use connections { @@ -243,17 +471,32 @@ namespace Azure { namespace Core { namespace Test { = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "0011"; // Creating a new connection with default options - auto connection - = Azure::Core::Http::_detail::CurlConnectionPool::ExtractOrCreateCurlConnection( - req, {}); + auto connection = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ExtractOrCreateCurlConnection(req, {}); - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 1); + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.size(), + 1); + } EXPECT_EQ(connection->GetConnectionKey(), expectedConnectionKey); // move connection back to the pool - Azure::Core::Http::_detail::CurlConnectionPool::MoveConnectionBackToPool( - std::move(connection), Azure::Core::Http::HttpStatusCode::Ok); + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .MoveConnectionBackToPool(std::move(connection), Azure::Core::Http::HttpStatusCode::Ok); + } + + { + // Make sure there is nothing in the pool + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 2); } - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 2); { // Request with port std::string const authority(AzureSdkHttpbinServer::WithPort()); @@ -263,19 +506,33 @@ namespace Azure { namespace Core { namespace Test { = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "4430011"; // Creating a new connection with default options - auto connection - = Azure::Core::Http::_detail::CurlConnectionPool::ExtractOrCreateCurlConnection( - req, {}); + auto connection = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ExtractOrCreateCurlConnection(req, {}); EXPECT_EQ(connection->GetConnectionKey(), expectedConnectionKey); - // Check connection in pool is not re-used because the port is different - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 1); + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + // Check connection in pool is not re-used because the port is different + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.size(), + 1); + } // move connection back to the pool - Azure::Core::Http::_detail::CurlConnectionPool::MoveConnectionBackToPool( - std::move(connection), Azure::Core::Http::HttpStatusCode::Ok); + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .MoveConnectionBackToPool(std::move(connection), Azure::Core::Http::HttpStatusCode::Ok); + } + { + std::lock_guard lock( + CurlConnectionPool::g_curlConnectionPool.ConnectionPoolMutex); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 2); + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .clear(); } - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 2); - Azure::Core::Http::_detail::CurlConnectionPool::ClearIndex(); } TEST(CurlConnectionPool, resiliencyOnConnectionClosed) @@ -285,8 +542,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Http::CurlTransportOptions options; auto connection - = Azure::Core::Http::_detail::CurlConnectionPool::ExtractOrCreateCurlConnection( - req, options); + = CurlConnectionPool::g_curlConnectionPool.ExtractOrCreateCurlConnection(req, options); // Simulate connection lost (like server disconnection). connection->Shutdown(); diff --git a/sdk/core/azure-core/test/ut/curl_options.cpp b/sdk/core/azure-core/test/ut/curl_options.cpp index c6749f14f..fc9ad416f 100644 --- a/sdk/core/azure-core/test/ut/curl_options.cpp +++ b/sdk/core/azure-core/test/ut/curl_options.cpp @@ -90,7 +90,8 @@ namespace Azure { namespace Core { namespace Test { // Clean the connection from the pool *Windows fails to clean if we leave to be clean uppon // app-destruction - EXPECT_NO_THROW(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.clear()); + EXPECT_NO_THROW(Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.clear()); } /* @@ -208,7 +209,8 @@ namespace Azure { namespace Core { namespace Test { // Clean the connection from the pool *Windows fails to clean if we leave to be clean uppon // app-destruction - EXPECT_NO_THROW(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.clear()); + EXPECT_NO_THROW(Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.clear()); } TEST(CurlTransportOptions, httpsDefault) @@ -238,9 +240,10 @@ namespace Azure { namespace Core { namespace Test { static_cast::type>( responseCode)); - // Clean the connection from the pool *Windows fails to clean if we leave to be clean uppon + // Clean the connection from the pool *Windows fails to clean if we leave to be clean upon // app-destruction - EXPECT_NO_THROW(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.clear()); + EXPECT_NO_THROW(Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool + .ConnectionPoolIndex.clear()); } TEST(CurlTransportOptions, disableKeepAlive) @@ -276,7 +279,10 @@ namespace Azure { namespace Core { namespace Test { responseCode)); } // Make sure there are no connections in the pool - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 0); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 0); } }}} // namespace Azure::Core::Test diff --git a/sdk/core/azure-core/test/ut/curl_session.hpp b/sdk/core/azure-core/test/ut/curl_session.hpp index e494b7886..50d2e0e12 100644 --- a/sdk/core/azure-core/test/ut/curl_session.hpp +++ b/sdk/core/azure-core/test/ut/curl_session.hpp @@ -38,7 +38,7 @@ namespace Azure { namespace Core { namespace Test { class MockCurlNetworkConnection : public Azure::Core::Http::CurlNetworkConnection { public: MOCK_METHOD(std::string const&, GetConnectionKey, (), (const, override)); - MOCK_METHOD(void, updateLastUsageTime, (), (override)); + MOCK_METHOD(void, UpdateLastUsageTime, (), (override)); MOCK_METHOD(bool, IsExpired, (), (override)); MOCK_METHOD( int64_t, diff --git a/sdk/core/azure-core/test/ut/curl_session_test.cpp b/sdk/core/azure-core/test/ut/curl_session_test.cpp index f8cfeaf65..4ad6e9df9 100644 --- a/sdk/core/azure-core/test/ut/curl_session_test.cpp +++ b/sdk/core/azure-core/test/ut/curl_session_test.cpp @@ -61,7 +61,7 @@ namespace Azure { namespace Core { namespace Test { SetArrayArgument<0>(response.data(), response.data() + response.size()), Return(response.size()))); EXPECT_CALL(*curlMock, GetConnectionKey()).WillRepeatedly(ReturnRef(connectionKey)); - EXPECT_CALL(*curlMock, updateLastUsageTime()); + EXPECT_CALL(*curlMock, UpdateLastUsageTime()); EXPECT_CALL(*curlMock, DestructObj()); // Create the unique ptr to take care about memory free at the end @@ -79,7 +79,8 @@ namespace Azure { namespace Core { namespace Test { EXPECT_NO_THROW(session->Perform(Azure::Core::Context::GetApplicationContext())); } // Clear the connections from the pool to invoke clean routine - Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.clear(); + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .clear(); } TEST_F(CurlSession, chunkBadFormatResponse) @@ -101,7 +102,7 @@ namespace Azure { namespace Core { namespace Test { SetArrayArgument<0>(response2.data(), response2.data() + response2.size()), Return(response2.size()))); EXPECT_CALL(*curlMock, GetConnectionKey()).WillRepeatedly(ReturnRef(connectionKey)); - EXPECT_CALL(*curlMock, updateLastUsageTime()); + EXPECT_CALL(*curlMock, UpdateLastUsageTime()); EXPECT_CALL(*curlMock, DestructObj()); // Create the unique ptr to take care about memory free at the end @@ -127,7 +128,8 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Http::TransportException); } // Clear the connections from the pool to invoke clean routine - Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.clear(); + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .clear(); } TEST_F(CurlSession, chunkSegmentedResponse) @@ -178,7 +180,7 @@ namespace Azure { namespace Core { namespace Test { SetArrayArgument<0>(response8.data(), response8.data() + response8.size()), Return(response8.size()))); EXPECT_CALL(*curlMock, GetConnectionKey()).WillRepeatedly(ReturnRef(connectionKey)); - EXPECT_CALL(*curlMock, updateLastUsageTime()); + EXPECT_CALL(*curlMock, UpdateLastUsageTime()); EXPECT_CALL(*curlMock, DestructObj()); // Create the unique ptr to take care about memory free at the end @@ -202,12 +204,14 @@ namespace Azure { namespace Core { namespace Test { EXPECT_NO_THROW(bodyS->ReadToEnd(Azure::Core::Context::GetApplicationContext())); } // Clear the connections from the pool to invoke clean routine - Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.clear(); + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .clear(); } TEST_F(CurlSession, DoNotReuseConnectionIfDownloadFail) { - + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .clear(); // Can't mock the curlMock directly from a unique ptr, heap allocate it first and then make a // unique ptr for it MockCurlNetworkConnection* curlMock = new MockCurlNetworkConnection(); @@ -231,6 +235,9 @@ namespace Azure { namespace Core { namespace Test { EXPECT_EQ(CURLE_SEND_ERROR, returnCode); } // Check connection pool is empty (connection was not moved to the pool) - EXPECT_EQ(Azure::Core::Http::_detail::CurlConnectionPool::ConnectionPoolIndex.size(), 0); + EXPECT_EQ( + Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool.ConnectionPoolIndex + .size(), + 0); } }}} // namespace Azure::Core::Test