Skip to content

Commit 1542ee1

Browse files
tintinhamanstintinhamans
committed
fix(online): harden lobby and social async state
1 parent c929fa0 commit 1542ee1

4 files changed

Lines changed: 64 additions & 35 deletions

File tree

‎GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "Common/Player.h"
99
#include "GameClient/InGameUI.h"
1010
#include "GameLogic/VictoryConditions.h"
11+
#include <atomic>
1112

1213
extern NGMPGame* TheNGMPGame;
1314

@@ -196,17 +197,17 @@ class NGMP_OnlineServices_LobbyInterface
196197

197198
void SetLobbyListDirty()
198199
{
199-
m_bLobbyListDirty = true;
200+
m_bLobbyListDirty.store(true);
200201
}
201202

202203
void ConsumeLobbyListDirtyFlag()
203204
{
204-
m_bLobbyListDirty = false;
205+
m_bLobbyListDirty.store(false);
205206
}
206207

207-
bool IsLobbyListDirty()
208+
bool IsLobbyListDirty() const
208209
{
209-
return m_bLobbyListDirty;
210+
return m_bLobbyListDirty.load();
210211
}
211212

212213
UnicodeString m_PendingCreation_LobbyName;
@@ -463,7 +464,7 @@ class NGMP_OnlineServices_LobbyInterface
463464
// TODO_NGMP: cleanup
464465
NetworkMesh* m_pLobbyMesh = nullptr;
465466

466-
bool m_bLobbyListDirty = false;
467+
std::atomic_bool m_bLobbyListDirty = false;
467468

468469

469470
#if !defined(GENERALS_ONLINE_DISABLE_AUTO_ACCEPT)
@@ -473,7 +474,7 @@ class NGMP_OnlineServices_LobbyInterface
473474
bool m_bAttemptingToJoinLobby = false;
474475
LobbyEntry m_LobbyTryingToJoin;
475476

476-
bool m_bSearchInProgress = false;
477+
std::atomic_bool m_bSearchInProgress = false;
477478

478479
bool m_bMarkedGameAsFinished = false;
479480

‎GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_SocialInterface.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class NGMP_OnlineServices_SocialInterface
9595
m_cbOnChatMessage = cbOnChatMessage;
9696
}
9797

