Skip to content

Commit 1350601

Browse files
committed
Fix cleanup deadlock caused by re-entrant lock during HTTP request cancellation
When HCCleanupAsync cancels pending HTTP requests via XAsyncCancel(), the AsyncState destructor can be invoked synchronously, which attempts to re-acquire the NetworkState lock (m_mutex) that is already held by CleanupAsyncProvider. This manifests as a deadlock when cleanup is called with active HTTP requests that have pending retries. The fix snapshots the active requests while holding the lock, then releases the lock before calling XAsyncCancel() to avoid re-entrant lock acquisition. Also applies the same pattern to connected websocket disconnection for consistency. Fixes deadlock in TestAsyncCleanupWithHttpCallPendingRetry.
1 parent fcbe2e6 commit 1350601

1 file changed

Lines changed: 18 additions & 7 deletions

File tree

Source/Global/NetworkState.cpp

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -376,19 +376,31 @@ HRESULT CALLBACK NetworkState::CleanupAsyncProvider(XAsyncOp op, const XAsyncPro
376376
{
377377
case XAsyncOp::Begin:
378378
{
379-
std::unique_lock<std::mutex> lock{ state->m_mutex };
380-
state->m_cleanupAsyncBlock = data->async;
381-
bool scheduleCleanup = state->ScheduleCleanup();
379+
std::vector<XAsyncBlock*> activeRequests;
380+
#ifndef HC_NOWEBSOCKETS
381+
std::vector<ActiveWebSocketContext*> connectedWebSockets;
382+
#endif
383+
bool scheduleCleanup = false;
384+
{
385+
std::unique_lock<std::mutex> lock{ state->m_mutex };
386+
state->m_cleanupAsyncBlock = data->async;
387+
scheduleCleanup = state->ScheduleCleanup();
382388

389+
#ifndef HC_NOWEBSOCKETS
390+
HC_TRACE_VERBOSE(HTTPCLIENT, "NetworkState::CleanupAsyncProvider::Begin: HTTP active=%llu, WebSocket Connecting=%llu, WebSocket Connected=%llu", state->m_activeHttpRequests.size(), state->m_connectingWebSockets.size(), state->m_connectedWebSockets.size());
391+
#endif
392+
activeRequests.assign(state->m_activeHttpRequests.begin(), state->m_activeHttpRequests.end());
383393
#ifndef HC_NOWEBSOCKETS
384-
HC_TRACE_VERBOSE(HTTPCLIENT, "NetworkState::CleanupAsyncProvider::Begin: HTTP active=%llu, WebSocket Connecting=%llu, WebSocket Connected=%llu", state->m_activeHttpRequests.size(), state->m_connectingWebSockets.size(), state->m_connectedWebSockets.size());
394+
connectedWebSockets.assign(state->m_connectedWebSockets.begin(), state->m_connectedWebSockets.end());
385395
#endif
386-
for (auto& activeRequest : state->m_activeHttpRequests)
396+
}
397+
398+
for (auto& activeRequest : activeRequests)
387399
{
388400
XAsyncCancel(activeRequest);
389401
}
390402
#ifndef HC_NOWEBSOCKETS
391-
for (auto& context : state->m_connectedWebSockets)
403+
for (auto& context : connectedWebSockets)
392404
{
393405
HRESULT hr = context->websocketObserver->websocket->Disconnect();
394406
if (FAILED(hr))
@@ -397,7 +409,6 @@ HRESULT CALLBACK NetworkState::CleanupAsyncProvider(XAsyncOp op, const XAsyncPro
397409
}
398410
}
399411
#endif
400-
lock.unlock();
401412

402413
if (scheduleCleanup)
403414
{

0 commit comments

Comments
 (0)