From 1542ee174e93bb864c82ea3c56eeb47ff550b8aa Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Fri, 21 Aug 2026 09:01:52 +0200 Subject: [PATCH] fix(online): harden lobby and social async state --- .../OnlineServices_LobbyInterface.h | 13 ++--- .../OnlineServices_SocialInterface.h | 6 ++- .../OnlineServices_LobbyInterface.cpp | 51 +++++++++++++------ .../OnlineServices_SocialInterface.cpp | 29 ++++++----- 4 files changed, 64 insertions(+), 35 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h index 91304e02be5..23d585180f0 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h @@ -8,6 +8,7 @@ #include "Common/Player.h" #include "GameClient/InGameUI.h" #include "GameLogic/VictoryConditions.h" +#include extern NGMPGame* TheNGMPGame; @@ -196,17 +197,17 @@ class NGMP_OnlineServices_LobbyInterface void SetLobbyListDirty() { - m_bLobbyListDirty = true; + m_bLobbyListDirty.store(true); } void ConsumeLobbyListDirtyFlag() { - m_bLobbyListDirty = false; + m_bLobbyListDirty.store(false); } - bool IsLobbyListDirty() + bool IsLobbyListDirty() const { - return m_bLobbyListDirty; + return m_bLobbyListDirty.load(); } UnicodeString m_PendingCreation_LobbyName; @@ -463,7 +464,7 @@ class NGMP_OnlineServices_LobbyInterface // TODO_NGMP: cleanup NetworkMesh* m_pLobbyMesh = nullptr; - bool m_bLobbyListDirty = false; + std::atomic_bool m_bLobbyListDirty = false; #if !defined(GENERALS_ONLINE_DISABLE_AUTO_ACCEPT) @@ -473,7 +474,7 @@ class NGMP_OnlineServices_LobbyInterface bool m_bAttemptingToJoinLobby = false; LobbyEntry m_LobbyTryingToJoin; - bool m_bSearchInProgress = false; + std::atomic_bool m_bSearchInProgress = false; bool m_bMarkedGameAsFinished = false; diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_SocialInterface.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_SocialInterface.h index 7c44c04b658..8dd72586073 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_SocialInterface.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_SocialInterface.h @@ -95,7 +95,7 @@ class NGMP_OnlineServices_SocialInterface m_cbOnChatMessage = cbOnChatMessage; } - std::unordered_map GetRecentlyPlayedWithList() + const std::unordered_map& GetRecentlyPlayedWithList() { // is it stale? clear it out const int64_t recentPlayersListLifespan = 600000; // 10 minutes @@ -110,11 +110,13 @@ class NGMP_OnlineServices_SocialInterface std::unordered_map GetCachedFriendsList() { + std::scoped_lock lock(m_friendsMapMutex); return m_mapFriends; } std::unordered_map GetCachedRequestsList() { + std::scoped_lock lock(m_friendsMapMutex); return m_mapPendingRequests; } @@ -177,6 +179,8 @@ class NGMP_OnlineServices_SocialInterface std::function m_cbOnChatMessage = nullptr; // Cached, may be out of date if friends UI isnt active, optimized for lookup + // Rebuilt on the HTTP thread and read from the main thread, so all access goes through m_friendsMapMutex. + mutable std::mutex m_friendsMapMutex; std::unordered_map m_mapFriends; std::unordered_map m_mapPendingRequests; std::unordered_map m_mapBlocked; diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp index fcc2e1d2d16..86de1fe0c4b 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp @@ -519,24 +519,35 @@ NGMP_OnlineServices_LobbyInterface::NGMP_OnlineServices_LobbyInterface() void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function onStartCallback, std::function)> onCompleteCallback) { - if (m_bSearchInProgress) + const bool lobbyListWasDirty = m_bLobbyListDirty.exchange(false); + if (m_bSearchInProgress.exchange(true)) { + if (lobbyListWasDirty) + { + m_bLobbyListDirty.store(true); + } return; } + // A notification received after the exchange above stays set and triggers a later refresh. m_fnCallbackSearchForLobbiesComplete = onCompleteCallback; - - m_bSearchInProgress = true; - m_vecLobbies.clear(); + if (onStartCallback != nullptr) + { + onStartCallback(); + } std::string strURI = NGMP_OnlineServicesManager::GetAPIEndpoint("Lobbies"); std::map mapHeaders; NGMP_OnlineServicesManager::GetInstance()->GetHTTPManager()->SendGETRequest(strURI.c_str(), EIPProtocolVersion::DONT_CARE, mapHeaders, [=](bool bSuccess, int statusCode, std::string strBody, HTTPRequest* pReq) { - try + bool bSearchSucceeded = false; + if (bSuccess && statusCode == 200) { + try + { nlohmann::json jsonObject = nlohmann::json::parse(strBody); + std::vector parsedLobbies; std::vector vecLatencies; std::map mapPlayerLatencies; @@ -632,19 +643,29 @@ void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function lobbyEntry.members.push_back(memberEntry); } - m_vecLobbies.push_back(lobbyEntry); + parsedLobbies.push_back(std::move(lobbyEntry)); } - } - catch (...) - { + m_vecLobbies = std::move(parsedLobbies); + bSearchSucceeded = true; + } + catch (...) + { - } + } + } - if (m_fnCallbackSearchForLobbiesComplete != nullptr) - { - m_fnCallbackSearchForLobbiesComplete(m_vecLobbies); - } - m_bSearchInProgress = false; + if (!bSearchSucceeded) + { + m_bLobbyListDirty.store(true); + } + + const std::function)> completionCallback = m_fnCallbackSearchForLobbiesComplete; + const std::vector lobbies = m_vecLobbies; + m_bSearchInProgress.store(false); + if (completionCallback != nullptr) + { + completionCallback(lobbies); + } }); } diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_SocialInterface.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_SocialInterface.cpp index a1f55f7e0a4..6ab4574c045 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_SocialInterface.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_SocialInterface.cpp @@ -44,15 +44,12 @@ void NGMP_OnlineServices_SocialInterface::GetFriendsList(bool bUseCache, std::fu try { - // Note: m_mapFriends and m_mapPendingRequests access happens in HTTP thread context - // This is a design issue but adding lock would be too invasive at this point - // The callback execution uses localCallback which is safe NGMP_OnlineServices_SocialInterface* pThis = NGMP_OnlineServicesManager::GetInterface(); if (pThis == nullptr) return; - pThis->m_mapFriends.clear(); - pThis->m_mapPendingRequests.clear(); + std::unordered_map mapFriends{}; + std::unordered_map mapPendingRequests{}; nlohmann::json jsonObject = nlohmann::json::parse(strBody); @@ -70,7 +67,7 @@ void NGMP_OnlineServices_SocialInterface::GetFriendsList(bool bUseCache, std::fu friendsResult.vecFriends.push_back(newFriend); // cache - pThis->m_mapFriends[newFriend.user_id] = newFriend; + mapFriends.insert_or_assign(newFriend.user_id, newFriend); } // pending requests @@ -85,8 +82,12 @@ void NGMP_OnlineServices_SocialInterface::GetFriendsList(bool bUseCache, std::fu friendsResult.vecPendingRequests.push_back(newEntry); // cache - pThis->m_mapPendingRequests[newEntry.user_id] = newEntry; + mapPendingRequests.insert_or_assign(newEntry.user_id, newEntry); } + + std::scoped_lock lock(pThis->m_friendsMapMutex); + pThis->m_mapFriends.swap(mapFriends); + pThis->m_mapPendingRequests.swap(mapPendingRequests); } catch (...) { @@ -112,10 +113,9 @@ void NGMP_OnlineServices_SocialInterface::GetBlockList(std::function mapBlocked{}; nlohmann::json jsonObject = nlohmann::json::parse(strBody); for (const auto& blockedEntryIter : jsonObject["blocked"]) @@ -129,8 +129,11 @@ void NGMP_OnlineServices_SocialInterface::GetBlockList(std::function lock(m_friendsMapMutex); + m_mapBlocked.swap(mapBlocked); } catch (...) { @@ -285,7 +288,6 @@ void NGMP_OnlineServices_SocialInterface::OnChatMessage(int64_t source_user_id, } } m_mapCachedMessages[user_id_to_store].push_back(unicodeStr); - if (m_cbOnChatMessage != nullptr) { m_cbOnChatMessage(source_user_id, target_user_id, unicodeStr); @@ -346,16 +348,17 @@ void NGMP_OnlineServices_SocialInterface::OnFriendRequestAccepted(std::string st bool NGMP_OnlineServices_SocialInterface::IsUserIgnored(int64_t target_user_id) { + const std::scoped_lock lock(m_friendsMapMutex); return m_mapBlocked.contains(target_user_id); } - bool NGMP_OnlineServices_SocialInterface::IsUserFriend(int64_t target_user_id) { + const std::scoped_lock lock(m_friendsMapMutex); return m_mapFriends.contains(target_user_id); } - bool NGMP_OnlineServices_SocialInterface::IsUserPendingRequest(int64_t target_user_id) { + std::scoped_lock lock(m_friendsMapMutex); return m_mapPendingRequests.contains(target_user_id); }