Skip to content

Commit

Permalink
Fix #3167: avoid infinite wait on failure to init curl handle
Browse files Browse the repository at this point in the history
  • Loading branch information
SergeyRyabinin committed Nov 19, 2024
1 parent 710e6c5 commit 91bded2
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
26 changes: 26 additions & 0 deletions src/aws-cpp-sdk-core/include/aws/core/utils/ResourceManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<typename std::enable_if<std::is_pointer<RESOURCE_TYPE>::value>::type * = nullptr>
RESOURCE_TYPE TryAcquire(const uint64_t timeoutMs) {
std::unique_lock<std::mutex> 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
*
Expand Down Expand Up @@ -122,6 +147,7 @@ namespace Aws
{
std::unique_lock<std::mutex> locker(m_queueLock);
m_shutdown = true;
m_semaphore.notify_all();

//wait for all acquired resources to be released.
while (m_resources.size() < resourceCount)
Expand Down
20 changes: 19 additions & 1 deletion src/aws-cpp-sdk-core/source/http/curl/CurlHandleContainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 91bded2

Please sign in to comment.