Skip to content

Commit 419321a

Browse files
wjlafranceclaude
andcommitted
Fix GameState.Close teardown with lock instead of local captures
Local variable captures only prevented NullReferenceException but not the logical race (double cleanup if two threads call Close concurrently). Use lock(this) to make the entire teardown atomic. This is safe because Channel.RemoveUser already locks on the GameState (reentrant), and no other code path holds a different lock before calling Close. Co-Authored-By: Claude Opus 4.6 <[email protected]>
1 parent f8120d3 commit 419321a

1 file changed

Lines changed: 29 additions & 30 deletions

File tree

src/Atlasd/Battlenet/Protocols/Game/GameState.cs

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -125,42 +125,41 @@ public static bool CanStatstringUpdate(Product.ProductCode code)
125125

126126
public void Close()
127127
{
128-
// Remove this GameState from ActiveChannel
129-
var channel = ActiveChannel;
130-
if (channel != null)
128+
lock (this)
131129
{
132-
channel.RemoveUser(this); // will change this.ActiveChannel to null.
133-
}
130+
// Remove this GameState from ActiveChannel
131+
if (ActiveChannel != null)
132+
{
133+
ActiveChannel.RemoveUser(this); // will change this.ActiveChannel to null.
134+
}
134135

135-
// Notify clan members
136-
var clan = ActiveClan;
137-
if (clan != null)
138-
{
139-
clan.WriteStatusChange(this, false); // offline
140-
}
136+
// Notify clan members
137+
if (ActiveClan != null)
138+
{
139+
ActiveClan.WriteStatusChange(this, false); // offline
140+
}
141141

142-
// Update keys of ActiveAccount
143-
var account = ActiveAccount;
144-
if (account != null)
145-
{
146-
account.Set(Account.LastLogoffKey, DateTime.Now);
142+
// Update keys of ActiveAccount
143+
if (ActiveAccount != null)
144+
{
145+
ActiveAccount.Set(Account.LastLogoffKey, DateTime.Now);
147146

148-
var timeLogged = (UInt32)account.Get(Account.TimeLoggedKey);
149-
var diff = DateTime.Now - ConnectedTimestamp;
150-
timeLogged += (UInt32)Math.Round(diff.TotalSeconds);
151-
account.Set(Account.TimeLoggedKey, timeLogged);
152-
}
147+
var timeLogged = (UInt32)ActiveAccount.Get(Account.TimeLoggedKey);
148+
var diff = DateTime.Now - ConnectedTimestamp;
149+
timeLogged += (UInt32)Math.Round(diff.TotalSeconds);
150+
ActiveAccount.Set(Account.TimeLoggedKey, timeLogged);
151+
}
153152

154-
// Remove this OnlineName from ActiveAccounts and ActiveGameStates
155-
if (!string.IsNullOrEmpty(OnlineName))
156-
{
157-
Battlenet.Common.ActiveAccounts.TryRemove(OnlineName, out _);
158-
Battlenet.Common.ActiveGameStates.TryRemove(OnlineName, out _);
159-
}
153+
// Remove this OnlineName from ActiveAccounts and ActiveGameStates
154+
if (!string.IsNullOrEmpty(OnlineName))
155+
{
156+
Battlenet.Common.ActiveAccounts.TryRemove(OnlineName, out _);
157+
Battlenet.Common.ActiveGameStates.TryRemove(OnlineName, out _);
158+
}
160159

161-
// Remove this GameAd
162-
var gameAd = GameAd;
163-
if (gameAd != null && gameAd.RemoveClient(this)) GameAd = null;
160+
// Remove this GameAd
161+
if (GameAd != null && GameAd.RemoveClient(this)) GameAd = null;
162+
}
164163
}
165164

166165
public void Dispose() /* part of IDisposable */

0 commit comments

Comments
 (0)