From 06b515930671a477f1123ae7ccdfeb7c158f660a Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 26 Sep 2026 17:50:51 +0200 Subject: [PATCH 01/12] fix(lobby): Make lobby ID assignment and AC probe counters atomic --- GenOnlineService/LobbyManager.cs | 51 +++++++++++--------------------- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/GenOnlineService/LobbyManager.cs b/GenOnlineService/LobbyManager.cs index 4dea5c5..f8a1016 100644 --- a/GenOnlineService/LobbyManager.cs +++ b/GenOnlineService/LobbyManager.cs @@ -149,38 +149,19 @@ public class Lobby ConcurrentDictionary m_dictProbe2_Received = new(); public void RegisterProbeSent_Type1(Int64 userID) { - if (!m_dictProbe1_Sent.ContainsKey(userID)) - { - m_dictProbe1_Sent[userID] = 1; - } - else - { - ++m_dictProbe1_Sent[userID]; - } + // AddOrUpdate is atomic; the previous check-then-write on the dictionary indexer + // could lose an increment when two probes for the same user race. + m_dictProbe1_Sent.AddOrUpdate(userID, 1, (_, count) => count + 1); } public void RegisterProbeSent_Type2(Int64 userID) { - if (!m_dictProbe2_Sent.ContainsKey(userID)) - { - m_dictProbe2_Sent[userID] = 1; - } - else - { - ++m_dictProbe2_Sent[userID]; - } + m_dictProbe2_Sent.AddOrUpdate(userID, 1, (_, count) => count + 1); } public void RegisterProbeResponse_Type1(Int64 userID) { - if (!m_dictProbe1_Received.ContainsKey(userID)) - { - m_dictProbe1_Received[userID] = 1; - } - else - { - ++m_dictProbe1_Received[userID]; - } + m_dictProbe1_Received.AddOrUpdate(userID, 1, (_, count) => count + 1); } public async Task RegisterProbeResponse_Malformed_Type1(Int64 userID) @@ -194,14 +175,7 @@ public async Task RegisterProbeResponse_Malformed_Type1(Int64 userID) public void RegisterProbeResponse_Type2(Int64 userID) { - if (!m_dictProbe2_Received.ContainsKey(userID)) - { - m_dictProbe2_Received[userID] = 1; - } - else - { - ++m_dictProbe2_Received[userID]; - } + m_dictProbe2_Received.AddOrUpdate(userID, 1, (_, count) => count + 1); } public async Task RegisterProbeResponse_Malformed_Type2(Int64 userID) @@ -1783,6 +1757,16 @@ public class LobbyManager private Int64 m_NextLobbyID = 0; + // Concurrent CreateLobby calls (e.g. a custom lobby and a QuickMatch allocation racing) must never be + // handed the same ID. Interlocked.Increment returns the post-increment value, so subtracting 1 keeps + // the original "starts at 0" sequence while making the read-and-bump atomic. + // Internal (not private) so it is unit-testable via InternalsVisibleTo without needing the rest of + // CreateLobby's database dependencies. + internal Int64 GenerateNextLobbyID() + { + return Interlocked.Increment(ref m_NextLobbyID) - 1; + } + private readonly IServiceProvider _services; public LobbyManager(IServiceProvider services) @@ -1859,8 +1843,7 @@ public async Task CreateLobby(AppDbContext _db, UserSession owningSession int rng_seed = new Random().Next(); - Int64 newLobbyID = m_NextLobbyID; - ++m_NextLobbyID; + Int64 newLobbyID = GenerateNextLobbyID(); // load and apply user preferences (custom game only) bool bLimitSuperweapons = false; From 3bf8f70d14abbf16fc0778fcd2dc34e7f1b2af28 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 26 Sep 2026 17:51:27 +0200 Subject: [PATCH 02/12] fix(lobby): Await LeaveAnyLobby before creating or joining a lobby --- GenOnlineService/Controllers/Lobby/LobbyController.cs | 4 ++-- GenOnlineService/LobbyManager.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/GenOnlineService/Controllers/Lobby/LobbyController.cs b/GenOnlineService/Controllers/Lobby/LobbyController.cs index f53d46f..79029fc 100644 --- a/GenOnlineService/Controllers/Lobby/LobbyController.cs +++ b/GenOnlineService/Controllers/Lobby/LobbyController.cs @@ -833,7 +833,7 @@ public async Task Put(Int64 lobbyID) if (playerSession != null) { // leave any lobby - _lobbyManager.LeaveAnyLobby(user_id); + await _lobbyManager.LeaveAnyLobby(user_id); await using var db = await _dbFactory.CreateDbContextAsync(); string strDisplayName = await Database.Users.GetDisplayName(db, user_id); @@ -879,4 +879,4 @@ public async Task Put(Int64 lobbyID) } } -} \ No newline at end of file +} diff --git a/GenOnlineService/LobbyManager.cs b/GenOnlineService/LobbyManager.cs index f8a1016..ef1d4f7 100644 --- a/GenOnlineService/LobbyManager.cs +++ b/GenOnlineService/LobbyManager.cs @@ -1839,7 +1839,7 @@ public async Task CreateLobby(AppDbContext _db, UserSession owningSession await CleanupUserLobbiesNotStarted(owningSession.m_UserID); Console.WriteLine("[Source 3] User {0} Leave Any Lobby", owningSession.m_UserID); - this.LeaveAnyLobby(owningSession.m_UserID); + await this.LeaveAnyLobby(owningSession.m_UserID); int rng_seed = new Random().Next(); From 87e81afc580549a5b5bcf6f5c3e05c3ede87b393 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 26 Sep 2026 23:08:23 +0200 Subject: [PATCH 03/12] fix(lobby): Serialize lobby mutations behind one per-lobby gate --- .../Controllers/Lobby/LobbyController.cs | 533 +++++++++--------- .../WebSocket/WebSocketController.cs | 19 +- GenOnlineService/LobbyManager.cs | 120 ++-- GenOnlineService/MatchmakingManager.cs | 2 +- 4 files changed, 365 insertions(+), 309 deletions(-) diff --git a/GenOnlineService/Controllers/Lobby/LobbyController.cs b/GenOnlineService/Controllers/Lobby/LobbyController.cs index 79029fc..4e73922 100644 --- a/GenOnlineService/Controllers/Lobby/LobbyController.cs +++ b/GenOnlineService/Controllers/Lobby/LobbyController.cs @@ -464,292 +464,303 @@ public async Task Post(Int64 lobbyID) } } - // reset everyones ready states when anything changes (minus dummy actions) - if (field != ELobbyUpdateField.HOST_ACTION_FORCE_START - && field != ELobbyUpdateField.LOCAL_PLAYER_HAS_MAP - && field != ELobbyUpdateField.HOST_ACTION_KICK_USER) - { - lobby.ResetReadyStates(); - } + // Every mutation below touches this lobby's Members, slot state, or ready state, + // so the whole field-update dispatch runs as one atomic operation under the + // per-lobby gate instead of each setter mutating state unguarded. + await lobby.RunExclusiveAsync(() => ApplyLobbyFieldUpdateAsync(lobby, SourceMember, field, data)); + } + } - if (field == ELobbyUpdateField.LOBBY_MAP) - { - if (data.ContainsKey("map") - && data.ContainsKey("map_path") - && data.ContainsKey("max_players") - ) - { - string? strMap = data["map"].GetString(); - string? strMapPath = data["map_path"].GetString(); - bool bOfficialMap = data["map_official"].GetBoolean(); - int maxPlayers = data["max_players"].GetInt32(); + } - if (strMap != null && strMapPath != null) - { - await using var db = await _dbFactory.CreateDbContextAsync(); - await lobby.UpdateMap(db, strMap, strMapPath, bOfficialMap, maxPlayers); - } - } - } - else if (field == ELobbyUpdateField.MY_SIDE) - { - if (data.ContainsKey("side") - && data.ContainsKey("start_pos") - ) - { - int side = data["side"].GetInt32(); - int start_pos = data["start_pos"].GetInt32(); + } + catch + { + return result; + } + } - await using var db = await _dbFactory.CreateDbContextAsync(); - await SourceMember.UpdateSide(db, side, start_pos); - } - } - else if (field == ELobbyUpdateField.MY_COLOR) - { - if (data.ContainsKey("color")) - { - int color = data["color"].GetInt32(); + return result; + } - await using var db = await _dbFactory.CreateDbContextAsync(); - await SourceMember.UpdateColor(db, color); - } - } - else if (field == ELobbyUpdateField.MY_START_POS) - { - if (data.ContainsKey("startpos")) - { - int startpos = data["startpos"].GetInt32(); - SourceMember.UpdateStartPos(startpos); - } - } - else if (field == ELobbyUpdateField.MY_TEAM) - { - if (data.ContainsKey("team")) - { - int team = data["team"].GetInt32(); - SourceMember.UpdateTeam(team); - } - } - else if (field == ELobbyUpdateField.LOBBY_STARTING_CASH) - { - if (data.ContainsKey("startingcash")) - { - UInt32 startingCash = data["startingcash"].GetUInt32(); + // Runs the per-field lobby update dispatch. Must only be called from inside + // lobby.RunExclusiveAsync: every branch here mutates this lobby's Members, slot + // state/fields, or ready state, and previously ran completely unguarded. + private async Task ApplyLobbyFieldUpdateAsync(Lobby lobby, LobbyMember SourceMember, ELobbyUpdateField field, Dictionary data) + { + // reset everyones ready states when anything changes (minus dummy actions) + if (field != ELobbyUpdateField.HOST_ACTION_FORCE_START + && field != ELobbyUpdateField.LOCAL_PLAYER_HAS_MAP + && field != ELobbyUpdateField.HOST_ACTION_KICK_USER) + { + lobby.ResetReadyStates(); + } - await using var db = await _dbFactory.CreateDbContextAsync(); - await lobby.UpdateStartingCash(db, startingCash); - } - } - else if (field == ELobbyUpdateField.LOBBY_LIMIT_SUPERWEAPONS) - { - if (data.ContainsKey("limit_superweapons")) - { - bool bLimitSuperweapons = data["limit_superweapons"].GetBoolean(); + if (field == ELobbyUpdateField.LOBBY_MAP) + { + if (data.ContainsKey("map") + && data.ContainsKey("map_path") + && data.ContainsKey("max_players") + ) + { + string? strMap = data["map"].GetString(); + string? strMapPath = data["map_path"].GetString(); + bool bOfficialMap = data["map_official"].GetBoolean(); + int maxPlayers = data["max_players"].GetInt32(); - await using var db = await _dbFactory.CreateDbContextAsync(); - await lobby.UpdateLimitSuperweapons(db, bLimitSuperweapons); - } - } - else if (field == ELobbyUpdateField.HOST_ACTION_FORCE_START) - { - // dummy action... just force everyone ready - lobby.ForceReady(); - } - else if (field == ELobbyUpdateField.LOCAL_PLAYER_HAS_MAP) - { - if (data.ContainsKey("has_map")) - { - bool bHasMap = data["has_map"].GetBoolean(); + if (strMap != null && strMapPath != null) + { + await using var db = await _dbFactory.CreateDbContextAsync(); + await lobby.UpdateMap(db, strMap, strMapPath, bOfficialMap, maxPlayers); + } + } + } + else if (field == ELobbyUpdateField.MY_SIDE) + { + if (data.ContainsKey("side") + && data.ContainsKey("start_pos") + ) + { + int side = data["side"].GetInt32(); + int start_pos = data["start_pos"].GetInt32(); - SourceMember.UpdateHasMap(bHasMap); - } - } - else if (field == ELobbyUpdateField.HOST_ACTION_KICK_USER) - { - if (data.ContainsKey("userid")) - { - // TODO: we should communicate the kick to the user... - Int64 KickedUserID = data["userid"].GetInt64(); + await using var db = await _dbFactory.CreateDbContextAsync(); + await SourceMember.UpdateSide(db, side, start_pos); + } + } + else if (field == ELobbyUpdateField.MY_COLOR) + { + if (data.ContainsKey("color")) + { + int color = data["color"].GetInt32(); - // the target must actually be in THIS lobby, otherwise a host could wipe the - // TURN credentials / lobby state of any arbitrary online player - if (lobby.GetMemberFromUserID(KickedUserID) != null) - { - _lobbyManager.LeaveSpecificLobby(KickedUserID, lobbyID); + await using var db = await _dbFactory.CreateDbContextAsync(); + await SourceMember.UpdateColor(db, color); + } + } + else if (field == ELobbyUpdateField.MY_START_POS) + { + if (data.ContainsKey("startpos")) + { + int startpos = data["startpos"].GetInt32(); + SourceMember.UpdateStartPos(startpos); + } + } + else if (field == ELobbyUpdateField.MY_TEAM) + { + if (data.ContainsKey("team")) + { + int team = data["team"].GetInt32(); + SourceMember.UpdateTeam(team); + } + } + else if (field == ELobbyUpdateField.LOBBY_STARTING_CASH) + { + if (data.ContainsKey("startingcash")) + { + UInt32 startingCash = data["startingcash"].GetUInt32(); - // cleanup TURN credentials - TURNCredentialManager.DeleteCredentialsForUser(KickedUserID); + await using var db = await _dbFactory.CreateDbContextAsync(); + await lobby.UpdateStartingCash(db, startingCash); + } + } + else if (field == ELobbyUpdateField.LOBBY_LIMIT_SUPERWEAPONS) + { + if (data.ContainsKey("limit_superweapons")) + { + bool bLimitSuperweapons = data["limit_superweapons"].GetBoolean(); - // clear our lobby ID - UserSession? sourceData = WebSocketManager.GetSessionFromUser(KickedUserID, EUserSessionType.GameClient); // user being kicked must be a game client + await using var db = await _dbFactory.CreateDbContextAsync(); + await lobby.UpdateLimitSuperweapons(db, bLimitSuperweapons); + } + } + else if (field == ELobbyUpdateField.HOST_ACTION_FORCE_START) + { + // dummy action... just force everyone ready + lobby.ForceReady(); + } + else if (field == ELobbyUpdateField.LOCAL_PLAYER_HAS_MAP) + { + if (data.ContainsKey("has_map")) + { + bool bHasMap = data["has_map"].GetBoolean(); - if (sourceData != null) - { - sourceData.UpdateSessionLobbyID(-1); - // NOTE: We dont update the match history match ID here, that is done by the match history service - } + SourceMember.UpdateHasMap(bHasMap); + } + } + else if (field == ELobbyUpdateField.HOST_ACTION_KICK_USER) + { + if (data.ContainsKey("userid")) + { + // TODO: we should communicate the kick to the user... + Int64 KickedUserID = data["userid"].GetInt64(); - // we have to manually send to the kicked user... they won't get the dirty lobby update anymore - await lobby.DirtyRetransmitToSingleMember(KickedUserID); - } - } - } - else if (field == ELobbyUpdateField.HOST_ACTION_SET_SLOT_STATE) - { - UInt16 slot_index = data["slot_index"].GetUInt16(); - EPlayerType slot_state = (EPlayerType)data["slot_state"].GetUInt16(); + // the target must actually be in THIS lobby, otherwise a host could wipe the + // TURN credentials / lobby state of any arbitrary online player + if (lobby.GetMemberFromUserID(KickedUserID) != null) + { + _lobbyManager.LeaveSpecificLobby(KickedUserID, lobby.LobbyID); - LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot_index); - if (TargetMember != null) - { - TargetMember.SetPlayerSlotState(slot_state); - } - } - else if (field == ELobbyUpdateField.AI_SIDE) - { - if (data.ContainsKey("slot") - && data.ContainsKey("side") - && data.ContainsKey("start_pos") - ) - { - int slot = data["slot"].GetInt32(); - int side = data["side"].GetInt32(); - int start_pos = data["start_pos"].GetInt32(); + // cleanup TURN credentials + TURNCredentialManager.DeleteCredentialsForUser(KickedUserID); - LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot); - if (TargetMember != null) - { - if (TargetMember.IsAI()) - { - await using var db = await _dbFactory.CreateDbContextAsync(); - await TargetMember.UpdateSide(db, side, start_pos); - } - } - } - } - else if (field == ELobbyUpdateField.AI_COLOR) - { - if (data.ContainsKey("slot") - && data.ContainsKey("color")) - { - int slot = data["slot"].GetInt32(); - int color = data["color"].GetInt32(); + // clear our lobby ID + UserSession? sourceData = WebSocketManager.GetSessionFromUser(KickedUserID, EUserSessionType.GameClient); // user being kicked must be a game client - LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot); - if (TargetMember != null) - { - if (TargetMember.IsAI()) - { - await using var db = await _dbFactory.CreateDbContextAsync(); - await TargetMember.UpdateColor(db, color); - } - } - } - } - else if (field == ELobbyUpdateField.AI_TEAM) - { - if (data.ContainsKey("slot") - && data.ContainsKey("team")) - { - int slot = data["slot"].GetInt32(); - int team = data["team"].GetInt32(); + if (sourceData != null) + { + sourceData.UpdateSessionLobbyID(-1); + // NOTE: We dont update the match history match ID here, that is done by the match history service + } - LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot); - if (TargetMember != null) - { - if (TargetMember.IsAI()) - { - TargetMember.UpdateTeam(team); - } - } - } - } - else if (field == ELobbyUpdateField.AI_START_POS) - { - if (data.ContainsKey("slot") - && data.ContainsKey("start_pos")) - { + // we have to manually send to the kicked user... they won't get the dirty lobby update anymore + await lobby.DirtyRetransmitToSingleMember(KickedUserID); + } + } + } + else if (field == ELobbyUpdateField.HOST_ACTION_SET_SLOT_STATE) + { + UInt16 slot_index = data["slot_index"].GetUInt16(); + EPlayerType slot_state = (EPlayerType)data["slot_state"].GetUInt16(); - int slot = data["slot"].GetInt32(); - int start_pos = data["start_pos"].GetInt32(); + LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot_index); + if (TargetMember != null) + { + TargetMember.SetPlayerSlotState(slot_state); + } + } + else if (field == ELobbyUpdateField.AI_SIDE) + { + if (data.ContainsKey("slot") + && data.ContainsKey("side") + && data.ContainsKey("start_pos") + ) + { + int slot = data["slot"].GetInt32(); + int side = data["side"].GetInt32(); + int start_pos = data["start_pos"].GetInt32(); - LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot); - if (TargetMember != null) - { - if (TargetMember.IsAI()) - { - TargetMember.UpdateStartPos(start_pos); - } - } - } - } - else if (field == ELobbyUpdateField.MAX_CAMERA_HEIGHT) - { - if (data.ContainsKey("max_camera_height")) - { - UInt16 maxCameraHeight = data["max_camera_height"].GetUInt16(); - lobby.UpdateMaxCameraHeight(maxCameraHeight); - } - } - else if (field == ELobbyUpdateField.JOINABILITY) - { - ELobbyJoinability newLobbyJoinability = (ELobbyJoinability)data["joinability"].GetInt32(); - lobby.UpdateJoinability(newLobbyJoinability); - } - else if (field == ELobbyUpdateField.HOST_ACTION_BULK_SLOT_UPDATE) - { - if (data.ContainsKey("slots")) - { - await using var db = await _dbFactory.CreateDbContextAsync(); - foreach (JsonElement slotEntry in data["slots"].EnumerateArray()) - { - try - { - if (!slotEntry.TryGetProperty("slot_index", out var slotIndexProp) || - !slotEntry.TryGetProperty("side", out var sideProp) || - !slotEntry.TryGetProperty("color", out var colorProp) || - !slotEntry.TryGetProperty("start_pos", out var startPosProp) || - !slotEntry.TryGetProperty("team", out var teamProp)) - { - continue; - } - - int slotIndex = slotIndexProp.GetInt32(); - int side = sideProp.GetInt32(); - int color = colorProp.GetInt32(); - int start_pos = startPosProp.GetInt32(); - int team = teamProp.GetInt32(); - - LobbyMember? TargetMember = lobby.GetMemberFromSlot(slotIndex); - if (TargetMember != null) - { - await TargetMember.UpdateSide(db, side, start_pos); - await TargetMember.UpdateColor(db, color); - TargetMember.UpdateStartPos(start_pos); - TargetMember.UpdateTeam(team); - } - } - catch - { - continue; - } - } - lobby.DirtyRetransmit(); - } - } - } - } + LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot); + if (TargetMember != null) + { + if (TargetMember.IsAI()) + { + await using var db = await _dbFactory.CreateDbContextAsync(); + await TargetMember.UpdateSide(db, side, start_pos); + } + } + } + } + else if (field == ELobbyUpdateField.AI_COLOR) + { + if (data.ContainsKey("slot") + && data.ContainsKey("color")) + { + int slot = data["slot"].GetInt32(); + int color = data["color"].GetInt32(); - } + LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot); + if (TargetMember != null) + { + if (TargetMember.IsAI()) + { + await using var db = await _dbFactory.CreateDbContextAsync(); + await TargetMember.UpdateColor(db, color); + } + } + } + } + else if (field == ELobbyUpdateField.AI_TEAM) + { + if (data.ContainsKey("slot") + && data.ContainsKey("team")) + { + int slot = data["slot"].GetInt32(); + int team = data["team"].GetInt32(); + LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot); + if (TargetMember != null) + { + if (TargetMember.IsAI()) + { + TargetMember.UpdateTeam(team); + } + } } - catch + } + else if (field == ELobbyUpdateField.AI_START_POS) + { + if (data.ContainsKey("slot") + && data.ContainsKey("start_pos")) { - return result; + + int slot = data["slot"].GetInt32(); + int start_pos = data["start_pos"].GetInt32(); + + LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot); + if (TargetMember != null) + { + if (TargetMember.IsAI()) + { + TargetMember.UpdateStartPos(start_pos); + } + } } } + else if (field == ELobbyUpdateField.MAX_CAMERA_HEIGHT) + { + if (data.ContainsKey("max_camera_height")) + { + UInt16 maxCameraHeight = data["max_camera_height"].GetUInt16(); + lobby.UpdateMaxCameraHeight(maxCameraHeight); + } + } + else if (field == ELobbyUpdateField.JOINABILITY) + { + ELobbyJoinability newLobbyJoinability = (ELobbyJoinability)data["joinability"].GetInt32(); + lobby.UpdateJoinability(newLobbyJoinability); + } + else if (field == ELobbyUpdateField.HOST_ACTION_BULK_SLOT_UPDATE) + { + if (data.ContainsKey("slots")) + { + await using var db = await _dbFactory.CreateDbContextAsync(); + foreach (JsonElement slotEntry in data["slots"].EnumerateArray()) + { + try + { + if (!slotEntry.TryGetProperty("slot_index", out var slotIndexProp) || + !slotEntry.TryGetProperty("side", out var sideProp) || + !slotEntry.TryGetProperty("color", out var colorProp) || + !slotEntry.TryGetProperty("start_pos", out var startPosProp) || + !slotEntry.TryGetProperty("team", out var teamProp)) + { + continue; + } - return result; + int slotIndex = slotIndexProp.GetInt32(); + int side = sideProp.GetInt32(); + int color = colorProp.GetInt32(); + int start_pos = startPosProp.GetInt32(); + int team = teamProp.GetInt32(); + + LobbyMember? TargetMember = lobby.GetMemberFromSlot(slotIndex); + if (TargetMember != null) + { + await TargetMember.UpdateSide(db, side, start_pos); + await TargetMember.UpdateColor(db, color); + TargetMember.UpdateStartPos(start_pos); + TargetMember.UpdateTeam(team); + } + } + catch + { + continue; + } + } + lobby.DirtyRetransmit(); + } + } } [HttpPut("{lobbyID}")] diff --git a/GenOnlineService/Controllers/WebSocket/WebSocketController.cs b/GenOnlineService/Controllers/WebSocket/WebSocketController.cs index 7abfa74..54fdcc0 100644 --- a/GenOnlineService/Controllers/WebSocket/WebSocketController.cs +++ b/GenOnlineService/Controllers/WebSocket/WebSocketController.cs @@ -673,12 +673,19 @@ private async Task ProcessWSMessage(UserWebSocketInstance sourceWS, UserSession Lobby? lobby = _lobbyManager.GetLobby(sourceUserSession.currentLobbyID); if (lobby != null) { - LobbyMember? member = lobby.GetMemberFromUserID(sourceUserSession.m_UserID); - - if (member != null) + // Ready state is part of the lobby's mutable state, so it goes through the + // same per-lobby gate as slot/member mutations. + await lobby.RunExclusiveAsync(() => { - member.SetReadyState(bReady); - } + LobbyMember? member = lobby.GetMemberFromUserID(sourceUserSession.m_UserID); + + if (member != null) + { + member.SetReadyState(bReady); + } + + return Task.CompletedTask; + }); } } } @@ -1030,7 +1037,7 @@ private async Task ProcessWSMessage(UserWebSocketInstance sourceWS, UserSession lobbyInfo.CloseOpenSlots(); // mark lobby as in progress of full mesh connectivity checks - lobbyInfo.StartFullMeshConnectivityCheck(); + await lobbyInfo.StartFullMeshConnectivityCheck(); // start full mesh connectivity checks lobbyInfo.SendFullMeshConnectivityCheckRequestToMembers(); diff --git a/GenOnlineService/LobbyManager.cs b/GenOnlineService/LobbyManager.cs index ef1d4f7..3b455c1 100644 --- a/GenOnlineService/LobbyManager.cs +++ b/GenOnlineService/LobbyManager.cs @@ -105,8 +105,45 @@ public class Lobby [JsonIgnore] public Int64 TimeStartFullMeshChecks { get; private set; } = -1; - private readonly object m_FullMeshCheckLock = new(); + // Single per-lobby exclusive gate for every mutation of this lobby's Members/Owner, slot + // state/fields, ready state, and mesh-check state. Everything that used to be split across + // g_SlotLock (slots) and m_FullMeshCheckLock (mesh-check fields) now goes through this one + // SemaphoreSlim, so there is exactly one lock to reason about and no lock-ordering hazard + // between the two. It is NOT reentrant: a callback passed to RunExclusiveAsync must never + // call back into RunExclusiveAsync on this same lobby, or it will deadlock - callers that are + // already inside a callback (e.g. DoHostMigration, called from RemoveMember's callback) must + // mutate state directly instead. Sending a websocket message from inside a callback is safe: + // QueueWebsocketSend only enqueues bytes onto an in-memory bounded channel (a non-blocking + // TryWrite) - it never performs real network I/O itself, so it cannot block while the gate is + // held. + private readonly SemaphoreSlim m_LobbyGate = new SemaphoreSlim(1, 1); + public async Task RunExclusiveAsync(Func action) + { + await m_LobbyGate.WaitAsync(); + try + { + await action(); + } + finally + { + m_LobbyGate.Release(); + } + } + + public async Task RunExclusiveAsync(Func> action) + { + await m_LobbyGate.WaitAsync(); + try + { + return await action(); + } + finally + { + m_LobbyGate.Release(); + } + } + [JsonIgnore] public bool? LastFullMeshConnectivityCheckOutcome { get; private set; } = null; @@ -188,16 +225,17 @@ public async Task RegisterProbeResponse_Malformed_Type2(Int64 userID) } // End AC Probes - public void StartFullMeshConnectivityCheck() + public async Task StartFullMeshConnectivityCheck() { - lock (m_FullMeshCheckLock) + await RunExclusiveAsync(() => { FullMeshCheckID = Interlocked.Increment(ref s_NextFullMeshCheckID); FullMeshCheckAttempt = 1; m_TimeToRetryFullMeshChecks = -1; LastFullMeshConnectivityCheckOutcome = null; BeginFullMeshConnectivityCheckAttempt(); - } + return Task.CompletedTask; + }); } private void BeginFullMeshConnectivityCheckAttempt() @@ -298,9 +336,9 @@ private void SendStartSignallingToMember(LobbyMember recipient, LobbyMember peer } } - public Task StoreFullMeshConnectivityResponse(Int64 sourceUser, WebSocketMessage_FullMeshConnectivityCheckResponseFromUser response) + public async Task StoreFullMeshConnectivityResponse(Int64 sourceUser, WebSocketMessage_FullMeshConnectivityCheckResponseFromUser response) { - lock (m_FullMeshCheckLock) + await RunExclusiveAsync(() => { bool bLegacyResponse = FullMeshCheckProtocol.IsLegacyResponse(response); bool bMatchesCurrentAttempt = FullMeshCheckProtocol.MatchesCurrentAttempt( @@ -320,21 +358,20 @@ public Task StoreFullMeshConnectivityResponse(Int64 sourceUser, WebSocketMessage } ProcessPendingFullMeshConnectivityChecksInternal(); - } - - return Task.CompletedTask; + return Task.CompletedTask; + }); } - public Task ProcessPendingFullMeshConnectivityChecks() + public async Task ProcessPendingFullMeshConnectivityChecks() { - lock (m_FullMeshCheckLock) + await RunExclusiveAsync(() => { ProcessPendingFullMeshConnectivityChecksInternal(); - } - - return Task.CompletedTask; + return Task.CompletedTask; + }); } + // Must only be called while holding m_LobbyGate (via RunExclusiveAsync). private void ProcessPendingFullMeshConnectivityChecksInternal() { if (!PendingFullMeshConnectivityChecks) @@ -784,7 +821,10 @@ public Lobby(Int64 lobby_id, UserSession owner, string name, ELobbyState state, public event Action? OnLobbyNeedsDestroyed; - public async Task OnAfterPlayerLeft(Int64 leavingUserID) + // Must only be called while holding m_LobbyGate (from within RemoveMember's callback): it reads + // and mutates Members/Owner and must observe RemoveMember's slot clear atomically with any + // concurrent join/leave/migration. + private void OnAfterPlayerLeftLocked(Int64 leavingUserID) { // NOTE: By the time this is called, the member is no longer in the members list bool bNeedsHostMigrate = Owner == leavingUserID; @@ -866,7 +906,9 @@ public void CloseOpenSlots() DirtyRetransmit(); } - public void DoHostMigration() + // Must only be called while holding m_LobbyGate (currently only true from + // OnAfterPlayerLeftLocked, itself only called from within RemoveMember's callback). + private void DoHostMigration() { Int64 oldOwner = Owner; @@ -993,12 +1035,11 @@ public async Task Tick() } } - private readonly SemaphoreSlim g_SlotLock = new SemaphoreSlim(1, 1); public async Task AddMember(UserSession playerSession, string strDisplayName, UInt16 userPreferredPort, bool bHasMap, UserLobbyPreferences lobbyPrefs) { - // NOTE: AddMember is called async, so timing + slot determination could result in players being inserted in the same slot - await g_SlotLock.WaitAsync(); - try + // NOTE: AddMember runs inside the per-lobby gate, so timing + slot determination can no + // longer result in two concurrent joins picking the same slot. + return await RunExclusiveAsync(async () => { if (State != ELobbyState.GAME_SETUP) { @@ -1191,11 +1232,7 @@ public async Task AddMember(UserSession playerSession, string strDisplayNa Console.WriteLine("User {0} joined lobby {1}: {2} (Slot was {3})", playerSession.m_UserID, LobbyID, true, slotIndex); return true; - } - finally - { - g_SlotLock.Release(); - } + }); } public void SendPeerTeardownToDepartingMember(LobbyMember departingMember) @@ -1232,24 +1269,31 @@ public void SendPeerTeardownToDepartingMember(LobbyMember departingMember) public async Task RemoveMember(LobbyMember member) { // Matchmaking cancellation and the client's explicit lobby leave can arrive concurrently. - // Claim the slot once so teardown, host migration, and destruction callbacks stay idempotent. - await g_SlotLock.WaitAsync(); - try + // Claim the slot once, and run host migration in the same critical section as the slot + // clear, so teardown, host migration, and destruction callbacks stay atomic and idempotent + // (previously DoHostMigration ran after this gate was released, so it could interleave + // with a concurrent AddMember/RemoveMember and observe a half-updated Members/Owner). + bool bRemoved = await RunExclusiveAsync(() => { if (member.SlotIndex < 0 || member.SlotIndex >= Members.Length || !ReferenceEquals(Members[member.SlotIndex], member)) { - return; + return Task.FromResult(false); } LobbyMember placeholderMember = new LobbyMember(this, null, -1, String.Empty, String.Empty, 0, -1, -1, -1, EPlayerType.SLOT_OPEN, member.SlotIndex, true); Members[member.SlotIndex] = placeholderMember; TimeMemberLeft[member.UserID] = DateTime.UtcNow; - } - finally + + OnAfterPlayerLeftLocked(member.UserID); + + return Task.FromResult(true); + }); + + if (!bRemoved) { - g_SlotLock.Release(); + return; } // TODO_LOBBY: Optimize this @@ -1296,8 +1340,6 @@ public async Task RemoveMember(LobbyMember member) // END NETWORK DISCONNECT } - await OnAfterPlayerLeft(UserID); - DirtyRetransmitLobbyList(); } @@ -1494,15 +1536,11 @@ public bool HadAIAtStart() public async Task UpdateState(ELobbyState state) { - await g_SlotLock.WaitAsync(); - try + await RunExclusiveAsync(() => { State = state; - } - finally - { - g_SlotLock.Release(); - } + return Task.CompletedTask; + }); // if start, init our AC probe if (state == ELobbyState.INGAME) diff --git a/GenOnlineService/MatchmakingManager.cs b/GenOnlineService/MatchmakingManager.cs index 3769961..2a645d2 100644 --- a/GenOnlineService/MatchmakingManager.cs +++ b/GenOnlineService/MatchmakingManager.cs @@ -907,7 +907,7 @@ private void QueueSetupProgress(int timeoutMSec, int countdownMSec = 0) private async Task TriggerFullMeshConnectivityChecks(Lobby lobby) { - lobby.StartFullMeshConnectivityCheck(); + await lobby.StartFullMeshConnectivityCheck(); lock (m_StateLock) { // Publish the state transition before any notification work. If a send fails, the From 551461c93864b632b0f0752d2ed3bd06200d97c2 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 26 Sep 2026 23:10:46 +0200 Subject: [PATCH 04/12] fix(lobby): Reject out-of-range slot_state values on HOST_ACTION_SET_SLOT_STATE --- .../Controllers/Lobby/LobbyController.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/GenOnlineService/Controllers/Lobby/LobbyController.cs b/GenOnlineService/Controllers/Lobby/LobbyController.cs index 4e73922..ce29f42 100644 --- a/GenOnlineService/Controllers/Lobby/LobbyController.cs +++ b/GenOnlineService/Controllers/Lobby/LobbyController.cs @@ -483,6 +483,15 @@ public async Task Post(Int64 lobbyID) return result; } + // Rejects any wire value that isn't a real EPlayerType member, so a malformed or + // out-of-range slot_state from HOST_ACTION_SET_SLOT_STATE can't be cast into an enum value + // the rest of the lobby code (SetPlayerSlotState, IsAI, serialization, ...) never expects. + internal static bool TryParseSlotState(UInt16 rawValue, out EPlayerType slotState) + { + slotState = (EPlayerType)rawValue; + return Enum.IsDefined(typeof(EPlayerType), slotState); + } + // Runs the per-field lobby update dispatch. Must only be called from inside // lobby.RunExclusiveAsync: every branch here mutates this lobby's Members, slot // state/fields, or ready state, and previously ran completely unguarded. @@ -621,7 +630,12 @@ private async Task ApplyLobbyFieldUpdateAsync(Lobby lobby, LobbyMember SourceMem else if (field == ELobbyUpdateField.HOST_ACTION_SET_SLOT_STATE) { UInt16 slot_index = data["slot_index"].GetUInt16(); - EPlayerType slot_state = (EPlayerType)data["slot_state"].GetUInt16(); + + if (!TryParseSlotState(data["slot_state"].GetUInt16(), out EPlayerType slot_state)) + { + Response.StatusCode = (int)HttpStatusCode.BadRequest; + return; + } LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot_index); if (TargetMember != null) From 5d61ffc4c5cff973b99530a266a97cf9d0b15c73 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sun, 27 Sep 2026 02:44:13 +0200 Subject: [PATCH 05/12] fix(session): Make the reconnect grace period configurable --- GenOnlineService/Constants.cs | 21 +++++++++++++++++++-- GenOnlineService/appsettings.json | 3 ++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/GenOnlineService/Constants.cs b/GenOnlineService/Constants.cs index 3316def..b5a1cf5 100644 --- a/GenOnlineService/Constants.cs +++ b/GenOnlineService/Constants.cs @@ -1137,6 +1137,19 @@ public SharedUserData(Int64 ownerID, UserSocialContainer socialContainer, string } } + // Core:reconnect_grace_period_ms in appsettings.json, read on use. Mirrors the pattern used by + // FullMeshCheckSettings in LobbyManager.cs. + internal static class UserSessionSettings + { + internal static Int64 ReconnectGracePeriodMS => Get("reconnect_grace_period_ms", 30000); + + private static Int64 Get(string key, Int64 defaultValue) + { + Int64 value = Program.g_Config?.GetSection("Core").GetValue(key, defaultValue) ?? defaultValue; + return value > 0 ? value : defaultValue; + } + } + public class UserSession { public Int64 m_UserID = -1; @@ -1292,8 +1305,12 @@ public async Task CloseWebsocket(WebSocketCloseStatus rea public bool NeedsCleanup() { - const Int64 timeBeforeConsideredAbandoned = 30000; // 5 minutes - return Environment.TickCount64 - m_timeAbandoned >= timeBeforeConsideredAbandoned; + // Grace period an abandoned (no live websocket) session gets before it's torn down, + // letting a brief disconnect reconnect instead of losing the slot. Configurable via + // Core:reconnect_grace_period_ms; defaults to 30 seconds, unchanged from before this + // was configurable (the old comment here said "5 minutes", which was wrong - 30000 is + // milliseconds, i.e. 30 seconds). + return Environment.TickCount64 - m_timeAbandoned >= UserSessionSettings.ReconnectGracePeriodMS; } private bool m_bSubscribedToRealtimeSocialupdates = false; diff --git a/GenOnlineService/appsettings.json b/GenOnlineService/appsettings.json index 29dc3bd..3e791a3 100644 --- a/GenOnlineService/appsettings.json +++ b/GenOnlineService/appsettings.json @@ -38,7 +38,8 @@ "full_mesh_check_attempt_window_ms": 8000, "full_mesh_check_snapshot_interval_ms": 1000, "full_mesh_check_retry_delay_ms": 3000, - "full_mesh_check_max_attempts": 2 + "full_mesh_check_max_attempts": 2, + "reconnect_grace_period_ms": 30000 }, "TURN": { "key": null, From 83017dae3f0b84e0a12a2bb5341e2b1678d46984 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 26 Sep 2026 23:35:01 +0200 Subject: [PATCH 06/12] fix(lobby): Await LeaveAnyLobby in the lobby delete handler --- GenOnlineService/Controllers/Lobby/LobbyController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GenOnlineService/Controllers/Lobby/LobbyController.cs b/GenOnlineService/Controllers/Lobby/LobbyController.cs index ce29f42..be0d141 100644 --- a/GenOnlineService/Controllers/Lobby/LobbyController.cs +++ b/GenOnlineService/Controllers/Lobby/LobbyController.cs @@ -246,7 +246,7 @@ public async Task Delete(Int64 lobbyID) } Console.WriteLine("[Source 1] User {0} Leave Any Lobby", user_id); - _lobbyManager.LeaveAnyLobby(user_id); + await _lobbyManager.LeaveAnyLobby(user_id); // cleanup TURN credentials TURNCredentialManager.DeleteCredentialsForUser(user_id); From f04165f53c55b496cdad4acbe39d71eef39dd2c4 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 26 Sep 2026 23:35:49 +0200 Subject: [PATCH 07/12] fix(lobby): Reject unknown field values on the lobby update endpoint --- .../Controllers/Lobby/LobbyController.cs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/GenOnlineService/Controllers/Lobby/LobbyController.cs b/GenOnlineService/Controllers/Lobby/LobbyController.cs index be0d141..d061e33 100644 --- a/GenOnlineService/Controllers/Lobby/LobbyController.cs +++ b/GenOnlineService/Controllers/Lobby/LobbyController.cs @@ -448,8 +448,11 @@ public async Task Post(Int64 lobbyID) return result; } - // TODO: Safety - ELobbyUpdateField field = (ELobbyUpdateField)data["field"].GetInt32(); + if (!TryParseLobbyUpdateField(data["field"].GetInt32(), out ELobbyUpdateField field)) + { + Response.StatusCode = (int)HttpStatusCode.BadRequest; + return result; + } // check permissions ELobbyUpdatePermissions updatePerms = g_dictLobbyUpdatePermissionsTable[field]; @@ -492,6 +495,16 @@ internal static bool TryParseSlotState(UInt16 rawValue, out EPlayerType slotStat return Enum.IsDefined(typeof(EPlayerType), slotState); } + // Rejects any wire value that isn't a real ELobbyUpdateField member, so an unknown/malformed + // field can't be cast into an enum value that then gets indexed straight into + // g_dictLobbyUpdatePermissionsTable (which previously threw KeyNotFoundException, silently + // swallowed by Post's outer catch, for anything out of range). + internal static bool TryParseLobbyUpdateField(int rawValue, out ELobbyUpdateField field) + { + field = (ELobbyUpdateField)rawValue; + return Enum.IsDefined(typeof(ELobbyUpdateField), field); + } + // Runs the per-field lobby update dispatch. Must only be called from inside // lobby.RunExclusiveAsync: every branch here mutates this lobby's Members, slot // state/fields, or ready state, and previously ran completely unguarded. From a433e54afcb22b8931e2b66689d5c7d5257b84e1 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 26 Sep 2026 23:36:14 +0200 Subject: [PATCH 08/12] fix(lobby): Route CloseOpenSlots through the per-lobby gate --- .../WebSocket/WebSocketController.cs | 6 ++++-- GenOnlineService/LobbyManager.cs | 21 +++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/GenOnlineService/Controllers/WebSocket/WebSocketController.cs b/GenOnlineService/Controllers/WebSocket/WebSocketController.cs index 54fdcc0..0adf62c 100644 --- a/GenOnlineService/Controllers/WebSocket/WebSocketController.cs +++ b/GenOnlineService/Controllers/WebSocket/WebSocketController.cs @@ -961,7 +961,7 @@ await lobby.RunExclusiveAsync(() => } // lock slots - lobbyInfo.CloseOpenSlots(); + await lobbyInfo.CloseOpenSlots(); } else if (msgID == EWebSocketMessageID.START_GAME) { @@ -1034,7 +1034,9 @@ await lobby.RunExclusiveAsync(() => } // lock slots (more people joining when we're already doing connectivity checks won't help the situation) - lobbyInfo.CloseOpenSlots(); + // Awaited separately from StartFullMeshConnectivityCheck below: each is its own + // gated operation on the lobby, so neither ever nests inside the other's gate use. + await lobbyInfo.CloseOpenSlots(); // mark lobby as in progress of full mesh connectivity checks await lobbyInfo.StartFullMeshConnectivityCheck(); diff --git a/GenOnlineService/LobbyManager.cs b/GenOnlineService/LobbyManager.cs index 3b455c1..7a924b5 100644 --- a/GenOnlineService/LobbyManager.cs +++ b/GenOnlineService/LobbyManager.cs @@ -893,17 +893,26 @@ public async Task FinalizeACChecks() } } - public void CloseOpenSlots() + // Runs under the per-lobby gate: this mutates slot state and previously ran unguarded, + // racing against AddMember/RemoveMember/other slot updates. Sequential with (never nested + // inside) StartFullMeshConnectivityCheck's own gate use - callers await this first and then + // await StartFullMeshConnectivityCheck as a separate gated operation, so the gate is never + // re-entered. + public async Task CloseOpenSlots() { - foreach (LobbyMember member in Members) + await RunExclusiveAsync(() => { - if (member.SlotState == EPlayerType.SLOT_OPEN) + foreach (LobbyMember member in Members) { - member.SetPlayerSlotState(EPlayerType.SLOT_CLOSED); + if (member.SlotState == EPlayerType.SLOT_OPEN) + { + member.SetPlayerSlotState(EPlayerType.SLOT_CLOSED); + } } - } - DirtyRetransmit(); + DirtyRetransmit(); + return Task.CompletedTask; + }); } // Must only be called while holding m_LobbyGate (currently only true from From 0526e5226f45c7fe5c8b17bcfdc8481ebcba3ca2 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sun, 27 Sep 2026 00:16:03 +0200 Subject: [PATCH 09/12] fix(lobby): Run the actual kick removal after the gate is released --- .../Controllers/Lobby/LobbyController.cs | 66 +++++++++++++------ 1 file changed, 45 insertions(+), 21 deletions(-) diff --git a/GenOnlineService/Controllers/Lobby/LobbyController.cs b/GenOnlineService/Controllers/Lobby/LobbyController.cs index d061e33..62fb454 100644 --- a/GenOnlineService/Controllers/Lobby/LobbyController.cs +++ b/GenOnlineService/Controllers/Lobby/LobbyController.cs @@ -470,7 +470,16 @@ public async Task Post(Int64 lobbyID) // Every mutation below touches this lobby's Members, slot state, or ready state, // so the whole field-update dispatch runs as one atomic operation under the // per-lobby gate instead of each setter mutating state unguarded. - await lobby.RunExclusiveAsync(() => ApplyLobbyFieldUpdateAsync(lobby, SourceMember, field, data)); + // A kick can't finish the actual removal in here: RemoveMember acquires this + // same lobby's gate, which is not reentrant, so ApplyLobbyFieldUpdateAsync only + // validates the target and hands the user ID back for PerformKickAsync to + // process once the gate below has been released. + Int64? kickedUserID = await lobby.RunExclusiveAsync(() => ApplyLobbyFieldUpdateAsync(lobby, SourceMember, field, data)); + + if (kickedUserID.HasValue) + { + await PerformKickAsync(lobby, kickedUserID.Value); + } } } @@ -508,7 +517,10 @@ internal static bool TryParseLobbyUpdateField(int rawValue, out ELobbyUpdateFiel // Runs the per-field lobby update dispatch. Must only be called from inside // lobby.RunExclusiveAsync: every branch here mutates this lobby's Members, slot // state/fields, or ready state, and previously ran completely unguarded. - private async Task ApplyLobbyFieldUpdateAsync(Lobby lobby, LobbyMember SourceMember, ELobbyUpdateField field, Dictionary data) + // Returns the kicked user's ID when HOST_ACTION_KICK_USER validated a real target, so the + // caller can run the actual removal after releasing the gate; null for every other field + // (including a kick request with no valid target). + private async Task ApplyLobbyFieldUpdateAsync(Lobby lobby, LobbyMember SourceMember, ELobbyUpdateField field, Dictionary data) { // reset everyones ready states when anything changes (minus dummy actions) if (field != ELobbyUpdateField.HOST_ACTION_FORCE_START @@ -614,29 +626,15 @@ private async Task ApplyLobbyFieldUpdateAsync(Lobby lobby, LobbyMember SourceMem { if (data.ContainsKey("userid")) { - // TODO: we should communicate the kick to the user... Int64 KickedUserID = data["userid"].GetInt64(); // the target must actually be in THIS lobby, otherwise a host could wipe the - // TURN credentials / lobby state of any arbitrary online player + // TURN credentials / lobby state of any arbitrary online player. Checked here, + // under the gate, so it can't race a concurrent leave/kick of the same target; + // the actual removal runs after the gate is released (see PerformKickAsync). if (lobby.GetMemberFromUserID(KickedUserID) != null) { - _lobbyManager.LeaveSpecificLobby(KickedUserID, lobby.LobbyID); - - // cleanup TURN credentials - TURNCredentialManager.DeleteCredentialsForUser(KickedUserID); - - // clear our lobby ID - UserSession? sourceData = WebSocketManager.GetSessionFromUser(KickedUserID, EUserSessionType.GameClient); // user being kicked must be a game client - - if (sourceData != null) - { - sourceData.UpdateSessionLobbyID(-1); - // NOTE: We dont update the match history match ID here, that is done by the match history service - } - - // we have to manually send to the kicked user... they won't get the dirty lobby update anymore - await lobby.DirtyRetransmitToSingleMember(KickedUserID); + return KickedUserID; } } } @@ -647,7 +645,7 @@ private async Task ApplyLobbyFieldUpdateAsync(Lobby lobby, LobbyMember SourceMem if (!TryParseSlotState(data["slot_state"].GetUInt16(), out EPlayerType slot_state)) { Response.StatusCode = (int)HttpStatusCode.BadRequest; - return; + return null; } LobbyMember? TargetMember = lobby.GetMemberFromSlot(slot_index); @@ -788,6 +786,32 @@ private async Task ApplyLobbyFieldUpdateAsync(Lobby lobby, LobbyMember SourceMem lobby.DirtyRetransmit(); } } + + return null; + } + + // Runs the parts of a kick that RemoveMember itself performs (leaving the lobby, which + // acquires this same lobby's gate) plus TURN/session cleanup - all after + // ApplyLobbyFieldUpdateAsync's gate has been released, never from inside it. + private async Task PerformKickAsync(Lobby lobby, Int64 kickedUserID) + { + // TODO: we should communicate the kick to the user... + await _lobbyManager.LeaveSpecificLobby(kickedUserID, lobby.LobbyID); + + // cleanup TURN credentials + TURNCredentialManager.DeleteCredentialsForUser(kickedUserID); + + // clear our lobby ID + UserSession? sourceData = WebSocketManager.GetSessionFromUser(kickedUserID, EUserSessionType.GameClient); // user being kicked must be a game client + + if (sourceData != null) + { + sourceData.UpdateSessionLobbyID(-1); + // NOTE: We dont update the match history match ID here, that is done by the match history service + } + + // we have to manually send to the kicked user... they won't get the dirty lobby update anymore + await lobby.DirtyRetransmitToSingleMember(kickedUserID); } [HttpPut("{lobbyID}")] From c6531b064b79706cf9b05dc8da11a47190b17a2e Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sun, 27 Sep 2026 00:16:24 +0200 Subject: [PATCH 10/12] fix(lobby): Add a debug-only reentrancy guard to RunExclusiveAsync --- GenOnlineService/LobbyManager.cs | 37 ++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/GenOnlineService/LobbyManager.cs b/GenOnlineService/LobbyManager.cs index 7a924b5..00d3d83 100644 --- a/GenOnlineService/LobbyManager.cs +++ b/GenOnlineService/LobbyManager.cs @@ -118,28 +118,65 @@ public class Lobby // held. private readonly SemaphoreSlim m_LobbyGate = new SemaphoreSlim(1, 1); +#if DEBUG + // Debug-only reentrancy guard. AsyncLocal flows through the awaited async chain (and into + // fire-and-forget children too, since ExecutionContext is captured when they're created), so + // a callback that calls back into RunExclusiveAsync on this same lobby - the exact bug class + // HOST_ACTION_KICK_USER hit - throws immediately here instead of deadlocking (if awaited) or + // silently racing (if not). Cheap enough to leave on for tests; compiled out of Release. + private readonly AsyncLocal m_bGateHeldInThisFlow = new(); +#endif + public async Task RunExclusiveAsync(Func action) { +#if DEBUG + if (m_bGateHeldInThisFlow.Value) + { + throw new InvalidOperationException( + $"Lobby {LobbyID}: RunExclusiveAsync was re-entered on the same lobby within the same async flow. " + + "This gate is not reentrant and a nested call would deadlock (or race, if not awaited) in Release."); + } +#endif await m_LobbyGate.WaitAsync(); +#if DEBUG + m_bGateHeldInThisFlow.Value = true; +#endif try { await action(); } finally { +#if DEBUG + m_bGateHeldInThisFlow.Value = false; +#endif m_LobbyGate.Release(); } } public async Task RunExclusiveAsync(Func> action) { +#if DEBUG + if (m_bGateHeldInThisFlow.Value) + { + throw new InvalidOperationException( + $"Lobby {LobbyID}: RunExclusiveAsync was re-entered on the same lobby within the same async flow. " + + "This gate is not reentrant and a nested call would deadlock (or race, if not awaited) in Release."); + } +#endif await m_LobbyGate.WaitAsync(); +#if DEBUG + m_bGateHeldInThisFlow.Value = true; +#endif try { return await action(); } finally { +#if DEBUG + m_bGateHeldInThisFlow.Value = false; +#endif m_LobbyGate.Release(); } } From a1c47217d3416074fae9e1a2f1e51d718194ac20 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sun, 27 Sep 2026 00:40:32 +0200 Subject: [PATCH 11/12] fix(session): Re-check an abandoned session right before clearing it --- GenOnlineService/Constants.cs | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/GenOnlineService/Constants.cs b/GenOnlineService/Constants.cs index b5a1cf5..95c1b80 100644 --- a/GenOnlineService/Constants.cs +++ b/GenOnlineService/Constants.cs @@ -549,6 +549,19 @@ public static int GetNumberOfUsersOnline() return numSessions; } + // Pure re-check used by CheckForTimeouts right before actually clearing a snapshotted + // abandoned+expired entry: refuses unless it's still the SAME session object (a reconnect in + // between would have registered a new one) and it is STILL abandoned and expired. + internal static bool ShouldStillClearAbandonedSession(UserSession? currentSession, UserSession snapshotSession) + { + if (!ReferenceEquals(currentSession, snapshotSession)) + { + return false; + } + + return currentSession.IsAbandoned() && currentSession.NeedsCleanup(); + } + public static async Task CheckForTimeouts() { foreach (var sessionDataByClient in m_dictWebsockets) @@ -560,7 +573,7 @@ public static async Task CheckForTimeouts() } // do we need to clear out cache entries? - List> lstCacheEntriesToDestroy = new(); + List<(Int64 UserID, EUserSessionType SessionType, UserSession Session)> lstCacheEntriesToDestroy = new(); foreach (var sessionDataPerClientType in m_dictUserSessions) { foreach (var sessionData in sessionDataPerClientType.Value) @@ -569,15 +582,25 @@ public static async Task CheckForTimeouts() { if (sessionData.Value.NeedsCleanup()) { - lstCacheEntriesToDestroy.Add(new Tuple(sessionData.Key, sessionData.Value.GetSessionType())); + lstCacheEntriesToDestroy.Add((sessionData.Key, sessionData.Value.GetSessionType(), sessionData.Value)); } } } } - foreach (Tuple userData in lstCacheEntriesToDestroy) + foreach (var userData in lstCacheEntriesToDestroy) { - await ClearDataFromUser(userData.Item1, userData.Item2); + // A reconnect between the snapshot above and now would have replaced this user's + // session with a live one; only clear if the SAME session object is still registered + // and still abandoned+expired, so a fresh reconnect never has its live session torn + // down (kicked from its lobby, deregistered from matchmaking) by a stale sweep entry. + UserSession? currentSession = GetSessionFromUser(userData.UserID, userData.SessionType); + if (!ShouldStillClearAbandonedSession(currentSession, userData.Session)) + { + continue; + } + + await ClearDataFromUser(userData.UserID, userData.SessionType); } } From c7c7d491ebfedf0a819e090126783d573083e534 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sun, 27 Sep 2026 00:40:44 +0200 Subject: [PATCH 12/12] fix(anticheat): Await RegisterExeCRC in the probe response handler --- GenOnlineService/Controllers/WebSocket/WebSocketController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GenOnlineService/Controllers/WebSocket/WebSocketController.cs b/GenOnlineService/Controllers/WebSocket/WebSocketController.cs index 0adf62c..bab3424 100644 --- a/GenOnlineService/Controllers/WebSocket/WebSocketController.cs +++ b/GenOnlineService/Controllers/WebSocket/WebSocketController.cs @@ -1205,7 +1205,7 @@ await lobby.RunExclusiveAsync(() => if (data != null && data.ContainsKey("timestamp")) { string strExeCRC = data["timestamp"].GetString(); - sourceUserSession.RegisterExeCRC(strExeCRC); + await sourceUserSession.RegisterExeCRC(strExeCRC); if (lobbyInfo != null) {