From 91bded264cad7840a245e174d90f1dcb2a045c0c Mon Sep 17 00:00:00 2001 From: SergeyRyabinin Date: Tue, 19 Nov 2024 20:09:10 +0000 Subject: [PATCH] Fix #3167: avoid infinite wait on failure to init curl handle --- .../include/aws/core/utils/ResourceManager.h | 26 +++++++++++++++++++ .../source/http/curl/CurlHandleContainer.cpp | 20 +++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/aws-cpp-sdk-core/include/aws/core/utils/ResourceManager.h b/src/aws-cpp-sdk-core/include/aws/core/utils/ResourceManager.h index 51dd59623fc..a9b193f7c5c 100644 --- a/src/aws-cpp-sdk-core/include/aws/core/utils/ResourceManager.h +++ b/src/aws-cpp-sdk-core/include/aws/core/utils/ResourceManager.h @@ -74,6 +74,31 @@ namespace Aws return resource; } + /** + * Returns a resource with exclusive ownership or a nullptr. + * If resource is available within the wait timeout then the resource is returned. + * otherwise (if the timeout has expired or container is shutdown) the nullptr is returned. + * You must call Release on the resource when you are finished. + * This method is enabled only for pointer RESOURCE_TYPE type. + * + * @return instance of RESOURCE_TYPE, nullptr if the resource manager is being shutdown + */ + template::value>::type * = nullptr> + RESOURCE_TYPE TryAcquire(const uint64_t timeoutMs) { + std::unique_lock locker(m_queueLock); + const bool isNotEmpty = m_semaphore.wait_for(locker, std::chrono::milliseconds(timeoutMs), + [&]() { return m_shutdown.load() || !m_resources.empty(); }); + + if (m_shutdown || !isNotEmpty) { + return nullptr; + } + + RESOURCE_TYPE resource = m_resources.back(); + m_resources.pop_back(); + + return resource; + } + /** * Returns whether or not resources are currently available for acquisition * @@ -122,6 +147,7 @@ namespace Aws { std::unique_lock locker(m_queueLock); m_shutdown = true; + m_semaphore.notify_all(); //wait for all acquired resources to be released. while (m_resources.size() < resourceCount) diff --git a/src/aws-cpp-sdk-core/source/http/curl/CurlHandleContainer.cpp b/src/aws-cpp-sdk-core/source/http/curl/CurlHandleContainer.cpp index dc64e905142..03d2d5d1930 100644 --- a/src/aws-cpp-sdk-core/source/http/curl/CurlHandleContainer.cpp +++ b/src/aws-cpp-sdk-core/source/http/curl/CurlHandleContainer.cpp @@ -32,6 +32,7 @@ CurlHandleContainer::~CurlHandleContainer() AWS_LOGSTREAM_DEBUG(CURL_HANDLE_CONTAINER_TAG, "Cleaning up " << handle); curl_easy_cleanup(handle); } + m_poolSize = 0; } CURL* CurlHandleContainer::AcquireCurlHandle() @@ -44,7 +45,19 @@ CURL* CurlHandleContainer::AcquireCurlHandle() CheckAndGrowPool(); } - CURL* handle = m_handleContainer.TryAcquire(); + // TODO: 1.12: start to fail instead of infinite loop + CURL* handle = nullptr; + bool errorLogged = false; // avoid log explosion on legacy app behavior + while (!handle && m_poolSize) { + constexpr unsigned long DEFAULT_TIMEOUT = 1000l; + handle = m_handleContainer.TryAcquire(std::max(m_connectTimeout, DEFAULT_TIMEOUT)); + if (!handle && !errorLogged) { + AWS_LOGSTREAM_ERROR(CURL_HANDLE_CONTAINER_TAG, "Unable to Acquire a curl handle within the specified connect timeout: " + << m_connectTimeout << ". Waiting further, this method will start failing in 1.12.x. " + "Please increase the pool size or the connect timeout value."); + errorLogged = true; + } + } AWS_LOGSTREAM_DEBUG(CURL_HANDLE_CONTAINER_TAG, "Connection has been released. Continuing."); AWS_LOGSTREAM_DEBUG(CURL_HANDLE_CONTAINER_TAG, "Returning connection handle " << handle); return handle; @@ -80,10 +93,15 @@ void CurlHandleContainer::DestroyCurlHandle(CURL* handle) // If the handle is not released back to the pool, it could create a deadlock // Create a new handle and release that into the pool handle = CreateCurlHandleInPool(); + if (!handle && m_poolSize) { + m_poolSize -= 1; + } } if (handle) { AWS_LOGSTREAM_DEBUG(CURL_HANDLE_CONTAINER_TAG, "Created replacement handle and released to pool: " << handle); + } else { + AWS_LOGSTREAM_ERROR(CURL_HANDLE_CONTAINER_TAG, "Failed to create a replacement handle. The handle pool size reduced to " << m_poolSize); } }