edg-l/ddnet
1
0
Fork 0
mirror of https://github.com/ddnet/ddnet.git synced 2024-09-19 17:14:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix HTTP client shutdown and deadlock on request error

Browse source 
Fix HTTP client effectively shutting down by entering the `ERROR` state when a request cannot be added. Now only the invalid request is aborted immediately. The `ERROR` state is now only entered when a curl function fails in a way where recovery is not possible.

Fix occasional deadlock when HTTP client is shutting down after entering the `ERROR` state, by also immediately aborting new requests when the HTTP client is already in the `ERROR` state.

Remove unused `CHttp::EState::STOPPING`.
This commit is contained in:
Robert Müller 2024-03-04 23:29:01 +01:00
parent f3de37d9d4
commit 9323e18616
2 changed files with 16 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
src/engine/shared/http.cpp
View file

@ -471,7 +471,7 @@ void CHttp::RunLoop()
if(PollCode != CURLM_OK) if(PollCode != CURLM_OK)
{ {
Lock.lock(); Lock.lock();
log_error("http", "Failed multi wait: %s", curl_multi_strerror(PollCode)); log_error("http", "curl_multi_poll failed: %s", curl_multi_strerror(PollCode));
m_State = CHttp::ERROR; m_State = CHttp::ERROR;
break; break;
} }
@ -480,7 +480,7 @@ void CHttp::RunLoop()
if(PerformCode != CURLM_OK) if(PerformCode != CURLM_OK)
{ {
Lock.lock(); Lock.lock();
log_error("http", "Failed multi perform: %s", curl_multi_strerror(PerformCode)); log_error("http", "curl_multi_perform failed: %s", curl_multi_strerror(PerformCode));
m_State = CHttp::ERROR; m_State = CHttp::ERROR;
break; break;
} }
@ -514,23 +514,33 @@ void CHttp::RunLoop()
CURL *pEH = curl_easy_init(); CURL *pEH = curl_easy_init();
if(!pEH) if(!pEH)
{
log_error("http", "curl_easy_init failed");
goto error_init; goto error_init;
}
if(!pRequest->ConfigureHandle(pEH)) if(!pRequest->ConfigureHandle(pEH))
goto error_configure; {
curl_easy_cleanup(pEH);
str_copy(pRequest->m_aErr, "Failed to initialize request");
pRequest->OnCompletionInternal(nullptr, CURLE_ABORTED_BY_CALLBACK);
NewRequests.pop_front();
continue;
}
if(curl_multi_add_handle(m_pMultiH, pEH) != CURLM_OK) if(curl_multi_add_handle(m_pMultiH, pEH) != CURLM_OK)
{
log_error("http", "curl_multi_add_handle failed");
goto error_configure; goto error_configure;
}
m_RunningRequests.emplace(pEH, std::move(pRequest)); m_RunningRequests.emplace(pEH, std::move(pRequest));
NewRequests.pop_front(); NewRequests.pop_front();
continue; continue;
error_configure: error_configure:
curl_easy_cleanup(pEH); curl_easy_cleanup(pEH);
error_init: error_init:
log_error("http", "failed to start new request");
Lock.lock(); Lock.lock();
m_State = CHttp::ERROR; m_State = CHttp::ERROR;
break; break;
@ -579,7 +589,7 @@ void CHttp::Run(std::shared_ptr<IHttpRequest> pRequest)
{ {
std::shared_ptr<CHttpRequest> pRequestImpl = std::static_pointer_cast<CHttpRequest>(pRequest); std::shared_ptr<CHttpRequest> pRequestImpl = std::static_pointer_cast<CHttpRequest>(pRequest);
std::unique_lock Lock(m_Lock); std::unique_lock Lock(m_Lock);
if(m_Shutdown) if(m_Shutdown || m_State == CHttp::ERROR)
{ {
str_copy(pRequestImpl->m_aErr, "Shutting down"); str_copy(pRequestImpl->m_aErr, "Shutting down");
pRequestImpl->OnCompletionInternal(nullptr, CURLE_ABORTED_BY_CALLBACK); pRequestImpl->OnCompletionInternal(nullptr, CURLE_ABORTED_BY_CALLBACK);

1
src/engine/shared/http.h
View file

@ -255,7 +255,6 @@ class CHttp : public IHttp
{ {
UNINITIALIZED, UNINITIALIZED,
RUNNING, RUNNING,
STOPPING,
ERROR, ERROR,
}; };