Location: apps/counterstrikesharp/src/FiveStack.Services/MatchEvents.cs:318 (game-server)
What: _pendingMessages is a plain non-thread-safe Dictionary that is written from three different thread contexts: the game thread (PublishGameEvent -> Publish adds/removes at lines 366-371), the retry System.Timers.Timer callback which runs on a ThreadPool thread (RetryPendingMessages enumerates via .Where().ToList() and reassigns entries at 318-342), and the WebSocket receive continuation which also runs off the game thread (MonitorConnection calls _pendingMessages.Remove at line 189). None of these accesses are synchronized.
Impact: During a live match many game events are published on the game thread while the 5s retry timer thread simultaneously enumerates/writes _pendingMessages and the socket thread removes acked ids. Concurrent Dictionary writes either throw InvalidOperationException/'collection was modified' or corrupt the internal bucket structure, which can spin an insert into an infinite loop and hang or crash the server process mid-match.
Suggested fix: Guard every read/write of _pendingMessages with a lock (or switch to ConcurrentDictionary), or marshal all mutations onto a single thread via Server.NextFrame.
Verifier evidence
MatchEvents.cs:18-21 private Dictionary<Guid,(EventData<...> Event, DateTime Timestamp)> _pendingMessages = new(); (non-thread-safe, no guard).
MatchEvents.cs:43-44 _retryTimer = new System.Timers.Timer(RETRY_INTERVAL_MS); _retryTimer.Elapsed += async (sender, e) => await RetryPendingMessages(); (ThreadPool, no SynchronizingObject).
MatchEvents.cs:318-322 var messagesToRetry = _pendingMessages.Where(m => ...).ToList(); (unguarded enumeration on timer thread).
MatchEvents.cs:342 _pendingMessages[message.Key] = (message.Value.Event, currentTime); (write on timer thread).
MatchEvents.cs:189 _pendingMessages.Remove(messageId); (remove on socket-continuation thread inside MonitorConnection).
MatchEvents.cs:366-371 _pendingMessages[data.messageId] = (typedData, DateTime.UtcNow); if (_pendingMessages.Count > 1000){ var oldest=_pendingMessages.OrderBy(...).First(); _pendingMessages.Remove(oldest.Key);} (add/remove on game thread via Publish, before first await at line 379).
grep across file: 0 matches for lock/Concurrent/SynchronizingObject/NextFrame. Callers confirming game-thread frequency: PlayerKills.cs:51,98; PlayerDamage.cs:51; Bomb.cs:28,62,96; RoundEnd.cs:223; PlayerUtility.cs (7 sites), etc.
Found by the 2026-07 multi-agent code audit; adversarially verified (CONFIRMED, P2). One of a batch of findings from that pass.
Location:
apps/counterstrikesharp/src/FiveStack.Services/MatchEvents.cs:318(game-server)What: _pendingMessages is a plain non-thread-safe Dictionary that is written from three different thread contexts: the game thread (PublishGameEvent -> Publish adds/removes at lines 366-371), the retry System.Timers.Timer callback which runs on a ThreadPool thread (RetryPendingMessages enumerates via .Where().ToList() and reassigns entries at 318-342), and the WebSocket receive continuation which also runs off the game thread (MonitorConnection calls _pendingMessages.Remove at line 189). None of these accesses are synchronized.
Impact: During a live match many game events are published on the game thread while the 5s retry timer thread simultaneously enumerates/writes _pendingMessages and the socket thread removes acked ids. Concurrent Dictionary writes either throw InvalidOperationException/'collection was modified' or corrupt the internal bucket structure, which can spin an insert into an infinite loop and hang or crash the server process mid-match.
Suggested fix: Guard every read/write of _pendingMessages with a lock (or switch to ConcurrentDictionary), or marshal all mutations onto a single thread via Server.NextFrame.
Verifier evidence
MatchEvents.cs:18-21
private Dictionary<Guid,(EventData<...> Event, DateTime Timestamp)> _pendingMessages = new();(non-thread-safe, no guard).MatchEvents.cs:43-44
_retryTimer = new System.Timers.Timer(RETRY_INTERVAL_MS); _retryTimer.Elapsed += async (sender, e) => await RetryPendingMessages();(ThreadPool, no SynchronizingObject).MatchEvents.cs:318-322
var messagesToRetry = _pendingMessages.Where(m => ...).ToList();(unguarded enumeration on timer thread).MatchEvents.cs:342
_pendingMessages[message.Key] = (message.Value.Event, currentTime);(write on timer thread).MatchEvents.cs:189
_pendingMessages.Remove(messageId);(remove on socket-continuation thread inside MonitorConnection).MatchEvents.cs:366-371
_pendingMessages[data.messageId] = (typedData, DateTime.UtcNow); if (_pendingMessages.Count > 1000){ var oldest=_pendingMessages.OrderBy(...).First(); _pendingMessages.Remove(oldest.Key);}(add/remove on game thread via Publish, before first await at line 379).grep across file: 0 matches for lock/Concurrent/SynchronizingObject/NextFrame. Callers confirming game-thread frequency: PlayerKills.cs:51,98; PlayerDamage.cs:51; Bomb.cs:28,62,96; RoundEnd.cs:223; PlayerUtility.cs (7 sites), etc.
Found by the 2026-07 multi-agent code audit; adversarially verified (CONFIRMED, P2). One of a batch of findings from that pass.