From 1dc84964709e40afa088f98085d631cc571af116 Mon Sep 17 00:00:00 2001 From: Learath Date: Mon, 18 Dec 2023 20:01:26 +0100 Subject: [PATCH 1/8] Use curl-multi --- CMakeLists.txt | 1 + src/base/system.h | 4 +- src/engine/client/client.cpp | 11 +- src/engine/client/client.h | 2 + src/engine/client/serverbrowser.cpp | 5 +- src/engine/client/serverbrowser.h | 2 + src/engine/client/serverbrowser_http.cpp | 52 ++- src/engine/client/serverbrowser_http.h | 3 +- src/engine/client/updater.cpp | 28 +- src/engine/client/updater.h | 3 +- src/engine/http.h | 16 + src/engine/shared/http.cpp | 343 +++++++++++++------ src/engine/shared/http.h | 78 ++++- src/game/client/component.cpp | 2 + src/game/client/component.h | 5 + src/game/client/components/menus.h | 14 +- src/game/client/components/menus_browser.cpp | 31 +- src/game/client/components/skins.cpp | 16 +- src/game/client/components/skins.h | 2 +- src/game/client/gameclient.cpp | 1 + src/game/client/gameclient.h | 2 + 21 files changed, 429 insertions(+), 192 deletions(-) create mode 100644 src/engine/http.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 239f3a185..6d321f11e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1883,6 +1883,7 @@ set_src(ENGINE_INTERFACE GLOB src/engine friends.h ghost.h graphics.h + http.h input.h kernel.h keys.h diff --git a/src/base/system.h b/src/base/system.h index c575949e6..919cbc6eb 100644 --- a/src/base/system.h +++ b/src/base/system.h @@ -2524,9 +2524,9 @@ int kill_process(PROCESS process); /** * Checks if a process is alive. - * + * * @param process Handle/PID of the process. - * + * * @return bool Returns true if the process is currently running, false if the process is not running (dead). */ bool is_process_alive(PROCESS process); diff --git a/src/engine/client/client.cpp b/src/engine/client/client.cpp index e79597092..3eb18ed4d 100644 --- a/src/engine/client/client.cpp +++ b/src/engine/client/client.cpp @@ -1447,7 +1447,7 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy) m_pMapdownloadTask->Timeout(CTimeout{g_Config.m_ClMapDownloadConnectTimeoutMs, 0, g_Config.m_ClMapDownloadLowSpeedLimit, g_Config.m_ClMapDownloadLowSpeedTime}); m_pMapdownloadTask->MaxResponseSize(1024 * 1024 * 1024); // 1 GiB m_pMapdownloadTask->ExpectSha256(*pMapSha256); - Engine()->AddJob(m_pMapdownloadTask); + Http()->Run(m_pMapdownloadTask); } else { @@ -2664,6 +2664,7 @@ void CClient::RegisterInterfaces() #endif Kernel()->RegisterInterface(static_cast(&m_Friends), false); Kernel()->ReregisterInterface(static_cast(&m_Foes)); + Kernel()->RegisterInterface(static_cast(&m_Http), false); } void CClient::InitInterfaces() @@ -2688,12 +2689,12 @@ void CClient::InitInterfaces() m_DemoEditor.Init(m_pGameClient->NetVersion(), &m_SnapshotDelta, m_pConsole, m_pStorage); + m_Http.Init(std::chrono::seconds{1}); + m_ServerBrowser.SetBaseInfo(&m_aNetClient[CONN_CONTACT], m_pGameClient->NetVersion()); - HttpInit(m_pStorage); - #if defined(CONF_AUTOUPDATE) - m_Updater.Init(); + m_Updater.Init(&m_Http); #endif m_pConfigManager->RegisterCallback(IFavorites::ConfigSaveCallback, m_pFavorites); @@ -4582,7 +4583,7 @@ void CClient::RequestDDNetInfo() m_pDDNetInfoTask = HttpGetFile(aUrl, Storage(), m_aDDNetInfoTmp, IStorage::TYPE_SAVE); m_pDDNetInfoTask->Timeout(CTimeout{10000, 0, 500, 10}); m_pDDNetInfoTask->IpResolve(IPRESOLVE::V4); - Engine()->AddJob(m_pDDNetInfoTask); + Http()->Run(m_pDDNetInfoTask); } int CClient::GetPredictionTime() diff --git a/src/engine/client/client.h b/src/engine/client/client.h index 48cfca823..e4f9c3a20 100644 --- a/src/engine/client/client.h +++ b/src/engine/client/client.h @@ -76,6 +76,7 @@ class CClient : public IClient, public CDemoPlayer::IListener IStorage *m_pStorage = nullptr; IEngineTextRender *m_pTextRender = nullptr; IUpdater *m_pUpdater = nullptr; + CHttp m_Http; CNetClient m_aNetClient[NUM_CONNS]; CDemoPlayer m_DemoPlayer; @@ -266,6 +267,7 @@ public: IStorage *Storage() { return m_pStorage; } IEngineTextRender *TextRender() { return m_pTextRender; } IUpdater *Updater() { return m_pUpdater; } + IHttp *Http() { return &m_Http; } CClient(); diff --git a/src/engine/client/serverbrowser.cpp b/src/engine/client/serverbrowser.cpp index 22939fe1f..7e538d690 100644 --- a/src/engine/client/serverbrowser.cpp +++ b/src/engine/client/serverbrowser.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include class CSortWrap @@ -95,6 +96,8 @@ void CServerBrowser::SetBaseInfo(class CNetClient *pClient, const char *pNetVers m_pFavorites = Kernel()->RequestInterface(); m_pFriends = Kernel()->RequestInterface(); m_pStorage = Kernel()->RequestInterface(); + m_pHttpClient = Kernel()->RequestInterface(); + dbg_msg("cserverbrowser", "%p", m_pHttpClient); m_pPingCache = CreateServerBrowserPingCache(m_pConsole, m_pStorage); RegisterCommands(); @@ -102,7 +105,7 @@ void CServerBrowser::SetBaseInfo(class CNetClient *pClient, const char *pNetVers void CServerBrowser::OnInit() { - m_pHttp = CreateServerBrowserHttp(m_pEngine, m_pConsole, m_pStorage, g_Config.m_BrCachedBestServerinfoUrl); + m_pHttp = CreateServerBrowserHttp(m_pEngine, m_pConsole, m_pStorage, m_pHttpClient, g_Config.m_BrCachedBestServerinfoUrl); } void CServerBrowser::RegisterCommands() diff --git a/src/engine/client/serverbrowser.h b/src/engine/client/serverbrowser.h index 1920b6c74..a8df4ec97 100644 --- a/src/engine/client/serverbrowser.h +++ b/src/engine/client/serverbrowser.h @@ -21,6 +21,7 @@ class IFriends; class IServerBrowserHttp; class IServerBrowserPingCache; class IStorage; +class IHttp; class CCommunityServer { @@ -143,6 +144,7 @@ private: IFriends *m_pFriends = nullptr; IFavorites *m_pFavorites = nullptr; IStorage *m_pStorage = nullptr; + IHttp *m_pHttpClient = nullptr; char m_aNetVersion[128]; bool m_RefreshingHttp = false; diff --git a/src/engine/client/serverbrowser_http.cpp b/src/engine/client/serverbrowser_http.cpp index df9221292..8d9734519 100644 --- a/src/engine/client/serverbrowser_http.cpp +++ b/src/engine/client/serverbrowser_http.cpp @@ -29,7 +29,7 @@ public: { MAX_URLS = 16, }; - CChooseMaster(IEngine *pEngine, VALIDATOR pfnValidator, const char **ppUrls, int NumUrls, int PreviousBestIndex); + CChooseMaster(IEngine *pEngine, IHttp *pHttp, VALIDATOR pfnValidator, const char **ppUrls, int NumUrls, int PreviousBestIndex); virtual ~CChooseMaster(); bool GetBestUrl(const char **pBestUrl) const; @@ -51,26 +51,29 @@ private: }; class CJob : public IJob { + CChooseMaster *m_pParent; CLock m_Lock; std::shared_ptr m_pData; - std::unique_ptr m_pHead PT_GUARDED_BY(m_Lock); - std::unique_ptr m_pGet PT_GUARDED_BY(m_Lock); + std::shared_ptr m_pHead PT_GUARDED_BY(m_Lock); + std::shared_ptr m_pGet PT_GUARDED_BY(m_Lock); void Run() override REQUIRES(!m_Lock); public: - CJob(std::shared_ptr pData) : - m_pData(std::move(pData)) {} + CJob(CChooseMaster *pParent, std::shared_ptr pData) : + m_pParent(pParent), m_pData(std::move(pData)) {} void Abort() REQUIRES(!m_Lock); }; IEngine *m_pEngine; + IHttp *m_pHttp; int m_PreviousBestIndex; std::shared_ptr m_pData; std::shared_ptr m_pJob; }; -CChooseMaster::CChooseMaster(IEngine *pEngine, VALIDATOR pfnValidator, const char **ppUrls, int NumUrls, int PreviousBestIndex) : +CChooseMaster::CChooseMaster(IEngine *pEngine, IHttp *pHttp, VALIDATOR pfnValidator, const char **ppUrls, int NumUrls, int PreviousBestIndex) : m_pEngine(pEngine), + m_pHttp(pHttp), m_PreviousBestIndex(PreviousBestIndex) { dbg_assert(NumUrls >= 0, "no master URLs"); @@ -128,7 +131,7 @@ void CChooseMaster::Reset() void CChooseMaster::Refresh() { if(m_pJob == nullptr || m_pJob->Status() == IJob::STATE_DONE) - m_pEngine->AddJob(m_pJob = std::make_shared(m_pData)); + m_pEngine->AddJob(m_pJob = std::make_shared(this, m_pData)); } void CChooseMaster::CJob::Abort() @@ -171,14 +174,16 @@ void CChooseMaster::CJob::Run() { aTimeMs[i] = -1; const char *pUrl = m_pData->m_aaUrls[aRandomized[i]]; - CHttpRequest *pHead = HttpHead(pUrl).release(); + std::shared_ptr pHead = std::move(HttpHead(pUrl)); pHead->Timeout(Timeout); pHead->LogProgress(HTTPLOG::FAILURE); { CLockScope ls(m_Lock); - m_pHead = std::unique_ptr(pHead); + m_pHead = pHead; } - IEngine::RunJobBlocking(pHead); + + m_pParent->m_pHttp->Run(pHead); + pHead->Wait(); if(pHead->State() == HTTP_ABORTED) { dbg_msg("serverbrowse_http", "master chooser aborted"); @@ -188,15 +193,19 @@ void CChooseMaster::CJob::Run() { continue; } + auto StartTime = time_get_nanoseconds(); - CHttpRequest *pGet = HttpGet(pUrl).release(); + std::shared_ptr pGet = std::move(HttpGet(pUrl)); pGet->Timeout(Timeout); pGet->LogProgress(HTTPLOG::FAILURE); { CLockScope ls(m_Lock); - m_pGet = std::unique_ptr(pGet); + m_pGet = pGet; } - IEngine::RunJobBlocking(pGet); + + m_pParent->m_pHttp->Run(pGet); + pGet->Wait(); + auto Time = std::chrono::duration_cast(time_get_nanoseconds() - StartTime); if(pHead->State() == HTTP_ABORTED) { @@ -212,6 +221,7 @@ void CChooseMaster::CJob::Run() { continue; } + bool ParseFailure = m_pData->m_pfnValidator(pJson); json_value_free(pJson); if(ParseFailure) @@ -221,6 +231,7 @@ void CChooseMaster::CJob::Run() dbg_msg("serverbrowse_http", "found master, url='%s' time=%dms", pUrl, (int)Time.count()); aTimeMs[i] = Time.count(); } + // Determine index of the minimum time. int BestIndex = -1; int BestTime = 0; @@ -241,6 +252,7 @@ void CChooseMaster::CJob::Run() dbg_msg("serverbrowse_http", "WARNING: no usable masters found"); return; } + dbg_msg("serverbrowse_http", "determined best master, url='%s' time=%dms", m_pData->m_aaUrls[BestIndex], BestTime); m_pData->m_BestIndex.store(BestIndex); } @@ -248,7 +260,7 @@ void CChooseMaster::CJob::Run() class CServerBrowserHttp : public IServerBrowserHttp { public: - CServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, const char **ppUrls, int NumUrls, int PreviousBestIndex); + CServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IHttp *pHttp, const char **ppUrls, int NumUrls, int PreviousBestIndex); ~CServerBrowserHttp() override; void Update() override; bool IsRefreshing() override { return m_State != STATE_DONE; } @@ -286,6 +298,7 @@ private: IEngine *m_pEngine; IConsole *m_pConsole; + IHttp *m_pHttp; int m_State = STATE_DONE; std::shared_ptr m_pGetServers; @@ -295,10 +308,11 @@ private: std::vector m_vLegacyServers; }; -CServerBrowserHttp::CServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, const char **ppUrls, int NumUrls, int PreviousBestIndex) : +CServerBrowserHttp::CServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IHttp *pHttp, const char **ppUrls, int NumUrls, int PreviousBestIndex) : m_pEngine(pEngine), m_pConsole(pConsole), - m_pChooseMaster(new CChooseMaster(pEngine, Validate, ppUrls, NumUrls, PreviousBestIndex)) + m_pHttp(pHttp), + m_pChooseMaster(new CChooseMaster(pEngine, pHttp, Validate, ppUrls, NumUrls, PreviousBestIndex)) { m_pChooseMaster->Refresh(); } @@ -328,7 +342,7 @@ void CServerBrowserHttp::Update() m_pGetServers = HttpGet(pBestUrl); // 10 seconds connection timeout, lower than 8KB/s for 10 seconds to fail. m_pGetServers->Timeout(CTimeout{10000, 0, 8000, 10}); - m_pEngine->AddJob(m_pGetServers); + m_pHttp->Run(m_pGetServers); m_State = STATE_REFRESHING; } else if(m_State == STATE_REFRESHING) @@ -469,7 +483,7 @@ static const char *DEFAULT_SERVERLIST_URLS[] = { "https://master4.ddnet.org/ddnet/15/servers.json", }; -IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IStorage *pStorage, const char *pPreviousBestUrl) +IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IStorage *pStorage, IHttp *pHttp, const char *pPreviousBestUrl) { char aaUrls[CChooseMaster::MAX_URLS][256]; const char *apUrls[CChooseMaster::MAX_URLS] = {0}; @@ -506,5 +520,5 @@ IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IConsole *pConsole break; } } - return new CServerBrowserHttp(pEngine, pConsole, ppUrls, NumUrls, PreviousBestIndex); + return new CServerBrowserHttp(pEngine, pConsole, pHttp, ppUrls, NumUrls, PreviousBestIndex); } diff --git a/src/engine/client/serverbrowser_http.h b/src/engine/client/serverbrowser_http.h index b6884f869..64c83d8a6 100644 --- a/src/engine/client/serverbrowser_http.h +++ b/src/engine/client/serverbrowser_http.h @@ -6,6 +6,7 @@ class CServerInfo; class IConsole; class IEngine; class IStorage; +class IHttp; class IServerBrowserHttp { @@ -25,5 +26,5 @@ public: virtual const NETADDR &LegacyServer(int Index) const = 0; }; -IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IStorage *pStorage, const char *pPreviousBestUrl); +IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IStorage *pStorage, IHttp *pHttp, const char *pPreviousBestUrl); #endif // ENGINE_CLIENT_SERVERBROWSER_HTTP_H diff --git a/src/engine/client/updater.cpp b/src/engine/client/updater.cpp index 3a6b6e389..1a3f17a24 100644 --- a/src/engine/client/updater.cpp +++ b/src/engine/client/updater.cpp @@ -25,7 +25,7 @@ class CUpdaterFetchTask : public CHttpRequest void OnProgress() override; protected: - int OnCompletion(int State) override; + void OnCompletion() override; public: CUpdaterFetchTask(CUpdater *pUpdater, const char *pFile, const char *pDestPath); @@ -61,10 +61,8 @@ void CUpdaterFetchTask::OnProgress() m_pUpdater->m_Percent = Progress(); } -int CUpdaterFetchTask::OnCompletion(int State) +void CUpdaterFetchTask::OnCompletion() { - State = CHttpRequest::OnCompletion(State); - const char *pFileName = 0; for(const char *pPath = Dest(); *pPath; pPath++) if(*pPath == '/') @@ -72,27 +70,26 @@ int CUpdaterFetchTask::OnCompletion(int State) pFileName = pFileName ? pFileName : Dest(); if(!str_comp(pFileName, "update.json")) { - if(State == HTTP_DONE) + if(State() == HTTP_DONE) m_pUpdater->SetCurrentState(IUpdater::GOT_MANIFEST); - else if(State == HTTP_ERROR) + else if(State() == HTTP_ERROR) m_pUpdater->SetCurrentState(IUpdater::FAIL); } else if(!str_comp(pFileName, m_pUpdater->m_aLastFile)) { - if(State == HTTP_DONE) + if(State() == HTTP_DONE) m_pUpdater->SetCurrentState(IUpdater::MOVE_FILES); - else if(State == HTTP_ERROR) + else if(State() == HTTP_ERROR) m_pUpdater->SetCurrentState(IUpdater::FAIL); } - - return State; } CUpdater::CUpdater() { - m_pClient = NULL; - m_pStorage = NULL; - m_pEngine = NULL; + m_pClient = nullptr; + m_pStorage = nullptr; + m_pEngine = nullptr; + m_pHttp = nullptr; m_State = CLEAN; m_Percent = 0; @@ -100,11 +97,12 @@ CUpdater::CUpdater() IStorage::FormatTmpPath(m_aServerExecTmp, sizeof(m_aServerExecTmp), SERVER_EXEC); } -void CUpdater::Init() +void CUpdater::Init(CHttp *pHttp) { m_pClient = Kernel()->RequestInterface(); m_pStorage = Kernel()->RequestInterface(); m_pEngine = Kernel()->RequestInterface(); + m_pHttp = pHttp; } void CUpdater::SetCurrentState(int NewState) @@ -133,7 +131,7 @@ int CUpdater::GetCurrentPercent() void CUpdater::FetchFile(const char *pFile, const char *pDestPath) { - m_pEngine->AddJob(std::make_shared(this, pFile, pDestPath)); + m_pHttp->Run(std::make_shared(this, pFile, pDestPath)); } bool CUpdater::MoveFile(const char *pFile) diff --git a/src/engine/client/updater.h b/src/engine/client/updater.h index 0b5c71c1d..c62148fec 100644 --- a/src/engine/client/updater.h +++ b/src/engine/client/updater.h @@ -41,6 +41,7 @@ class CUpdater : public IUpdater class IClient *m_pClient; class IStorage *m_pStorage; class IEngine *m_pEngine; + class CHttp *m_pHttp; CLock m_Lock; @@ -77,7 +78,7 @@ public: int GetCurrentPercent() override REQUIRES(!m_Lock); void InitiateUpdate() override; - void Init(); + void Init(CHttp *pHttp); void Update() override; }; diff --git a/src/engine/http.h b/src/engine/http.h new file mode 100644 index 000000000..29679bf35 --- /dev/null +++ b/src/engine/http.h @@ -0,0 +1,16 @@ +#ifndef HTTP_H +#define HTTP_H + +#include "kernel.h" + +class IHttpRequest {}; + +class IHttp : public IInterface +{ + MACRO_INTERFACE("http", 0) + +public: + virtual void Run(std::shared_ptr pRequest) = 0; +}; + +#endif diff --git a/src/engine/shared/http.cpp b/src/engine/shared/http.cpp index 259ff8d04..7b52c5059 100644 --- a/src/engine/shared/http.cpp +++ b/src/engine/shared/http.cpp @@ -8,6 +8,9 @@ #include #include +#include +#include + #if !defined(CONF_FAMILY_WINDOWS) #include #endif @@ -15,35 +18,6 @@ #define WIN32_LEAN_AND_MEAN #include -// TODO: Non-global pls? -static CURLSH *gs_pShare; -static CLock gs_aLocks[CURL_LOCK_DATA_LAST + 1]; -static bool gs_Initialized = false; - -static int GetLockIndex(int Data) -{ - if(!(0 <= Data && Data < CURL_LOCK_DATA_LAST)) - { - Data = CURL_LOCK_DATA_LAST; - } - return Data; -} - -static void CurlLock(CURL *pHandle, curl_lock_data Data, curl_lock_access Access, void *pUser) ACQUIRE(gs_aLocks[GetLockIndex(Data)]) -{ - (void)pHandle; - (void)Access; - (void)pUser; - gs_aLocks[GetLockIndex(Data)].lock(); -} - -static void CurlUnlock(CURL *pHandle, curl_lock_data Data, void *pUser) RELEASE(gs_aLocks[GetLockIndex(Data)]) -{ - (void)pHandle; - (void)pUser; - gs_aLocks[GetLockIndex(Data)].unlock(); -} - int CurlDebug(CURL *pHandle, curl_infotype Type, char *pData, size_t DataSize, void *pUser) { char TypeChar; @@ -71,39 +45,6 @@ int CurlDebug(CURL *pHandle, curl_infotype Type, char *pData, size_t DataSize, v return 0; } -bool HttpInit(IStorage *pStorage) -{ - if(curl_global_init(CURL_GLOBAL_DEFAULT)) - { - return true; - } - gs_pShare = curl_share_init(); - if(!gs_pShare) - { - return true; - } - // print curl version - { - curl_version_info_data *pVersion = curl_version_info(CURLVERSION_NOW); - dbg_msg("http", "libcurl version %s (compiled = " LIBCURL_VERSION ")", pVersion->version); - } - - curl_share_setopt(gs_pShare, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS); - curl_share_setopt(gs_pShare, CURLSHOPT_SHARE, CURL_LOCK_DATA_SSL_SESSION); - curl_share_setopt(gs_pShare, CURLSHOPT_LOCKFUNC, CurlLock); - curl_share_setopt(gs_pShare, CURLSHOPT_UNLOCKFUNC, CurlUnlock); - -#if !defined(CONF_FAMILY_WINDOWS) - // As a multithreaded application we have to tell curl to not install signal - // handlers and instead ignore SIGPIPE from OpenSSL ourselves. - signal(SIGPIPE, SIG_IGN); -#endif - - gs_Initialized = true; - - return false; -} - void EscapeUrl(char *pBuf, int Size, const char *pStr) { char *pEsc = curl_easy_escape(0, pStr, 0); @@ -143,24 +84,6 @@ CHttpRequest::~CHttpRequest() } } -void CHttpRequest::Run() -{ - dbg_assert(gs_Initialized, "must initialize HTTP before running HTTP requests"); - int FinalState; - if(!BeforeInit()) - { - FinalState = HTTP_ERROR; - } - else - { - CURL *pHandle = curl_easy_init(); - FinalState = RunImpl(pHandle); - curl_easy_cleanup(pHandle); - } - - m_State = OnCompletion(FinalState); -} - bool CHttpRequest::BeforeInit() { if(m_WriteToFile) @@ -181,12 +104,12 @@ bool CHttpRequest::BeforeInit() return true; } -int CHttpRequest::RunImpl(CURL *pUser) +bool CHttpRequest::ConfigureHandle(void *pUser) { CURL *pHandle = (CURL *)pUser; - if(!pHandle) + if(!BeforeInit()) { - return HTTP_ERROR; + return false; } if(g_Config.m_DbgCurl) @@ -199,8 +122,8 @@ int CHttpRequest::RunImpl(CURL *pUser) { Protocols |= CURLPROTO_HTTP; } - char aErr[CURL_ERROR_SIZE]; - curl_easy_setopt(pHandle, CURLOPT_ERRORBUFFER, aErr); + + curl_easy_setopt(pHandle, CURLOPT_ERRORBUFFER, m_aErr); curl_easy_setopt(pHandle, CURLOPT_CONNECTTIMEOUT_MS, m_Timeout.ConnectTimeoutMs); curl_easy_setopt(pHandle, CURLOPT_TIMEOUT_MS, m_Timeout.TimeoutMs); @@ -211,7 +134,6 @@ int CHttpRequest::RunImpl(CURL *pUser) curl_easy_setopt(pHandle, CURLOPT_MAXFILESIZE_LARGE, (curl_off_t)m_MaxResponseSize); } - curl_easy_setopt(pHandle, CURLOPT_SHARE, gs_pShare); // ‘CURLOPT_PROTOCOLS’ is deprecated: since 7.85.0. Use CURLOPT_PROTOCOLS_STR // Wait until all platforms have 7.85.0 #ifdef __GNUC__ @@ -285,22 +207,7 @@ int CHttpRequest::RunImpl(CURL *pUser) curl_easy_setopt(pHandle, CURLOPT_HTTPHEADER, m_pHeaders); - if(g_Config.m_DbgCurl || m_LogProgress >= HTTPLOG::ALL) - dbg_msg("http", "fetching %s", m_aUrl); - m_State = HTTP_RUNNING; - int Ret = curl_easy_perform(pHandle); - if(Ret != CURLE_OK) - { - if(g_Config.m_DbgCurl || m_LogProgress >= HTTPLOG::FAILURE) - dbg_msg("http", "%s failed. libcurl error (%d): %s", m_aUrl, (int)Ret, aErr); - return (Ret == CURLE_ABORTED_BY_CALLBACK) ? HTTP_ABORTED : HTTP_ERROR; - } - else - { - if(g_Config.m_DbgCurl || m_LogProgress >= HTTPLOG::ALL) - dbg_msg("http", "task done %s", m_aUrl); - return HTTP_DONE; - } + return true; } size_t CHttpRequest::OnData(char *pData, size_t DataSize) @@ -356,10 +263,29 @@ int CHttpRequest::ProgressCallback(void *pUser, double DlTotal, double DlCurr, d return pTask->m_Abort ? -1 : 0; } -int CHttpRequest::OnCompletion(int State) +void CHttpRequest::OnCompletionInternal(std::optional Result) { - if(m_Abort) - State = HTTP_ABORTED; + int State; + if(Result.has_value()) + { + CURLcode Code = static_cast(Result.value()); + if(Code != CURLE_OK) + { + if(g_Config.m_DbgCurl || m_LogProgress >= HTTPLOG::FAILURE) + dbg_msg("http", "%s failed. libcurl error (%u): %s", m_aUrl, Code, m_aErr); + State = (Code == CURLE_ABORTED_BY_CALLBACK) ? HTTP_ABORTED : HTTP_ERROR; + } + else + { + if(g_Config.m_DbgCurl || m_LogProgress >= HTTPLOG::ALL) + dbg_msg("http", "task done: %s", m_aUrl); + State = HTTP_DONE; + } + } + else { + dbg_msg("http", "%s failed. internal error: %s", m_aUrl, m_aErr); + State = HTTP_ERROR; + } if(State == HTTP_DONE && m_ExpectedSha256 != SHA256_ZEROED) { @@ -391,7 +317,9 @@ int CHttpRequest::OnCompletion(int State) fs_remove(m_aDestAbsolute); } } - return State; + + m_State = State; + OnCompletion(); } void CHttpRequest::WriteToFile(IStorage *pStorage, const char *pDest, int StorageType) @@ -413,6 +341,20 @@ void CHttpRequest::Header(const char *pNameColonValue) m_pHeaders = curl_slist_append((curl_slist *)m_pHeaders, pNameColonValue); } +void CHttpRequest::Wait() +{ + using namespace std::chrono_literals; + + // This is so uncommon that polling just might work + for(;;) { + int State = m_State.load(std::memory_order_seq_cst); + if(State != HTTP_QUEUED && State != HTTP_RUNNING) { + return; + } + std::this_thread::sleep_for(10ms); + } +} + void CHttpRequest::Result(unsigned char **ppResult, size_t *pResultLength) const { if(m_WriteToFile || State() != HTTP_DONE) @@ -436,3 +378,192 @@ json_value *CHttpRequest::ResultJson() const } return json_parse((char *)pResult, ResultLength); } + +bool CHttp::Init(std::chrono::milliseconds ShutdownDelay) +{ + m_ShutdownDelay = ShutdownDelay; + +#if !defined(CONF_FAMILY_WINDOWS) + // As a multithreaded application we have to tell curl to not install signal + // handlers and instead ignore SIGPIPE from OpenSSL ourselves. + signal(SIGPIPE, SIG_IGN); +#endif + m_pThread = thread_init(CHttp::ThreadMain, this, "http"); + + std::unique_lock Lock(m_Lock); + m_Cv.wait(Lock, [this](){ return m_State != UNINITIALIZED; }); + if(m_State != RUNNING) + { + return false; + } + + return true; +} + +void CHttp::ThreadMain(void *pUser) +{ + CHttp *pHttp = static_cast(pUser); + pHttp->RunLoop(); +} + +void CHttp::RunLoop() +{ + std::unique_lock Lock(m_Lock); + if(curl_global_init(CURL_GLOBAL_DEFAULT)) + { + dbg_msg("http", "curl_global_init failed"); + m_State = ERROR; + m_Cv.notify_all(); + return; + } + + m_pMultiH = curl_multi_init(); + if(!m_pMultiH) + { + dbg_msg("http", "curl_multi_init failed"); + m_State = ERROR; + m_Cv.notify_all(); + return; + } + + // print curl version + { + curl_version_info_data *pVersion = curl_version_info(CURLVERSION_NOW); + dbg_msg("http", "libcurl version %s (compiled = " LIBCURL_VERSION ")", pVersion->version); + } + + m_State = RUNNING; + m_Cv.notify_all(); + dbg_msg("http", "running"); + Lock.unlock(); + + while(m_State == RUNNING) { + static int NextTimeout = std::numeric_limits::max(); + int Events = 0; + CURLMcode mc = curl_multi_poll(m_pMultiH, NULL, 0, NextTimeout, &Events); + + // We may have been woken up for a shutdown + if(m_Shutdown) { + auto Now = std::chrono::steady_clock::now(); + if(!m_ShutdownTime.has_value()) { + m_ShutdownTime = Now + m_ShutdownDelay; + NextTimeout = m_ShutdownDelay.count(); + } + else if(m_ShutdownTime < Now || m_RunningRequests.empty()) { + break; + } + } + + if(mc != CURLM_OK) { + Lock.lock(); + dbg_msg("http", "Failed multi wait: %s", curl_multi_strerror(mc)); + m_State = ERROR; + break; + } + + mc = curl_multi_perform(m_pMultiH, &Events); + if(mc != CURLM_OK) { + Lock.lock(); + dbg_msg("http", "Failed multi perform: %s", curl_multi_strerror(mc)); + m_State = ERROR; + break; + } + + struct CURLMsg *m; + while((m = curl_multi_info_read(m_pMultiH, &Events))) { + if(m->msg == CURLMSG_DONE) { + auto RequestIt = m_RunningRequests.find(m->easy_handle); + dbg_assert(RequestIt != m_RunningRequests.end(), "Running handle not added to map"); + auto pRequest = std::move(RequestIt->second); + m_RunningRequests.erase(RequestIt); + + pRequest->OnCompletionInternal(m->data.result); + curl_multi_remove_handle(m_pMultiH, m->easy_handle); + curl_easy_cleanup(m->easy_handle); + } + } + + decltype(m_PendingRequests) NewRequests = {}; + Lock.lock(); + std::swap(m_PendingRequests, NewRequests); + Lock.unlock(); + + while(!NewRequests.empty()) { + auto &pRequest = NewRequests.front(); + dbg_msg("http", "task: %s %s", CHttpRequest::GetRequestType(pRequest->m_Type), pRequest->m_aUrl); + + CURL *pEH = curl_easy_init(); + if(!pEH) + goto error_init; + + if(!pRequest->ConfigureHandle(pEH)) + goto error_configure; + + mc = curl_multi_add_handle(m_pMultiH, pEH); + if(mc != CURLM_OK) + goto error_configure; + + m_RunningRequests.emplace(pEH, std::move(pRequest)); + NewRequests.pop_front(); + + continue; + + error_configure: + curl_easy_cleanup(pEH); + error_init: + dbg_msg("http", "failed to start new request"); + Lock.lock(); + m_State = ERROR; + break; + } + + // Only happens if m_State == ERROR, thus we already hold the lock + if(!NewRequests.empty()) { + m_PendingRequests.insert(m_PendingRequests.end(), std::make_move_iterator(NewRequests.begin()), std::make_move_iterator(NewRequests.end())); + break; + } + } + + if(!Lock.owns_lock()) + Lock.lock(); + + bool Cleanup = m_State != ERROR; + for(auto &pRequest : m_PendingRequests) { + str_copy(pRequest->m_aErr, "Shutting down"); + pRequest->OnCompletionInternal(std::nullopt); + } + + for(auto &ReqPair : m_RunningRequests) { + auto &[pHandle, pRequest] = ReqPair; + if(Cleanup) { + curl_multi_remove_handle(m_pMultiH, pHandle); + curl_easy_cleanup(pHandle); + } + + str_copy(pRequest->m_aErr, "Shutting down"); + pRequest->OnCompletionInternal(std::nullopt); + } + + if(Cleanup) { + curl_multi_cleanup(m_pMultiH); + curl_global_cleanup(); + } +} + +void CHttp::Run(std::shared_ptr pRequest) +{ + std::unique_lock Lock(m_Lock); + m_Cv.wait(Lock, [this](){ return m_State != UNINITIALIZED; }); + m_PendingRequests.emplace_back(std::move(std::static_pointer_cast(pRequest))); + curl_multi_wakeup(m_pMultiH); +} + +void CHttp::Shutdown() +{ + std::unique_lock Lock(m_Lock); + if(m_Shutdown || m_State != RUNNING) + return; + + m_Shutdown = true; + curl_multi_wakeup(m_pMultiH); +} diff --git a/src/engine/shared/http.h b/src/engine/shared/http.h index eb90fcaf1..04f24b057 100644 --- a/src/engine/shared/http.h +++ b/src/engine/shared/http.h @@ -7,6 +7,12 @@ #include #include +#include +#include +#include +#include + +#include typedef struct _json_value json_value; class IStorage; @@ -42,8 +48,10 @@ struct CTimeout long LowSpeedTime; }; -class CHttpRequest : public IJob +class CHttpRequest : public IHttpRequest { + friend class CHttp; + enum class REQUEST { GET = 0, @@ -51,6 +59,24 @@ class CHttpRequest : public IJob POST, POST_JSON, }; + + static constexpr const char *GetRequestType(REQUEST Type) + { + switch(Type) + { + case REQUEST::GET: + return "GET"; + case REQUEST::HEAD: + return "HEAD"; + case REQUEST::POST: + case REQUEST::POST_JSON: + return "POST"; + } + + // Unreachable, maybe assert instead? + return "UNKNOWN"; + } + char m_aUrl[256] = {0}; void *m_pHeaders = nullptr; @@ -83,13 +109,14 @@ class CHttpRequest : public IJob HTTPLOG m_LogProgress = HTTPLOG::ALL; IPRESOLVE m_IpResolve = IPRESOLVE::WHATEVER; + char m_aErr[256]; // 256 == CURL_ERROR_SIZE std::atomic m_State{HTTP_QUEUED}; std::atomic m_Abort{false}; - void Run() override; // Abort the request with an error if `BeforeInit()` returns false. bool BeforeInit(); - int RunImpl(void *pUser); + bool ConfigureHandle(void *pHandle); // void * == CURL * + void OnCompletionInternal(std::optional Result); // unsigned int == CURLcode // Abort the request if `OnData()` returns something other than // `DataSize`. @@ -99,8 +126,9 @@ class CHttpRequest : public IJob static size_t WriteCallback(char *pData, size_t Size, size_t Number, void *pUser); protected: - virtual void OnProgress() {} - virtual int OnCompletion(int State); + // These run on the curl thread now, DO NOT STALL THE THREAD + virtual void OnProgress() {}; + virtual void OnCompletion() {}; public: CHttpRequest(const char *pUrl); @@ -157,8 +185,11 @@ public: double Size() const { return m_Size.load(std::memory_order_relaxed); } int Progress() const { return m_Progress.load(std::memory_order_relaxed); } int State() const { return m_State; } + bool Done() const { int State = m_State; return State != HTTP_QUEUED && State != HTTP_DONE; } void Abort() { m_Abort = true; } + void Wait(); + void Result(unsigned char **ppResult, size_t *pResultLength) const; json_value *ResultJson() const; }; @@ -202,4 +233,41 @@ inline std::unique_ptr HttpPostJson(const char *pUrl, const char * bool HttpInit(IStorage *pStorage); void EscapeUrl(char *pBuf, int Size, const char *pStr); bool HttpHasIpresolveBug(); + +// In an ideal world this would be a kernel interface +class CHttp : public IHttp +{ + enum EState { + UNINITIALIZED, + RUNNING, + STOPPING, + ERROR, + }; + + void *m_pThread = nullptr; + + std::mutex m_Lock{}; + std::condition_variable m_Cv{}; + std::atomic m_State = UNINITIALIZED; + std::deque> m_PendingRequests{}; + std::unordered_map> m_RunningRequests{}; // void * == CURL * + std::chrono::milliseconds m_ShutdownDelay{}; + std::optional> m_ShutdownTime{}; + std::atomic m_Shutdown = false; + + // Only to be used with curl_multi_wakeup + void *m_pMultiH = nullptr; // void * == CURLM * + + static void ThreadMain(void *pUser); + void RunLoop(); + +public: + // Startup + bool Init(std::chrono::milliseconds ShutdownDelay); + + // User + virtual void Run(std::shared_ptr pRequest) override; + void Shutdown() override; +}; + #endif // ENGINE_SHARED_HTTP_H diff --git a/src/game/client/component.cpp b/src/game/client/component.cpp index 754aea48d..cabe1fae6 100644 --- a/src/game/client/component.cpp +++ b/src/game/client/component.cpp @@ -40,3 +40,5 @@ class IClient *CComponent::Client() const { return m_pClient->Client(); } + +class IHttp *CComponent::Http() const { return m_pClient->Http(); } diff --git a/src/game/client/component.h b/src/game/client/component.h index 8e754e7f3..edf2c6a70 100644 --- a/src/game/client/component.h +++ b/src/game/client/component.h @@ -128,6 +128,11 @@ protected: */ float LocalTime() const; + /** + * Get the http interface + */ + class IHttp *Http() const; + public: /** * The component virtual destructor. diff --git a/src/game/client/components/menus.h b/src/game/client/components/menus.h index b9dd4999f..b20250d24 100644 --- a/src/game/client/components/menus.h +++ b/src/game/client/components/menus.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -515,34 +516,35 @@ protected: char m_aPath[IO_MAX_PATH_LENGTH]; int m_StorageType; bool m_Success = false; - CImageInfo m_ImageInfo; SHA256_DIGEST m_Sha256; CAbstractCommunityIconJob(CMenus *pMenus, const char *pCommunityId, int StorageType); - virtual ~CAbstractCommunityIconJob(); + virtual ~CAbstractCommunityIconJob() {}; public: const char *CommunityId() const { return m_aCommunityId; } bool Success() const { return m_Success; } - CImageInfo &&ImageInfo() { return std::move(m_ImageInfo); } SHA256_DIGEST &&Sha256() { return std::move(m_Sha256); } }; + class CCommunityIconLoadJob : public IJob, public CAbstractCommunityIconJob { + CImageInfo m_ImageInfo; protected: void Run() override; public: CCommunityIconLoadJob(CMenus *pMenus, const char *pCommunityId, int StorageType); + + CImageInfo &&ImageInfo() { return std::move(m_ImageInfo); } }; + class CCommunityIconDownloadJob : public CHttpRequest, public CAbstractCommunityIconJob { - protected: - int OnCompletion(int State) override; - public: CCommunityIconDownloadJob(CMenus *pMenus, const char *pCommunityId, const char *pUrl, const SHA256_DIGEST &Sha256); }; + struct SCommunityIcon { char m_aCommunityId[CServerInfo::MAX_COMMUNITY_ID_LENGTH]; diff --git a/src/game/client/components/menus_browser.cpp b/src/game/client/components/menus_browser.cpp index 34d0250a1..1a1df4321 100644 --- a/src/game/client/components/menus_browser.cpp +++ b/src/game/client/components/menus_browser.cpp @@ -1802,25 +1802,6 @@ CMenus::CAbstractCommunityIconJob::CAbstractCommunityIconJob(CMenus *pMenus, con str_format(m_aPath, sizeof(m_aPath), "communityicons/%s.png", pCommunityId); } -CMenus::CAbstractCommunityIconJob::~CAbstractCommunityIconJob() -{ - free(m_ImageInfo.m_pData); - m_ImageInfo.m_pData = nullptr; -} - -int CMenus::CCommunityIconDownloadJob::OnCompletion(int State) -{ - State = CHttpRequest::OnCompletion(State); - if(State == HTTP_DONE) - { - if(m_pMenus->LoadCommunityIconFile(Dest(), IStorage::TYPE_SAVE, m_ImageInfo, m_Sha256)) - m_Success = true; - else - State = HTTP_ERROR; - } - return State; -} - CMenus::CCommunityIconDownloadJob::CCommunityIconDownloadJob(CMenus *pMenus, const char *pCommunityId, const char *pUrl, const SHA256_DIGEST &Sha256) : CHttpRequest(pUrl), CAbstractCommunityIconJob(pMenus, pCommunityId, IStorage::TYPE_SAVE) @@ -1964,10 +1945,14 @@ void CMenus::UpdateCommunityIcons() if(!m_CommunityIconDownloadJobs.empty()) { std::shared_ptr pJob = m_CommunityIconDownloadJobs.front(); - if(pJob->Status() == IJob::STATE_DONE) + if(pJob->Done()) { - if(pJob->Success()) - LoadCommunityIconFinish(pJob->CommunityId(), pJob->ImageInfo(), pJob->Sha256()); + if(pJob->State() == HTTP_DONE) + { + std::shared_ptr pLoadJob = std::make_shared(this, pJob->CommunityId(), IStorage::TYPE_SAVE); + Engine()->AddJob(pLoadJob); + m_CommunityIconLoadJobs.emplace_back(std::move(pLoadJob)); + } m_CommunityIconDownloadJobs.pop_front(); } } @@ -2007,7 +1992,7 @@ void CMenus::UpdateCommunityIcons() if(pExistingDownload == m_CommunityIconDownloadJobs.end() && (ExistingIcon == m_vCommunityIcons.end() || ExistingIcon->m_Sha256 != Community.IconSha256())) { std::shared_ptr pJob = std::make_shared(this, Community.Id(), Community.IconUrl(), Community.IconSha256()); - Engine()->AddJob(pJob); + Http()->Run(pJob); m_CommunityIconDownloadJobs.push_back(pJob); } } diff --git a/src/game/client/components/skins.cpp b/src/game/client/components/skins.cpp index a400daf76..cbde46556 100644 --- a/src/game/client/components/skins.cpp +++ b/src/game/client/components/skins.cpp @@ -21,15 +21,13 @@ bool CSkins::IsVanillaSkin(const char *pName) return std::any_of(std::begin(VANILLA_SKINS), std::end(VANILLA_SKINS), [pName](const char *pVanillaSkin) { return str_comp(pName, pVanillaSkin) == 0; }); } -int CSkins::CGetPngFile::OnCompletion(int State) +void CSkins::CGetPngFile::OnCompletion() { - State = CHttpRequest::OnCompletion(State); - - if(State != HTTP_ERROR && State != HTTP_ABORTED && !m_pSkins->LoadSkinPNG(m_Info, Dest(), Dest(), IStorage::TYPE_SAVE)) + // Maybe this should start another thread to load the png in instead of stalling the curl thread + if(State() != HTTP_ERROR && State() != HTTP_ABORTED) { - State = HTTP_ERROR; + m_pSkins->LoadSkinPNG(m_Info, Dest(), Dest(), IStorage::TYPE_SAVE); } - return State; } CSkins::CGetPngFile::CGetPngFile(CSkins *pSkins, const char *pUrl, IStorage *pStorage, const char *pDest) : @@ -436,12 +434,16 @@ const CSkin *CSkins::FindImpl(const char *pName) char aEscapedName[256]; EscapeUrl(aEscapedName, sizeof(aEscapedName), pName); str_format(aUrl, sizeof(aUrl), "%s%s.png", g_Config.m_ClDownloadCommunitySkins != 0 ? g_Config.m_ClSkinCommunityDownloadUrl : g_Config.m_ClSkinDownloadUrl, aEscapedName); + char aBuf[IO_MAX_PATH_LENGTH]; str_format(Skin.m_aPath, sizeof(Skin.m_aPath), "downloadedskins/%s", IStorage::FormatTmpPath(aBuf, sizeof(aBuf), pName)); + Skin.m_pTask = std::make_shared(this, aUrl, Storage(), Skin.m_aPath); - m_pClient->Engine()->AddJob(Skin.m_pTask); + Http()->Run(Skin.m_pTask); + auto &&pDownloadSkin = std::make_unique(std::move(Skin)); m_DownloadSkins.insert({pDownloadSkin->GetName(), std::move(pDownloadSkin)}); ++m_DownloadingSkins; + return nullptr; } diff --git a/src/game/client/components/skins.h b/src/game/client/components/skins.h index 677350fca..6c42fd632 100644 --- a/src/game/client/components/skins.h +++ b/src/game/client/components/skins.h @@ -20,7 +20,7 @@ public: CSkins *m_pSkins; protected: - virtual int OnCompletion(int State) override; + virtual void OnCompletion() override; public: CGetPngFile(CSkins *pSkins, const char *pUrl, IStorage *pStorage, const char *pDest); diff --git a/src/game/client/gameclient.cpp b/src/game/client/gameclient.cpp index 29063df98..5807f1ea8 100644 --- a/src/game/client/gameclient.cpp +++ b/src/game/client/gameclient.cpp @@ -100,6 +100,7 @@ void CGameClient::OnConsoleInit() #if defined(CONF_AUTOUPDATE) m_pUpdater = Kernel()->RequestInterface(); #endif + m_pHttp = Kernel()->RequestInterface(); m_Menus.SetMenuBackground(&m_MenuBackground); diff --git a/src/game/client/gameclient.h b/src/game/client/gameclient.h index 1e76b4cf4..52c5551a1 100644 --- a/src/game/client/gameclient.h +++ b/src/game/client/gameclient.h @@ -177,6 +177,7 @@ private: #if defined(CONF_AUTOUPDATE) class IUpdater *m_pUpdater; #endif + class IHttp *m_pHttp; CLayers m_Layers; CCollision m_Collision; @@ -251,6 +252,7 @@ public: return m_pUpdater; } #endif + class IHttp *Http() { return m_pHttp; } int NetobjNumCorrections() { From d847b0f60c53ec7a6d9a924e252aa0c6938e912f Mon Sep 17 00:00:00 2001 From: Learath Date: Mon, 18 Dec 2023 22:42:25 +0100 Subject: [PATCH 2/8] Fix double request, fix merge error --- src/engine/client/client.cpp | 3 +++ src/engine/http.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/engine/client/client.cpp b/src/engine/client/client.cpp index 3eb18ed4d..cc05ba571 100644 --- a/src/engine/client/client.cpp +++ b/src/engine/client/client.cpp @@ -4568,6 +4568,9 @@ bool CClient::RaceRecord_IsRecording() void CClient::RequestDDNetInfo() { + if(m_pDDNetInfoTask && !m_pDDNetInfoTask->Done()) + return; + char aUrl[256]; str_copy(aUrl, DDNET_INFO_URL); diff --git a/src/engine/http.h b/src/engine/http.h index 29679bf35..302f7d6d8 100644 --- a/src/engine/http.h +++ b/src/engine/http.h @@ -7,7 +7,7 @@ class IHttpRequest {}; class IHttp : public IInterface { - MACRO_INTERFACE("http", 0) + MACRO_INTERFACE("http") public: virtual void Run(std::shared_ptr pRequest) = 0; From 1f224344ec0c774d4f375c4cb98b27296594b5e4 Mon Sep 17 00:00:00 2001 From: Learath Date: Mon, 18 Dec 2023 22:53:23 +0100 Subject: [PATCH 3/8] Format and bump fake curl --- ddnet-libs | 2 +- src/engine/http.h | 8 ++-- src/engine/shared/http.cpp | 61 +++++++++++++++++++----------- src/engine/shared/http.h | 15 +++++--- src/game/client/components/menus.h | 3 +- src/game/client/gameclient.h | 5 ++- 6 files changed, 61 insertions(+), 33 deletions(-) diff --git a/ddnet-libs b/ddnet-libs index 59d64dbb3..4d796ea11 160000 --- a/ddnet-libs +++ b/ddnet-libs @@ -1 +1 @@ -Subproject commit 59d64dbb36ade02607ad20a7f3a45605ab1de80d +Subproject commit 4d796ea119b52c8901286e14ab96faf7353a5d59 diff --git a/src/engine/http.h b/src/engine/http.h index 302f7d6d8..aca3bf663 100644 --- a/src/engine/http.h +++ b/src/engine/http.h @@ -3,14 +3,16 @@ #include "kernel.h" -class IHttpRequest {}; +class IHttpRequest +{ +}; class IHttp : public IInterface { - MACRO_INTERFACE("http") + MACRO_INTERFACE("http") public: - virtual void Run(std::shared_ptr pRequest) = 0; + virtual void Run(std::shared_ptr pRequest) = 0; }; #endif diff --git a/src/engine/shared/http.cpp b/src/engine/shared/http.cpp index 7b52c5059..d29af6b15 100644 --- a/src/engine/shared/http.cpp +++ b/src/engine/shared/http.cpp @@ -8,8 +8,8 @@ #include #include -#include #include +#include #if !defined(CONF_FAMILY_WINDOWS) #include @@ -272,8 +272,8 @@ void CHttpRequest::OnCompletionInternal(std::optional Result) if(Code != CURLE_OK) { if(g_Config.m_DbgCurl || m_LogProgress >= HTTPLOG::FAILURE) - dbg_msg("http", "%s failed. libcurl error (%u): %s", m_aUrl, Code, m_aErr); - State = (Code == CURLE_ABORTED_BY_CALLBACK) ? HTTP_ABORTED : HTTP_ERROR; + dbg_msg("http", "%s failed. libcurl error (%u): %s", m_aUrl, Code, m_aErr); + State = (Code == CURLE_ABORTED_BY_CALLBACK) ? HTTP_ABORTED : HTTP_ERROR; } else { @@ -282,7 +282,8 @@ void CHttpRequest::OnCompletionInternal(std::optional Result) State = HTTP_DONE; } } - else { + else + { dbg_msg("http", "%s failed. internal error: %s", m_aUrl, m_aErr); State = HTTP_ERROR; } @@ -346,9 +347,11 @@ void CHttpRequest::Wait() using namespace std::chrono_literals; // This is so uncommon that polling just might work - for(;;) { + for(;;) + { int State = m_State.load(std::memory_order_seq_cst); - if(State != HTTP_QUEUED && State != HTTP_RUNNING) { + if(State != HTTP_QUEUED && State != HTTP_RUNNING) + { return; } std::this_thread::sleep_for(10ms); @@ -391,7 +394,7 @@ bool CHttp::Init(std::chrono::milliseconds ShutdownDelay) m_pThread = thread_init(CHttp::ThreadMain, this, "http"); std::unique_lock Lock(m_Lock); - m_Cv.wait(Lock, [this](){ return m_State != UNINITIALIZED; }); + m_Cv.wait(Lock, [this]() { return m_State != UNINITIALIZED; }); if(m_State != RUNNING) { return false; @@ -437,24 +440,29 @@ void CHttp::RunLoop() dbg_msg("http", "running"); Lock.unlock(); - while(m_State == RUNNING) { + while(m_State == RUNNING) + { static int NextTimeout = std::numeric_limits::max(); int Events = 0; CURLMcode mc = curl_multi_poll(m_pMultiH, NULL, 0, NextTimeout, &Events); // We may have been woken up for a shutdown - if(m_Shutdown) { + if(m_Shutdown) + { auto Now = std::chrono::steady_clock::now(); - if(!m_ShutdownTime.has_value()) { + if(!m_ShutdownTime.has_value()) + { m_ShutdownTime = Now + m_ShutdownDelay; NextTimeout = m_ShutdownDelay.count(); } - else if(m_ShutdownTime < Now || m_RunningRequests.empty()) { + else if(m_ShutdownTime < Now || m_RunningRequests.empty()) + { break; } } - if(mc != CURLM_OK) { + if(mc != CURLM_OK) + { Lock.lock(); dbg_msg("http", "Failed multi wait: %s", curl_multi_strerror(mc)); m_State = ERROR; @@ -462,7 +470,8 @@ void CHttp::RunLoop() } mc = curl_multi_perform(m_pMultiH, &Events); - if(mc != CURLM_OK) { + if(mc != CURLM_OK) + { Lock.lock(); dbg_msg("http", "Failed multi perform: %s", curl_multi_strerror(mc)); m_State = ERROR; @@ -470,8 +479,10 @@ void CHttp::RunLoop() } struct CURLMsg *m; - while((m = curl_multi_info_read(m_pMultiH, &Events))) { - if(m->msg == CURLMSG_DONE) { + while((m = curl_multi_info_read(m_pMultiH, &Events))) + { + if(m->msg == CURLMSG_DONE) + { auto RequestIt = m_RunningRequests.find(m->easy_handle); dbg_assert(RequestIt != m_RunningRequests.end(), "Running handle not added to map"); auto pRequest = std::move(RequestIt->second); @@ -488,7 +499,8 @@ void CHttp::RunLoop() std::swap(m_PendingRequests, NewRequests); Lock.unlock(); - while(!NewRequests.empty()) { + while(!NewRequests.empty()) + { auto &pRequest = NewRequests.front(); dbg_msg("http", "task: %s %s", CHttpRequest::GetRequestType(pRequest->m_Type), pRequest->m_aUrl); @@ -518,7 +530,8 @@ void CHttp::RunLoop() } // Only happens if m_State == ERROR, thus we already hold the lock - if(!NewRequests.empty()) { + if(!NewRequests.empty()) + { m_PendingRequests.insert(m_PendingRequests.end(), std::make_move_iterator(NewRequests.begin()), std::make_move_iterator(NewRequests.end())); break; } @@ -528,14 +541,17 @@ void CHttp::RunLoop() Lock.lock(); bool Cleanup = m_State != ERROR; - for(auto &pRequest : m_PendingRequests) { + for(auto &pRequest : m_PendingRequests) + { str_copy(pRequest->m_aErr, "Shutting down"); pRequest->OnCompletionInternal(std::nullopt); } - for(auto &ReqPair : m_RunningRequests) { + for(auto &ReqPair : m_RunningRequests) + { auto &[pHandle, pRequest] = ReqPair; - if(Cleanup) { + if(Cleanup) + { curl_multi_remove_handle(m_pMultiH, pHandle); curl_easy_cleanup(pHandle); } @@ -544,7 +560,8 @@ void CHttp::RunLoop() pRequest->OnCompletionInternal(std::nullopt); } - if(Cleanup) { + if(Cleanup) + { curl_multi_cleanup(m_pMultiH); curl_global_cleanup(); } @@ -553,7 +570,7 @@ void CHttp::RunLoop() void CHttp::Run(std::shared_ptr pRequest) { std::unique_lock Lock(m_Lock); - m_Cv.wait(Lock, [this](){ return m_State != UNINITIALIZED; }); + m_Cv.wait(Lock, [this]() { return m_State != UNINITIALIZED; }); m_PendingRequests.emplace_back(std::move(std::static_pointer_cast(pRequest))); curl_multi_wakeup(m_pMultiH); } diff --git a/src/engine/shared/http.h b/src/engine/shared/http.h index 04f24b057..451dbc3e1 100644 --- a/src/engine/shared/http.h +++ b/src/engine/shared/http.h @@ -7,9 +7,9 @@ #include #include -#include #include #include +#include #include #include @@ -127,8 +127,8 @@ class CHttpRequest : public IHttpRequest protected: // These run on the curl thread now, DO NOT STALL THE THREAD - virtual void OnProgress() {}; - virtual void OnCompletion() {}; + virtual void OnProgress(){}; + virtual void OnCompletion(){}; public: CHttpRequest(const char *pUrl); @@ -185,7 +185,11 @@ public: double Size() const { return m_Size.load(std::memory_order_relaxed); } int Progress() const { return m_Progress.load(std::memory_order_relaxed); } int State() const { return m_State; } - bool Done() const { int State = m_State; return State != HTTP_QUEUED && State != HTTP_DONE; } + bool Done() const + { + int State = m_State; + return State != HTTP_QUEUED && State != HTTP_DONE; + } void Abort() { m_Abort = true; } void Wait(); @@ -237,7 +241,8 @@ bool HttpHasIpresolveBug(); // In an ideal world this would be a kernel interface class CHttp : public IHttp { - enum EState { + enum EState + { UNINITIALIZED, RUNNING, STOPPING, diff --git a/src/game/client/components/menus.h b/src/game/client/components/menus.h index b20250d24..6f45fd405 100644 --- a/src/game/client/components/menus.h +++ b/src/game/client/components/menus.h @@ -519,7 +519,7 @@ protected: SHA256_DIGEST m_Sha256; CAbstractCommunityIconJob(CMenus *pMenus, const char *pCommunityId, int StorageType); - virtual ~CAbstractCommunityIconJob() {}; + virtual ~CAbstractCommunityIconJob(){}; public: const char *CommunityId() const { return m_aCommunityId; } @@ -530,6 +530,7 @@ protected: class CCommunityIconLoadJob : public IJob, public CAbstractCommunityIconJob { CImageInfo m_ImageInfo; + protected: void Run() override; diff --git a/src/game/client/gameclient.h b/src/game/client/gameclient.h index 52c5551a1..ac1129edf 100644 --- a/src/game/client/gameclient.h +++ b/src/game/client/gameclient.h @@ -252,7 +252,10 @@ public: return m_pUpdater; } #endif - class IHttp *Http() { return m_pHttp; } + class IHttp *Http() + { + return m_pHttp; + } int NetobjNumCorrections() { From 9b3ebf3f1b6f1e9d4f021fbcf4ba53972cedec35 Mon Sep 17 00:00:00 2001 From: Learath2 Date: Sun, 7 Jan 2024 17:48:48 +0300 Subject: [PATCH 4/8] Remove debug statement, fix CI --- src/engine/client/serverbrowser.cpp | 1 - src/engine/http.h | 4 +- src/engine/shared/http.cpp | 60 ++++++++++++++--------------- src/engine/shared/http.h | 2 +- 4 files changed, 33 insertions(+), 34 deletions(-) diff --git a/src/engine/client/serverbrowser.cpp b/src/engine/client/serverbrowser.cpp index 7e538d690..a59af7e08 100644 --- a/src/engine/client/serverbrowser.cpp +++ b/src/engine/client/serverbrowser.cpp @@ -97,7 +97,6 @@ void CServerBrowser::SetBaseInfo(class CNetClient *pClient, const char *pNetVers m_pFriends = Kernel()->RequestInterface(); m_pStorage = Kernel()->RequestInterface(); m_pHttpClient = Kernel()->RequestInterface(); - dbg_msg("cserverbrowser", "%p", m_pHttpClient); m_pPingCache = CreateServerBrowserPingCache(m_pConsole, m_pStorage); RegisterCommands(); diff --git a/src/engine/http.h b/src/engine/http.h index aca3bf663..18738e2d5 100644 --- a/src/engine/http.h +++ b/src/engine/http.h @@ -1,5 +1,5 @@ -#ifndef HTTP_H -#define HTTP_H +#ifndef ENGINE_HTTP_H +#define ENGINE_HTTP_H #include "kernel.h" diff --git a/src/engine/shared/http.cpp b/src/engine/shared/http.cpp index d29af6b15..c866a7590 100644 --- a/src/engine/shared/http.cpp +++ b/src/engine/shared/http.cpp @@ -104,9 +104,9 @@ bool CHttpRequest::BeforeInit() return true; } -bool CHttpRequest::ConfigureHandle(void *pUser) +bool CHttpRequest::ConfigureHandle(void *pHandle) { - CURL *pHandle = (CURL *)pUser; + CURL *pH = (CURL *)pHandle; if(!BeforeInit()) { return false; @@ -114,8 +114,8 @@ bool CHttpRequest::ConfigureHandle(void *pUser) if(g_Config.m_DbgCurl) { - curl_easy_setopt(pHandle, CURLOPT_VERBOSE, 1L); - curl_easy_setopt(pHandle, CURLOPT_DEBUGFUNCTION, CurlDebug); + curl_easy_setopt(pH, CURLOPT_VERBOSE, 1L); + curl_easy_setopt(pH, CURLOPT_DEBUGFUNCTION, CurlDebug); } long Protocols = CURLPROTO_HTTPS; if(g_Config.m_HttpAllowInsecure) @@ -123,15 +123,15 @@ bool CHttpRequest::ConfigureHandle(void *pUser) Protocols |= CURLPROTO_HTTP; } - curl_easy_setopt(pHandle, CURLOPT_ERRORBUFFER, m_aErr); + curl_easy_setopt(pH, CURLOPT_ERRORBUFFER, m_aErr); - curl_easy_setopt(pHandle, CURLOPT_CONNECTTIMEOUT_MS, m_Timeout.ConnectTimeoutMs); - curl_easy_setopt(pHandle, CURLOPT_TIMEOUT_MS, m_Timeout.TimeoutMs); - curl_easy_setopt(pHandle, CURLOPT_LOW_SPEED_LIMIT, m_Timeout.LowSpeedLimit); - curl_easy_setopt(pHandle, CURLOPT_LOW_SPEED_TIME, m_Timeout.LowSpeedTime); + curl_easy_setopt(pH, CURLOPT_CONNECTTIMEOUT_MS, m_Timeout.ConnectTimeoutMs); + curl_easy_setopt(pH, CURLOPT_TIMEOUT_MS, m_Timeout.TimeoutMs); + curl_easy_setopt(pH, CURLOPT_LOW_SPEED_LIMIT, m_Timeout.LowSpeedLimit); + curl_easy_setopt(pH, CURLOPT_LOW_SPEED_TIME, m_Timeout.LowSpeedTime); if(m_MaxResponseSize >= 0) { - curl_easy_setopt(pHandle, CURLOPT_MAXFILESIZE_LARGE, (curl_off_t)m_MaxResponseSize); + curl_easy_setopt(pH, CURLOPT_MAXFILESIZE_LARGE, (curl_off_t)m_MaxResponseSize); } // ‘CURLOPT_PROTOCOLS’ is deprecated: since 7.85.0. Use CURLOPT_PROTOCOLS_STR @@ -140,43 +140,43 @@ bool CHttpRequest::ConfigureHandle(void *pUser) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" #endif - curl_easy_setopt(pHandle, CURLOPT_PROTOCOLS, Protocols); + curl_easy_setopt(pH, CURLOPT_PROTOCOLS, Protocols); #ifdef __GNUC__ #pragma GCC diagnostic pop #endif - curl_easy_setopt(pHandle, CURLOPT_FOLLOWLOCATION, 1L); - curl_easy_setopt(pHandle, CURLOPT_MAXREDIRS, 4L); - curl_easy_setopt(pHandle, CURLOPT_FAILONERROR, 1L); - curl_easy_setopt(pHandle, CURLOPT_URL, m_aUrl); - curl_easy_setopt(pHandle, CURLOPT_NOSIGNAL, 1L); - curl_easy_setopt(pHandle, CURLOPT_USERAGENT, GAME_NAME " " GAME_RELEASE_VERSION " (" CONF_PLATFORM_STRING "; " CONF_ARCH_STRING ")"); - curl_easy_setopt(pHandle, CURLOPT_ACCEPT_ENCODING, ""); // Use any compression algorithm supported by libcurl. + curl_easy_setopt(pH, CURLOPT_FOLLOWLOCATION, 1L); + curl_easy_setopt(pH, CURLOPT_MAXREDIRS, 4L); + curl_easy_setopt(pH, CURLOPT_FAILONERROR, 1L); + curl_easy_setopt(pH, CURLOPT_URL, m_aUrl); + curl_easy_setopt(pH, CURLOPT_NOSIGNAL, 1L); + curl_easy_setopt(pH, CURLOPT_USERAGENT, GAME_NAME " " GAME_RELEASE_VERSION " (" CONF_PLATFORM_STRING "; " CONF_ARCH_STRING ")"); + curl_easy_setopt(pH, CURLOPT_ACCEPT_ENCODING, ""); // Use any compression algorithm supported by libcurl. - curl_easy_setopt(pHandle, CURLOPT_WRITEDATA, this); - curl_easy_setopt(pHandle, CURLOPT_WRITEFUNCTION, WriteCallback); - curl_easy_setopt(pHandle, CURLOPT_NOPROGRESS, 0L); - curl_easy_setopt(pHandle, CURLOPT_PROGRESSDATA, this); + curl_easy_setopt(pH, CURLOPT_WRITEDATA, this); + curl_easy_setopt(pH, CURLOPT_WRITEFUNCTION, WriteCallback); + curl_easy_setopt(pH, CURLOPT_NOPROGRESS, 0L); + curl_easy_setopt(pH, CURLOPT_PROGRESSDATA, this); // ‘CURLOPT_PROGRESSFUNCTION’ is deprecated: since 7.32.0. Use CURLOPT_XFERINFOFUNCTION // See problems with curl_off_t type in header file in https://github.com/ddnet/ddnet/pull/6185/ #ifdef __GNUC__ #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" #endif - curl_easy_setopt(pHandle, CURLOPT_PROGRESSFUNCTION, ProgressCallback); + curl_easy_setopt(pH, CURLOPT_PROGRESSFUNCTION, ProgressCallback); #ifdef __GNUC__ #pragma GCC diagnostic pop #endif - curl_easy_setopt(pHandle, CURLOPT_IPRESOLVE, m_IpResolve == IPRESOLVE::V4 ? CURL_IPRESOLVE_V4 : m_IpResolve == IPRESOLVE::V6 ? CURL_IPRESOLVE_V6 : CURL_IPRESOLVE_WHATEVER); + curl_easy_setopt(pH, CURLOPT_IPRESOLVE, m_IpResolve == IPRESOLVE::V4 ? CURL_IPRESOLVE_V4 : m_IpResolve == IPRESOLVE::V6 ? CURL_IPRESOLVE_V6 : CURL_IPRESOLVE_WHATEVER); if(g_Config.m_Bindaddr[0] != '\0') { - curl_easy_setopt(pHandle, CURLOPT_INTERFACE, g_Config.m_Bindaddr); + curl_easy_setopt(pH, CURLOPT_INTERFACE, g_Config.m_Bindaddr); } if(curl_version_info(CURLVERSION_NOW)->version_num < 0x074400) { // Causes crashes, see https://github.com/ddnet/ddnet/issues/4342. // No longer a problem in curl 7.68 and above, and 0x44 = 68. - curl_easy_setopt(pHandle, CURLOPT_FORBID_REUSE, 1L); + curl_easy_setopt(pH, CURLOPT_FORBID_REUSE, 1L); } #ifdef CONF_PLATFORM_ANDROID @@ -188,7 +188,7 @@ bool CHttpRequest::ConfigureHandle(void *pUser) case REQUEST::GET: break; case REQUEST::HEAD: - curl_easy_setopt(pHandle, CURLOPT_NOBODY, 1L); + curl_easy_setopt(pH, CURLOPT_NOBODY, 1L); break; case REQUEST::POST: case REQUEST::POST_JSON: @@ -200,12 +200,12 @@ bool CHttpRequest::ConfigureHandle(void *pUser) { Header("Content-Type:"); } - curl_easy_setopt(pHandle, CURLOPT_POSTFIELDS, m_pBody); - curl_easy_setopt(pHandle, CURLOPT_POSTFIELDSIZE, m_BodyLength); + curl_easy_setopt(pH, CURLOPT_POSTFIELDS, m_pBody); + curl_easy_setopt(pH, CURLOPT_POSTFIELDSIZE, m_BodyLength); break; } - curl_easy_setopt(pHandle, CURLOPT_HTTPHEADER, m_pHeaders); + curl_easy_setopt(pH, CURLOPT_HTTPHEADER, m_pHeaders); return true; } diff --git a/src/engine/shared/http.h b/src/engine/shared/http.h index 451dbc3e1..b87d53d22 100644 --- a/src/engine/shared/http.h +++ b/src/engine/shared/http.h @@ -188,7 +188,7 @@ public: bool Done() const { int State = m_State; - return State != HTTP_QUEUED && State != HTTP_DONE; + return State != HTTP_QUEUED && State != HTTP_RUNNING; } void Abort() { m_Abort = true; } From 32e968335b08cb959fd3d4c494135ce93b7ad046 Mon Sep 17 00:00:00 2001 From: Learath2 Date: Mon, 8 Jan 2024 18:01:37 +0300 Subject: [PATCH 5/8] Fix lots of weirdness in updater --- src/engine/client/updater.cpp | 82 ++++++++++++++++++++--------------- src/engine/client/updater.h | 13 ++++-- 2 files changed, 56 insertions(+), 39 deletions(-) diff --git a/src/engine/client/updater.cpp b/src/engine/client/updater.cpp index 1a3f17a24..eb238a709 100644 --- a/src/engine/client/updater.cpp +++ b/src/engine/client/updater.cpp @@ -13,7 +13,6 @@ #include // system -using std::map; using std::string; class CUpdaterFetchTask : public CHttpRequest @@ -57,7 +56,6 @@ CUpdaterFetchTask::CUpdaterFetchTask(CUpdater *pUpdater, const char *pFile, cons void CUpdaterFetchTask::OnProgress() { CLockScope ls(m_pUpdater->m_Lock); - str_copy(m_pUpdater->m_aStatus, Dest()); m_pUpdater->m_Percent = Progress(); } @@ -75,13 +73,6 @@ void CUpdaterFetchTask::OnCompletion() else if(State() == HTTP_ERROR) m_pUpdater->SetCurrentState(IUpdater::FAIL); } - else if(!str_comp(pFileName, m_pUpdater->m_aLastFile)) - { - if(State() == HTTP_DONE) - m_pUpdater->SetCurrentState(IUpdater::MOVE_FILES); - else if(State() == HTTP_ERROR) - m_pUpdater->SetCurrentState(IUpdater::FAIL); - } } CUpdater::CUpdater() @@ -92,6 +83,7 @@ CUpdater::CUpdater() m_pHttp = nullptr; m_State = CLEAN; m_Percent = 0; + m_pCurrentTask = nullptr; IStorage::FormatTmpPath(m_aClientExecTmp, sizeof(m_aClientExecTmp), CLIENT_EXEC); IStorage::FormatTmpPath(m_aServerExecTmp, sizeof(m_aServerExecTmp), SERVER_EXEC); @@ -131,7 +123,10 @@ int CUpdater::GetCurrentPercent() void CUpdater::FetchFile(const char *pFile, const char *pDestPath) { - m_pHttp->Run(std::make_shared(this, pFile, pDestPath)); + CLockScope ls(m_Lock); + m_pCurrentTask = std::make_shared(this, pFile, pDestPath); + str_copy(m_aStatus, m_pCurrentTask->Dest()); + m_pHttp->Run(m_pCurrentTask); } bool CUpdater::MoveFile(const char *pFile) @@ -173,6 +168,9 @@ void CUpdater::Update() case IUpdater::GOT_MANIFEST: PerformUpdate(); break; + case IUpdater::DOWNLOADING: + RunningUpdate(); + break; case IUpdater::MOVE_FILES: CommitUpdate(); break; @@ -183,7 +181,7 @@ void CUpdater::Update() void CUpdater::AddFileJob(const char *pFile, bool Job) { - m_FileJobs[string(pFile)] = Job; + m_FileJobs.emplace_front(std::make_pair(pFile, Job)); } bool CUpdater::ReplaceClient() @@ -276,33 +274,39 @@ void CUpdater::ParseUpdate() void CUpdater::InitiateUpdate() { - m_State = GETTING_MANIFEST; + SetCurrentState(IUpdater::GETTING_MANIFEST); FetchFile("update.json"); } void CUpdater::PerformUpdate() { - m_State = PARSING_UPDATE; + SetCurrentState(IUpdater::PARSING_UPDATE); dbg_msg("updater", "parsing update.json"); ParseUpdate(); - m_State = DOWNLOADING; + m_CurrentJob = m_FileJobs.begin(); + SetCurrentState(IUpdater::DOWNLOADING); +} - const char *pLastFile; - pLastFile = ""; - for(map::reverse_iterator it = m_FileJobs.rbegin(); it != m_FileJobs.rend(); ++it) +void CUpdater::RunningUpdate() +{ + if(m_pCurrentTask) { - if(it->second) + if(!m_pCurrentTask->Done()) { - pLastFile = it->first.c_str(); - break; + return; + } + else if(m_pCurrentTask->State() == HTTP_ERROR || m_pCurrentTask->State() == HTTP_ABORTED) + { + SetCurrentState(IUpdater::FAIL); } } - for(auto &FileJob : m_FileJobs) + if(m_CurrentJob != m_FileJobs.end()) { - if(FileJob.second) + auto &Job = *m_CurrentJob; + if(Job.second) { - const char *pFile = FileJob.first.c_str(); + const char *pFile = Job.first.c_str(); size_t len = str_length(pFile); if(!str_comp_nocase(pFile + len - 4, ".dll")) { @@ -330,24 +334,32 @@ void CUpdater::PerformUpdate() { FetchFile(pFile); } - pLastFile = pFile; } else - m_pStorage->RemoveBinaryFile(FileJob.first.c_str()); - } + { + m_pStorage->RemoveBinaryFile(Job.first.c_str()); + } - if(m_ServerUpdate) - { - FetchFile(PLAT_SERVER_DOWN, m_aServerExecTmp); - pLastFile = m_aServerExecTmp; + m_CurrentJob++; } - if(m_ClientUpdate) + else { - FetchFile(PLAT_CLIENT_DOWN, m_aClientExecTmp); - pLastFile = m_aClientExecTmp; - } + if(m_ServerUpdate) + { + FetchFile(PLAT_SERVER_DOWN, m_aServerExecTmp); + m_ServerUpdate = false; + return; + } - str_copy(m_aLastFile, pLastFile); + if(m_ClientUpdate) + { + FetchFile(PLAT_SERVER_DOWN, m_aServerExecTmp); + m_ClientUpdate = false; + return; + } + + SetCurrentState(IUpdater::MOVE_FILES); + } } void CUpdater::CommitUpdate() diff --git a/src/engine/client/updater.h b/src/engine/client/updater.h index c62148fec..eb7e5f222 100644 --- a/src/engine/client/updater.h +++ b/src/engine/client/updater.h @@ -5,7 +5,8 @@ #include -#include +#include +#include #include #define CLIENT_EXEC "DDNet" @@ -34,6 +35,8 @@ #define PLAT_CLIENT_EXEC CLIENT_EXEC PLAT_EXT #define PLAT_SERVER_EXEC SERVER_EXEC PLAT_EXT +class CUpdaterFetchTask; + class CUpdater : public IUpdater { friend class CUpdaterFetchTask; @@ -48,21 +51,23 @@ class CUpdater : public IUpdater int m_State; char m_aStatus[256] GUARDED_BY(m_Lock); int m_Percent GUARDED_BY(m_Lock); - char m_aLastFile[256]; char m_aClientExecTmp[64]; char m_aServerExecTmp[64]; + std::forward_list> m_FileJobs; + std::shared_ptr m_pCurrentTask; + decltype(m_FileJobs)::iterator m_CurrentJob; + bool m_ClientUpdate; bool m_ServerUpdate; - std::map m_FileJobs; - void AddFileJob(const char *pFile, bool Job); void FetchFile(const char *pFile, const char *pDestPath = nullptr); bool MoveFile(const char *pFile); void ParseUpdate(); void PerformUpdate(); + void RunningUpdate(); void CommitUpdate(); bool ReplaceClient(); From 6eb51239c95bb3e8959ec15a1bdcf1fa245c9042 Mon Sep 17 00:00:00 2001 From: Learath2 Date: Fri, 12 Jan 2024 23:17:16 +0300 Subject: [PATCH 6/8] Fix some clang-tidy issues and UB --- src/engine/client/serverbrowser_http.cpp | 10 ++++------ src/engine/client/updater.cpp | 8 +++++--- src/engine/client/updater.h | 16 ++++++++-------- src/engine/shared/http.cpp | 11 ++++++++++- src/engine/shared/http.h | 3 ++- src/game/client/components/menus.h | 1 + src/game/client/components/menus_browser.cpp | 6 ++++++ 7 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/engine/client/serverbrowser_http.cpp b/src/engine/client/serverbrowser_http.cpp index 8d9734519..817428dc5 100644 --- a/src/engine/client/serverbrowser_http.cpp +++ b/src/engine/client/serverbrowser_http.cpp @@ -54,8 +54,8 @@ private: CChooseMaster *m_pParent; CLock m_Lock; std::shared_ptr m_pData; - std::shared_ptr m_pHead PT_GUARDED_BY(m_Lock); - std::shared_ptr m_pGet PT_GUARDED_BY(m_Lock); + std::shared_ptr m_pHead; + std::shared_ptr m_pGet; void Run() override REQUIRES(!m_Lock); public: @@ -174,7 +174,7 @@ void CChooseMaster::CJob::Run() { aTimeMs[i] = -1; const char *pUrl = m_pData->m_aaUrls[aRandomized[i]]; - std::shared_ptr pHead = std::move(HttpHead(pUrl)); + std::shared_ptr pHead = HttpHead(pUrl); pHead->Timeout(Timeout); pHead->LogProgress(HTTPLOG::FAILURE); { @@ -195,7 +195,7 @@ void CChooseMaster::CJob::Run() } auto StartTime = time_get_nanoseconds(); - std::shared_ptr pGet = std::move(HttpGet(pUrl)); + std::shared_ptr pGet = HttpGet(pUrl); pGet->Timeout(Timeout); pGet->LogProgress(HTTPLOG::FAILURE); { @@ -296,7 +296,6 @@ private: static bool Validate(json_value *pJson); static bool Parse(json_value *pJson, std::vector *pvServers, std::vector *pvLegacyServers); - IEngine *m_pEngine; IConsole *m_pConsole; IHttp *m_pHttp; @@ -309,7 +308,6 @@ private: }; CServerBrowserHttp::CServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IHttp *pHttp, const char **ppUrls, int NumUrls, int PreviousBestIndex) : - m_pEngine(pEngine), m_pConsole(pConsole), m_pHttp(pHttp), m_pChooseMaster(new CChooseMaster(pEngine, pHttp, Validate, ppUrls, NumUrls, PreviousBestIndex)) diff --git a/src/engine/client/updater.cpp b/src/engine/client/updater.cpp index eb238a709..3d1487e8f 100644 --- a/src/engine/client/updater.cpp +++ b/src/engine/client/updater.cpp @@ -163,7 +163,8 @@ bool CUpdater::MoveFile(const char *pFile) void CUpdater::Update() { - switch(m_State) + auto State = GetCurrentState(); + switch(State) { case IUpdater::GOT_MANIFEST: PerformUpdate(); @@ -270,6 +271,7 @@ void CUpdater::ParseUpdate() break; } } + json_value_free(pVersions); } void CUpdater::InitiateUpdate() @@ -375,9 +377,9 @@ void CUpdater::CommitUpdate() if(m_ServerUpdate) Success &= ReplaceServer(); if(!Success) - m_State = FAIL; + SetCurrentState(IUpdater::FAIL); else if(m_pClient->State() == IClient::STATE_ONLINE || m_pClient->EditorHasUnsavedData()) - m_State = NEED_RESTART; + SetCurrentState(IUpdater::NEED_RESTART); else { m_pClient->Restart(); diff --git a/src/engine/client/updater.h b/src/engine/client/updater.h index eb7e5f222..68148f161 100644 --- a/src/engine/client/updater.h +++ b/src/engine/client/updater.h @@ -48,7 +48,7 @@ class CUpdater : public IUpdater CLock m_Lock; - int m_State; + int m_State GUARDED_BY(m_Lock); char m_aStatus[256] GUARDED_BY(m_Lock); int m_Percent GUARDED_BY(m_Lock); char m_aClientExecTmp[64]; @@ -62,13 +62,13 @@ class CUpdater : public IUpdater bool m_ServerUpdate; void AddFileJob(const char *pFile, bool Job); - void FetchFile(const char *pFile, const char *pDestPath = nullptr); + void FetchFile(const char *pFile, const char *pDestPath = nullptr) REQUIRES(!m_Lock); bool MoveFile(const char *pFile); - void ParseUpdate(); - void PerformUpdate(); - void RunningUpdate(); - void CommitUpdate(); + void ParseUpdate() REQUIRES(!m_Lock); + void PerformUpdate() REQUIRES(!m_Lock); + void RunningUpdate() REQUIRES(!m_Lock); + void CommitUpdate() REQUIRES(!m_Lock); bool ReplaceClient(); bool ReplaceServer(); @@ -82,9 +82,9 @@ public: void GetCurrentFile(char *pBuf, int BufSize) override REQUIRES(!m_Lock); int GetCurrentPercent() override REQUIRES(!m_Lock); - void InitiateUpdate() override; + void InitiateUpdate() REQUIRES(!m_Lock) override; void Init(CHttp *pHttp); - void Update() override; + void Update() REQUIRES(!m_Lock) override; }; #endif diff --git a/src/engine/shared/http.cpp b/src/engine/shared/http.cpp index c866a7590..d87d76230 100644 --- a/src/engine/shared/http.cpp +++ b/src/engine/shared/http.cpp @@ -571,7 +571,7 @@ void CHttp::Run(std::shared_ptr pRequest) { std::unique_lock Lock(m_Lock); m_Cv.wait(Lock, [this]() { return m_State != UNINITIALIZED; }); - m_PendingRequests.emplace_back(std::move(std::static_pointer_cast(pRequest))); + m_PendingRequests.emplace_back(std::static_pointer_cast(pRequest)); curl_multi_wakeup(m_pMultiH); } @@ -584,3 +584,12 @@ void CHttp::Shutdown() m_Shutdown = true; curl_multi_wakeup(m_pMultiH); } + +CHttp::~CHttp() +{ + if(!m_pThread) + return; + + Shutdown(); + thread_wait(m_pThread); +} diff --git a/src/engine/shared/http.h b/src/engine/shared/http.h index b87d53d22..599872fef 100644 --- a/src/engine/shared/http.h +++ b/src/engine/shared/http.h @@ -132,7 +132,7 @@ protected: public: CHttpRequest(const char *pUrl); - ~CHttpRequest(); + virtual ~CHttpRequest(); void Timeout(CTimeout Timeout) { m_Timeout = Timeout; } void MaxResponseSize(int64_t MaxResponseSize) { m_MaxResponseSize = MaxResponseSize; } @@ -273,6 +273,7 @@ public: // User virtual void Run(std::shared_ptr pRequest) override; void Shutdown() override; + ~CHttp(); }; #endif // ENGINE_SHARED_HTTP_H diff --git a/src/game/client/components/menus.h b/src/game/client/components/menus.h index 6f45fd405..6f34b2456 100644 --- a/src/game/client/components/menus.h +++ b/src/game/client/components/menus.h @@ -536,6 +536,7 @@ protected: public: CCommunityIconLoadJob(CMenus *pMenus, const char *pCommunityId, int StorageType); + ~CCommunityIconLoadJob(); CImageInfo &&ImageInfo() { return std::move(m_ImageInfo); } }; diff --git a/src/game/client/components/menus_browser.cpp b/src/game/client/components/menus_browser.cpp index 1a1df4321..c2f508c40 100644 --- a/src/game/client/components/menus_browser.cpp +++ b/src/game/client/components/menus_browser.cpp @@ -1822,6 +1822,12 @@ CMenus::CCommunityIconLoadJob::CCommunityIconLoadJob(CMenus *pMenus, const char { } +CMenus::CCommunityIconLoadJob::~CCommunityIconLoadJob() +{ + free(m_ImageInfo.m_pData); + m_ImageInfo.m_pData = nullptr; +} + int CMenus::CommunityIconScan(const char *pName, int IsDir, int DirType, void *pUser) { const char *pExtension = ".png"; From bcf86d81f376b2f3d8e25ab26f489aa7a1c95bd7 Mon Sep 17 00:00:00 2001 From: Learath2 Date: Sat, 13 Jan 2024 15:27:56 +0300 Subject: [PATCH 7/8] Fix CRegister --- src/engine/server/register.cpp | 28 +++++++++++++++++----------- src/engine/server/register.h | 3 ++- src/engine/server/server.cpp | 6 ++++-- src/engine/server/server.h | 2 ++ src/engine/shared/http.h | 1 - 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/engine/server/register.cpp b/src/engine/server/register.cpp index 34acda919..85a30d864 100644 --- a/src/engine/server/register.cpp +++ b/src/engine/server/register.cpp @@ -70,17 +70,19 @@ class CRegister : public IRegister int m_Index; int m_InfoSerial; std::shared_ptr m_pShared; - std::unique_ptr m_pRegister; + std::shared_ptr m_pRegister; + IHttp *m_pHttp; void Run() override; public: - CJob(int Protocol, int ServerPort, int Index, int InfoSerial, std::shared_ptr pShared, std::unique_ptr &&pRegister) : + CJob(int Protocol, int ServerPort, int Index, int InfoSerial, std::shared_ptr pShared, std::shared_ptr &&pRegister, IHttp *pHttp) : m_Protocol(Protocol), m_ServerPort(ServerPort), m_Index(Index), m_InfoSerial(InfoSerial), m_pShared(std::move(pShared)), - m_pRegister(std::move(pRegister)) + m_pRegister(std::move(pRegister)), + m_pHttp(pHttp) { } ~CJob() override = default; @@ -110,6 +112,8 @@ class CRegister : public IRegister CConfig *m_pConfig; IConsole *m_pConsole; IEngine *m_pEngine; + IHttp *m_pHttp; + // Don't start sending registers before the server has initialized // completely. bool m_GotFirstUpdateCall = false; @@ -130,7 +134,7 @@ class CRegister : public IRegister char m_aServerInfo[16384]; public: - CRegister(CConfig *pConfig, IConsole *pConsole, IEngine *pEngine, int ServerPort, unsigned SixupSecurityToken); + CRegister(CConfig *pConfig, IConsole *pConsole, IEngine *pEngine, IHttp *pHttp, int ServerPort, unsigned SixupSecurityToken); void Update() override; void OnConfigChange() override; bool OnPacket(const CNetChunk *pPacket) override; @@ -309,7 +313,7 @@ void CRegister::CProtocol::SendRegister() RequestIndex = m_pShared->m_NumTotalRequests; m_pShared->m_NumTotalRequests += 1; } - m_pParent->m_pEngine->AddJob(std::make_shared(m_Protocol, m_pParent->m_ServerPort, RequestIndex, InfoSerial, m_pShared, std::move(pRegister))); + m_pParent->m_pEngine->AddJob(std::make_shared(m_Protocol, m_pParent->m_ServerPort, RequestIndex, InfoSerial, m_pShared, std::move(pRegister), m_pParent->m_pHttp)); m_NewChallengeToken = false; m_PrevRegister = Now; @@ -332,7 +336,7 @@ void CRegister::CProtocol::SendDeleteIfRegistered(bool Shutdown) char aSecret[UUID_MAXSTRSIZE]; FormatUuid(m_pParent->m_Secret, aSecret, sizeof(aSecret)); - std::unique_ptr pDelete = HttpPost(m_pParent->m_pConfig->m_SvRegisterUrl, (const unsigned char *)"", 0); + std::shared_ptr pDelete = HttpPost(m_pParent->m_pConfig->m_SvRegisterUrl, (const unsigned char *)"", 0); pDelete->HeaderString("Action", "delete"); pDelete->HeaderString("Address", aAddress); pDelete->HeaderString("Secret", aSecret); @@ -348,7 +352,7 @@ void CRegister::CProtocol::SendDeleteIfRegistered(bool Shutdown) pDelete->Timeout(CTimeout{1000, 1000, 0, 0}); } log_info(ProtocolToSystem(m_Protocol), "deleting..."); - m_pParent->m_pEngine->AddJob(std::move(pDelete)); + m_pParent->m_pHttp->Run(pDelete); } CRegister::CProtocol::CProtocol(CRegister *pParent, int Protocol) : @@ -405,7 +409,8 @@ void CRegister::CProtocol::OnToken(const char *pToken) void CRegister::CProtocol::CJob::Run() { - IEngine::RunJobBlocking(m_pRegister.get()); + m_pHttp->Run(m_pRegister); + m_pRegister->Wait(); if(m_pRegister->State() != HTTP_DONE) { // TODO: log the error response content from master @@ -471,10 +476,11 @@ void CRegister::CProtocol::CJob::Run() } } -CRegister::CRegister(CConfig *pConfig, IConsole *pConsole, IEngine *pEngine, int ServerPort, unsigned SixupSecurityToken) : +CRegister::CRegister(CConfig *pConfig, IConsole *pConsole, IEngine *pEngine, IHttp *pHttp, int ServerPort, unsigned SixupSecurityToken) : m_pConfig(pConfig), m_pConsole(pConsole), m_pEngine(pEngine), + m_pHttp(pHttp), m_ServerPort(ServerPort), m_aProtocols{ CProtocol(this, PROTOCOL_TW6_IPV6), @@ -749,7 +755,7 @@ void CRegister::OnShutdown() } } -IRegister *CreateRegister(CConfig *pConfig, IConsole *pConsole, IEngine *pEngine, int ServerPort, unsigned SixupSecurityToken) +IRegister *CreateRegister(CConfig *pConfig, IConsole *pConsole, IEngine *pEngine, IHttp *pHttp, int ServerPort, unsigned SixupSecurityToken) { - return new CRegister(pConfig, pConsole, pEngine, ServerPort, SixupSecurityToken); + return new CRegister(pConfig, pConsole, pEngine, pHttp, ServerPort, SixupSecurityToken); } diff --git a/src/engine/server/register.h b/src/engine/server/register.h index 85bdf7a64..1fbfc3c56 100644 --- a/src/engine/server/register.h +++ b/src/engine/server/register.h @@ -4,6 +4,7 @@ class CConfig; class IConsole; class IEngine; +class IHttp; struct CNetChunk; class IRegister @@ -23,6 +24,6 @@ public: virtual void OnShutdown() = 0; }; -IRegister *CreateRegister(CConfig *pConfig, IConsole *pConsole, IEngine *pEngine, int ServerPort, unsigned SixupSecurityToken); +IRegister *CreateRegister(CConfig *pConfig, IConsole *pConsole, IEngine *pEngine, IHttp *pHttp, int ServerPort, unsigned SixupSecurityToken); #endif diff --git a/src/engine/server/server.cpp b/src/engine/server/server.cpp index 753faf03b..c4d4de7b8 100644 --- a/src/engine/server/server.cpp +++ b/src/engine/server/server.cpp @@ -2774,7 +2774,8 @@ int CServer::Run() #endif IEngine *pEngine = Kernel()->RequestInterface(); - m_pRegister = CreateRegister(&g_Config, m_pConsole, pEngine, this->Port(), m_NetServer.GetGlobalToken()); + IHttp *pHttp = Kernel()->RequestInterface(); + m_pRegister = CreateRegister(&g_Config, m_pConsole, pEngine, pHttp, this->Port(), m_NetServer.GetGlobalToken()); m_NetServer.SetCallbacks(NewClientCallback, NewClientNoAuthCallback, ClientRejoinCallback, DelClientCallback, this); @@ -3822,7 +3823,8 @@ void CServer::RegisterCommands() m_pStorage = Kernel()->RequestInterface(); m_pAntibot = Kernel()->RequestInterface(); - HttpInit(m_pStorage); + m_Http.Init(std::chrono::seconds{2}); + Kernel()->RegisterInterface(static_cast(&m_Http), false); // register console commands Console()->Register("kick", "i[id] ?r[reason]", CFGFLAG_SERVER, ConKick, this, "Kick player with specified id for any reason"); diff --git a/src/engine/server/server.h b/src/engine/server/server.h index a5dcb34ae..2aed3098b 100644 --- a/src/engine/server/server.h +++ b/src/engine/server/server.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -238,6 +239,7 @@ public: CEcon m_Econ; CFifo m_Fifo; CServerBan m_ServerBan; + CHttp m_Http; IEngineMap *m_pMap; diff --git a/src/engine/shared/http.h b/src/engine/shared/http.h index 599872fef..2b7fe66e1 100644 --- a/src/engine/shared/http.h +++ b/src/engine/shared/http.h @@ -234,7 +234,6 @@ inline std::unique_ptr HttpPostJson(const char *pUrl, const char * return pResult; } -bool HttpInit(IStorage *pStorage); void EscapeUrl(char *pBuf, int Size, const char *pStr); bool HttpHasIpresolveBug(); From f5910343e23b177a9810fe2061572a5416ac33c2 Mon Sep 17 00:00:00 2001 From: Learath2 Date: Sun, 14 Jan 2024 14:33:05 +0300 Subject: [PATCH 8/8] Fix CI --- src/engine/shared/http.cpp | 29 +++++++++++++++++------------ src/engine/shared/http.h | 1 + 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/engine/shared/http.cpp b/src/engine/shared/http.cpp index d87d76230..9e5bf8bfb 100644 --- a/src/engine/shared/http.cpp +++ b/src/engine/shared/http.cpp @@ -18,6 +18,11 @@ #define WIN32_LEAN_AND_MEAN #include +// There is a stray constant on Windows/MSVC... +#ifdef ERROR +#undef ERROR +#endif + int CurlDebug(CURL *pHandle, curl_infotype Type, char *pData, size_t DataSize, void *pUser) { char TypeChar; @@ -394,8 +399,8 @@ bool CHttp::Init(std::chrono::milliseconds ShutdownDelay) m_pThread = thread_init(CHttp::ThreadMain, this, "http"); std::unique_lock Lock(m_Lock); - m_Cv.wait(Lock, [this]() { return m_State != UNINITIALIZED; }); - if(m_State != RUNNING) + m_Cv.wait(Lock, [this]() { return m_State != CHttp::UNINITIALIZED; }); + if(m_State != CHttp::RUNNING) { return false; } @@ -415,7 +420,7 @@ void CHttp::RunLoop() if(curl_global_init(CURL_GLOBAL_DEFAULT)) { dbg_msg("http", "curl_global_init failed"); - m_State = ERROR; + m_State = CHttp::ERROR; m_Cv.notify_all(); return; } @@ -424,7 +429,7 @@ void CHttp::RunLoop() if(!m_pMultiH) { dbg_msg("http", "curl_multi_init failed"); - m_State = ERROR; + m_State = CHttp::ERROR; m_Cv.notify_all(); return; } @@ -435,12 +440,12 @@ void CHttp::RunLoop() dbg_msg("http", "libcurl version %s (compiled = " LIBCURL_VERSION ")", pVersion->version); } - m_State = RUNNING; + m_State = CHttp::RUNNING; m_Cv.notify_all(); dbg_msg("http", "running"); Lock.unlock(); - while(m_State == RUNNING) + while(m_State == CHttp::RUNNING) { static int NextTimeout = std::numeric_limits::max(); int Events = 0; @@ -465,7 +470,7 @@ void CHttp::RunLoop() { Lock.lock(); dbg_msg("http", "Failed multi wait: %s", curl_multi_strerror(mc)); - m_State = ERROR; + m_State = CHttp::ERROR; break; } @@ -474,7 +479,7 @@ void CHttp::RunLoop() { Lock.lock(); dbg_msg("http", "Failed multi perform: %s", curl_multi_strerror(mc)); - m_State = ERROR; + m_State = CHttp::ERROR; break; } @@ -525,7 +530,7 @@ void CHttp::RunLoop() error_init: dbg_msg("http", "failed to start new request"); Lock.lock(); - m_State = ERROR; + m_State = CHttp::ERROR; break; } @@ -540,7 +545,7 @@ void CHttp::RunLoop() if(!Lock.owns_lock()) Lock.lock(); - bool Cleanup = m_State != ERROR; + bool Cleanup = m_State != CHttp::ERROR; for(auto &pRequest : m_PendingRequests) { str_copy(pRequest->m_aErr, "Shutting down"); @@ -570,7 +575,7 @@ void CHttp::RunLoop() void CHttp::Run(std::shared_ptr pRequest) { std::unique_lock Lock(m_Lock); - m_Cv.wait(Lock, [this]() { return m_State != UNINITIALIZED; }); + m_Cv.wait(Lock, [this]() { return m_State != CHttp::UNINITIALIZED; }); m_PendingRequests.emplace_back(std::static_pointer_cast(pRequest)); curl_multi_wakeup(m_pMultiH); } @@ -578,7 +583,7 @@ void CHttp::Run(std::shared_ptr pRequest) void CHttp::Shutdown() { std::unique_lock Lock(m_Lock); - if(m_Shutdown || m_State != RUNNING) + if(m_Shutdown || m_State != CHttp::RUNNING) return; m_Shutdown = true; diff --git a/src/engine/shared/http.h b/src/engine/shared/http.h index 2b7fe66e1..e2e6604af 100644 --- a/src/engine/shared/http.h +++ b/src/engine/shared/http.h @@ -11,6 +11,7 @@ #include #include #include +#include #include