Quit client faster by aborting HTTP requests earlier

Shutdown `CHttp`, i.e. abort all HTTP requests, as early as possible when quitting/restarting the client, after the config has been saved but before shutting down the gameclient. Previously, this was delayed until the engine shutdown, so the requests would often finish entirely instead of being aborted by the callback.

Previously, HTTP requests being aborted due to `CHttp` shutting down were considered `EHttpState::ERROR`, which sometimes causes deadlocks during engine shutdown or destruction of `CHttp`. The state is now set to `ABORTED` by passing `CURLE_ABORTED_BY_CALLBACK` to `OnCompletionInternal`. The otherwise unused handling of internal errors is removed.
This commit is contained in:
Robert Müller 2024-02-10 22:16:47 +01:00
parent b2041f5703
commit 9d1ebe726c
3 changed files with 15 additions and 23 deletions

View file

@ -3048,7 +3048,7 @@ void CClient::Run()
}
m_Fifo.Shutdown();
m_Http.Shutdown();
GameClient()->OnShutdown();
Disconnect();

View file

@ -257,33 +257,25 @@ int CHttpRequest::ProgressCallback(void *pUser, double DlTotal, double DlCurr, d
return pTask->m_Abort ? -1 : 0;
}
void CHttpRequest::OnCompletionInternal(std::optional<unsigned int> Result)
void CHttpRequest::OnCompletionInternal(unsigned int Result)
{
EHttpState State;
if(Result.has_value())
const CURLcode Code = static_cast<CURLcode>(Result);
if(Code != CURLE_OK)
{
CURLcode Code = static_cast<CURLcode>(Result.value());
if(Code != CURLE_OK)
if(g_Config.m_DbgCurl || m_LogProgress >= HTTPLOG::FAILURE)
{
if(g_Config.m_DbgCurl || m_LogProgress >= HTTPLOG::FAILURE)
{
log_error("http", "%s failed. libcurl error (%u): %s", m_aUrl, Code, m_aErr);
}
State = (Code == CURLE_ABORTED_BY_CALLBACK) ? EHttpState::ABORTED : EHttpState::ERROR;
}
else
{
if(g_Config.m_DbgCurl || m_LogProgress >= HTTPLOG::ALL)
{
log_info("http", "task done: %s", m_aUrl);
}
State = EHttpState::DONE;
log_error("http", "%s failed. libcurl error (%u): %s", m_aUrl, Code, m_aErr);
}
State = (Code == CURLE_ABORTED_BY_CALLBACK) ? EHttpState::ABORTED : EHttpState::ERROR;
}
else
{
log_error("http", "%s failed. internal error: %s", m_aUrl, m_aErr);
State = EHttpState::ERROR;
if(g_Config.m_DbgCurl || m_LogProgress >= HTTPLOG::ALL)
{
log_info("http", "task done: %s", m_aUrl);
}
State = EHttpState::DONE;
}
if(State == EHttpState::DONE)
@ -543,7 +535,7 @@ void CHttp::RunLoop()
for(auto &pRequest : m_PendingRequests)
{
str_copy(pRequest->m_aErr, "Shutting down");
pRequest->OnCompletionInternal(std::nullopt);
pRequest->OnCompletionInternal(CURLE_ABORTED_BY_CALLBACK);
}
for(auto &ReqPair : m_RunningRequests)
@ -556,7 +548,7 @@ void CHttp::RunLoop()
}
str_copy(pRequest->m_aErr, "Shutting down");
pRequest->OnCompletionInternal(std::nullopt);
pRequest->OnCompletionInternal(CURLE_ABORTED_BY_CALLBACK);
}
if(Cleanup)

View file

@ -118,7 +118,7 @@ class CHttpRequest : public IHttpRequest
// Abort the request with an error if `BeforeInit()` returns false.
bool BeforeInit();
bool ConfigureHandle(void *pHandle); // void * == CURL *
void OnCompletionInternal(std::optional<unsigned int> Result); // unsigned int == CURLcode
void OnCompletionInternal(unsigned int Result); // unsigned int == CURLcode
// Abort the request if `OnData()` returns something other than
// `DataSize`.