98-
std::unordered_map<int64_t, FriendsEntry> GetRecentlyPlayedWithList()
98+
const std::unordered_map<int64_t, FriendsEntry>& GetRecentlyPlayedWithList()
9999
{
100100
// is it stale? clear it out
101101
const int64_t recentPlayersListLifespan = 600000; // 10 minutes
@@ -110,11 +110,13 @@ class NGMP_OnlineServices_SocialInterface
110110

111111
std::unordered_map<int64_t, FriendsEntry> GetCachedFriendsList()
112112
{
113+
std::scoped_lock<std::mutex> lock(m_friendsMapMutex);
113114
return m_mapFriends;
114115
}
115116

116117
std::unordered_map<int64_t, FriendsEntry> GetCachedRequestsList()
117118
{
119+
std::scoped_lock<std::mutex> lock(m_friendsMapMutex);
118120
return m_mapPendingRequests;
119121
}
120122

@@ -177,6 +179,8 @@ class NGMP_OnlineServices_SocialInterface
177179
std::function<void(int64_t source_user_id, int64_t target_user_id, UnicodeString unicodeStr)> m_cbOnChatMessage = nullptr;
178180

179181
// Cached, may be out of date if friends UI isnt active, optimized for lookup
182+
// Rebuilt on the HTTP thread and read from the main thread, so all access goes through m_friendsMapMutex.
183+
mutable std::mutex m_friendsMapMutex;
180184
std::unordered_map<int64_t, FriendsEntry> m_mapFriends;
181185
std::unordered_map<int64_t, FriendsEntry> m_mapPendingRequests;
182186
std::unordered_map<int64_t, FriendsEntry> m_mapBlocked;

‎GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -519,24 +519,35 @@ NGMP_OnlineServices_LobbyInterface::NGMP_OnlineServices_LobbyInterface()
519519

520520
void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function<void()> onStartCallback, std::function<void(std::vector<LobbyEntry>)> onCompleteCallback)
521521
{
522-
if (m_bSearchInProgress)
522+
const bool lobbyListWasDirty = m_bLobbyListDirty.exchange(false);
523+
if (m_bSearchInProgress.exchange(true))
523524
{
525+
if (lobbyListWasDirty)
526+
{
527+
m_bLobbyListDirty.store(true);
528+
}
524529
return;
525530
}
526531

532+
// A notification received after the exchange above stays set and triggers a later refresh.
527533
m_fnCallbackSearchForLobbiesComplete = onCompleteCallback;
528-
529-
m_bSearchInProgress = true;
530-
m_vecLobbies.clear();
534+
if (onStartCallback != nullptr)
535+
{
536+
onStartCallback();
537+
}
531538

532539
std::string strURI = NGMP_OnlineServicesManager::GetAPIEndpoint("Lobbies");
533540
std::map<std::string, std::string> mapHeaders;
534541

535542
NGMP_OnlineServicesManager::GetInstance()->GetHTTPManager()->SendGETRequest(strURI.c_str(), EIPProtocolVersion::DONT_CARE, mapHeaders, [=](bool bSuccess, int statusCode, std::string strBody, HTTPRequest* pReq)
536543
{
537-
try
544+
bool bSearchSucceeded = false;
545+
if (bSuccess && statusCode == 200)
538546
{
547+
try
548+
{
539549
nlohmann::json jsonObject = nlohmann::json::parse(strBody);
550+
std::vector<LobbyEntry> parsedLobbies;
540551

541552
std::vector<int> vecLatencies;
542553
std::map<int64_t, int> mapPlayerLatencies;
@@ -632,19 +643,29 @@ void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function<void()>
632643
lobbyEntry.members.push_back(memberEntry);
633644
}
634645

635-
m_vecLobbies.push_back(lobbyEntry);
646+
parsedLobbies.push_back(std::move(lobbyEntry));
636647
}
637-
}
638-
catch (...)
639-
{
648+
m_vecLobbies = std::move(parsedLobbies);
649+
bSearchSucceeded = true;
650+
}
651+
catch (...)
652+
{
640653

641-
}
654+
}
655+
}
642656

643-
if (m_fnCallbackSearchForLobbiesComplete != nullptr)
644-
{
645-
m_fnCallbackSearchForLobbiesComplete(m_vecLobbies);
646-
}
647-
m_bSearchInProgress = false;
657+
if (!bSearchSucceeded)
658+
{
659+
m_bLobbyListDirty.store(true);
660+
}
661+
662+
const std::function<void(std::vector<LobbyEntry>)> completionCallback = m_fnCallbackSearchForLobbiesComplete;
663+
const std::vector<LobbyEntry> lobbies = m_vecLobbies;
664+
m_bSearchInProgress.store(false);
665+
if (completionCallback != nullptr)
666+
{
667+
completionCallback(lobbies);
668+
}
648669
});
649670
}
650671

