Skip to content

Commit 35a349b

Browse files
wjlafranceclaude
andcommitted
Fix ActiveGameAds race conditions with consistent locking
ActiveGameAds was a List<GameAd> accessed from multiple threads with inconsistent locking. Add a dedicated ActiveGameAdsLock object and use it at all access sites. SID_STOPADV previously had no lock at all. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ed2b315 commit 35a349b

8 files changed

Lines changed: 8 additions & 7 deletions

File tree

src/Atlasd/Battlenet/Common.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public ShutdownEvent(string adminMessage, bool cancelled, DateTime eventDate, Ti
4343
public static ConcurrentDictionary<byte[], Clan> ActiveClans;
4444
public static ConcurrentDictionary<Socket, ClientState> ActiveClientStates;
4545
public static List<GameAd> ActiveGameAds;
46+
public static readonly object ActiveGameAdsLock = new object();
4647
public static ConcurrentDictionary<string, GameState> ActiveGameStates;
4748
public static IPAddress DefaultAddress { get; private set; }
4849
public static int DefaultPort { get; private set; }

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_ENTERCHAT.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public override bool Invoke(MessageContext context)
102102
{
103103
var gameAd = gameState.GameAd;
104104
if (gameAd.RemoveClient(gameState)) gameState.GameAd = null;
105-
if (gameAd.Clients.Count == 0) lock (Battlenet.Common.ActiveGameAds) Battlenet.Common.ActiveGameAds.Remove(gameAd);
105+
if (gameAd.Clients.Count == 0) lock (Battlenet.Common.ActiveGameAdsLock) Battlenet.Common.ActiveGameAds.Remove(gameAd);
106106
}
107107

108108
if (gameState.ActiveChannel == null)

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_GETADVLISTEX.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public override bool Invoke(MessageContext context)
5858

5959
var gameAds = new List<GameAd>();
6060

61-
lock (Battlenet.Common.ActiveGameAds)
61+
lock (Battlenet.Common.ActiveGameAdsLock)
6262
{
6363
IList<GameAd> toDelete = new List<GameAd>();
6464
foreach (var gameAd in Battlenet.Common.ActiveGameAds)

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_NOTIFYJOIN.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public override bool Invoke(MessageContext context)
5252
if (gameState.ActiveChannel != null)
5353
gameState.ActiveChannel.RemoveUser(gameState);
5454

55-
lock (Battlenet.Common.ActiveGameAds)
55+
lock (Battlenet.Common.ActiveGameAdsLock)
5656
{
5757
foreach (var gameAd in Battlenet.Common.ActiveGameAds)
5858
{

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public override bool Invoke(MessageContext context)
6969
Statuses status = Statuses.Failed;
7070
GameAd gameAd = null;
7171

72-
lock (Battlenet.Common.ActiveGameAds)
72+
lock (Battlenet.Common.ActiveGameAdsLock)
7373
{
7474
foreach (GameAd _ad in Battlenet.Common.ActiveGameAds)
7575
{

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX2.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public override bool Invoke(MessageContext context)
6969
Statuses status = Statuses.Failed;
7070
GameAd gameAd = null;
7171

72-
lock (Battlenet.Common.ActiveGameAds)
72+
lock (Battlenet.Common.ActiveGameAdsLock)
7373
{
7474
foreach (GameAd _ad in Battlenet.Common.ActiveGameAds)
7575
{

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX3.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public override bool Invoke(MessageContext context)
7373
Statuses status = Statuses.Error;
7474
GameAd gameAd = null;
7575

76-
lock (Battlenet.Common.ActiveGameAds)
76+
lock (Battlenet.Common.ActiveGameAdsLock)
7777
{
7878
foreach (GameAd _ad in Battlenet.Common.ActiveGameAds)
7979
{

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STOPADV.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public override bool Invoke(MessageContext context)
4242
if (!gameAdOwner)
4343
Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Game, context.Client.RemoteEndPoint, $"{MessageName(Id)} was received but they are not the owner of the game advertisement");
4444
else
45-
Battlenet.Common.ActiveGameAds.Remove(gs.GameAd);
45+
lock (Battlenet.Common.ActiveGameAdsLock) Battlenet.Common.ActiveGameAds.Remove(gs.GameAd);
4646

4747
return true;
4848
}

0 commit comments

Comments
 (0)