‎GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_SocialInterface.cpp‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,12 @@ void NGMP_OnlineServices_SocialInterface::GetFriendsList(bool bUseCache, std::fu
4444

4545
try
4646
{
47-
// Note: m_mapFriends and m_mapPendingRequests access happens in HTTP thread context
48-
// This is a design issue but adding lock would be too invasive at this point
49-
// The callback execution uses localCallback which is safe
5047
NGMP_OnlineServices_SocialInterface* pThis = NGMP_OnlineServicesManager::GetInterface<NGMP_OnlineServices_SocialInterface>();
5148
if (pThis == nullptr)
5249
return;
5350

54-
pThis->m_mapFriends.clear();
55-
pThis->m_mapPendingRequests.clear();
51+
std::unordered_map<int64_t, FriendsEntry> mapFriends{};
52+
std::unordered_map<int64_t, FriendsEntry> mapPendingRequests{};
5653

5754
nlohmann::json jsonObject = nlohmann::json::parse(strBody);
5855

@@ -70,7 +67,7 @@ void NGMP_OnlineServices_SocialInterface::GetFriendsList(bool bUseCache, std::fu
7067
friendsResult.vecFriends.push_back(newFriend);
7168

7269
// cache
73-
pThis->m_mapFriends[newFriend.user_id] = newFriend;
70+
mapFriends.insert_or_assign(newFriend.user_id, newFriend);
7471
}
7572

7673
// pending requests
@@ -85,8 +82,12 @@ void NGMP_OnlineServices_SocialInterface::GetFriendsList(bool bUseCache, std::fu
8582
friendsResult.vecPendingRequests.push_back(newEntry);
8683

8784
// cache
88-
pThis->m_mapPendingRequests[newEntry.user_id] = newEntry;
85+
mapPendingRequests.insert_or_assign(newEntry.user_id, newEntry);
8986
}
87+
88+
std::scoped_lock<std::mutex> lock(pThis->m_friendsMapMutex);
89+
pThis->m_mapFriends.swap(mapFriends);
90+
pThis->m_mapPendingRequests.swap(mapPendingRequests);
9091
}
9192
catch (...)
9293
{
@@ -112,10 +113,9 @@ void NGMP_OnlineServices_SocialInterface::GetBlockList(std::function<void(Blocke
112113
{
113114
BlockedResult blockedResult;
114115

115-
m_mapBlocked.clear();
116-
117116
try
118117
{
118+
std::unordered_map<int64_t, FriendsEntry> mapBlocked{};
119119
nlohmann::json jsonObject = nlohmann::json::parse(strBody);
120120

121121
for (const auto& blockedEntryIter : jsonObject["blocked"])
@@ -129,8 +129,11 @@ void NGMP_OnlineServices_SocialInterface::GetBlockList(std::function<void(Blocke
129129
blockedResult.vecBlocked.push_back(newEntry);
130130

131131
// cache
132-
m_mapBlocked[newEntry.user_id] = newEntry;
132+
mapBlocked.insert_or_assign(newEntry.user_id, newEntry);
133133
}
134+
135+
std::scoped_lock<std::mutex> lock(m_friendsMapMutex);
136+
m_mapBlocked.swap(mapBlocked);
134137
}
135138
catch (...)
136139
{
@@ -285,7 +288,6 @@ void NGMP_OnlineServices_SocialInterface::OnChatMessage(int64_t source_user_id,
285288
}
286289
}
287290
m_mapCachedMessages[user_id_to_store].push_back(unicodeStr);
288-
289291
if (m_cbOnChatMessage != nullptr)
290292
{
291293
m_cbOnChatMessage(source_user_id, target_user_id, unicodeStr);
@@ -346,16 +348,17 @@ void NGMP_OnlineServices_SocialInterface::OnFriendRequestAccepted(std::string st
346348

347349
bool NGMP_OnlineServices_SocialInterface::IsUserIgnored(int64_t target_user_id)
348350
{
351+
const std::scoped_lock<std::mutex> lock(m_friendsMapMutex);
349352
return m_mapBlocked.contains(target_user_id);
350353
}
351-
352354
bool NGMP_OnlineServices_SocialInterface::IsUserFriend(int64_t target_user_id)
353355
{
356+
const std::scoped_lock<std::mutex> lock(m_friendsMapMutex);
354357
return m_mapFriends.contains(target_user_id);
355358
}
356-
357359
bool NGMP_OnlineServices_SocialInterface::IsUserPendingRequest(int64_t target_user_id)
358360
{
361+
std::scoped_lock<std::mutex> lock(m_friendsMapMutex);
359362
return m_mapPendingRequests.contains(target_user_id);
360363
}
361364

0 commit comments

Comments
 (0)