Skip to content

refactor: replace Dictionary with ConcurrentDictionary for sessions#663

Merged
AngeloTadeucci merged 1 commit intomasterfrom
concurrent-sessions-dict
Mar 6, 2026
Merged

refactor: replace Dictionary with ConcurrentDictionary for sessions#663
AngeloTadeucci merged 1 commit intomasterfrom
concurrent-sessions-dict

Conversation

@AngeloTadeucci
Copy link
Collaborator

@AngeloTadeucci AngeloTadeucci commented Mar 6, 2026

Replace the Dictionary<long, GameSession> with
ConcurrentDictionary<long, GameSession> to provide thread-safe access without explicit locking. This eliminates the need for mutex locks around session dictionary operations.

Remove mutex locks from OnConnected, OnDisconnected, GetSession, GetSessionByAccountId, GetSessions, and event broadcasting methods since ConcurrentDictionary handles synchronization internally.

Update OnDisconnected to use TryRemove with KeyValuePair for atomic reference equality checks, ensuring the correct session is removed during reconnection scenarios (same-channel migration).

Remove unused Maple2.Graphics.Interface project.

Summary by CodeRabbit

  • Performance & Stability
    • Improved session management with enhanced concurrency handling to ensure more reliable and stable server operations during high-load scenarios.

Replace the Dictionary<long, GameSession> with
ConcurrentDictionary<long, GameSession> to provide thread-safe
access without explicit locking. This eliminates the need for mutex
locks around session dictionary operations.

Remove mutex locks from OnConnected, OnDisconnected, GetSession,
GetSessionByAccountId, GetSessions, and event broadcasting methods
since ConcurrentDictionary handles synchronization internally.

Update OnDisconnected to use TryRemove with KeyValuePair for atomic
reference equality checks, ensuring the correct session is removed
during reconnection scenarios (same-channel migration).

Remove unused Maple2.Graphics.Interface project.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 6, 2026

📝 Walkthrough

Walkthrough

The PR replaces manual mutex-based locking on session management with a lock-free ConcurrentDictionary approach, eliminating explicit synchronization code. Additionally, the Graphics.Interface project configuration file is completely cleared of its build settings.

Changes

Cohort / File(s) Summary
Project Configuration
Maple2.Graphics.Interface/Maple2.Graphics.Interface.csproj
Removed all project configuration including TargetFramework, ImplicitUsings, and Nullable settings.
Session Concurrency Refactor
Maple2.Server.Game/GameServer.cs
Replaced Dictionary with ConcurrentDictionary for session storage; removed all explicit mutex locking and simplified accessor methods to directly query the concurrent collection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • MS2Community/Maple2#450: Implements alternative approach to session concurrency management in GameServer.cs using mutex-based locking and GetSessions returning a List, creating a direct conflict with this PR's lock-free ConcurrentDictionary strategy.

Suggested reviewers

  • Zintixx

Poem

🐰 Hop-hop, the locks go away,
Concurrent dictionary saves the day!
No more mutex, just thread-safe delight,
Sessions now dance without locks held tight! 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main change: replacing Dictionary with ConcurrentDictionary for thread-safe session management in GameServer.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch concurrent-sessions-dict

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
Maple2.Server.Game/GameServer.cs (1)

91-93: Consider performance for high session counts.

This performs an O(n) scan over all sessions. While acceptable for typical server populations, if session counts grow large, consider maintaining a secondary ConcurrentDictionary<long, GameSession> keyed by AccountId for O(1) lookups.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Maple2.Server.Game/GameServer.cs` around lines 91 - 93, GetSessionByAccountId
currently scans sessions.Values with FirstOrDefault which is O(n); add and
maintain a secondary concurrent map keyed by AccountId (e.g.,
ConcurrentDictionary<long, GameSession> accountSessions) and update it whenever
sessions are added/removed/changed (the same places that modify the primary
sessions collection) so GetSessionByAccountId can perform an O(1) lookup from
accountSessions.TryGetValue(accountId, out var session) and return the session
or null; ensure thread-safety and keep accountSessions in sync with the primary
sessions map and reference the symbols GetSessionByAccountId, sessions,
AccountId, and GameSession when making changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@Maple2.Server.Game/GameServer.cs`:
- Around line 91-93: GetSessionByAccountId currently scans sessions.Values with
FirstOrDefault which is O(n); add and maintain a secondary concurrent map keyed
by AccountId (e.g., ConcurrentDictionary<long, GameSession> accountSessions) and
update it whenever sessions are added/removed/changed (the same places that
modify the primary sessions collection) so GetSessionByAccountId can perform an
O(1) lookup from accountSessions.TryGetValue(accountId, out var session) and
return the session or null; ensure thread-safety and keep accountSessions in
sync with the primary sessions map and reference the symbols
GetSessionByAccountId, sessions, AccountId, and GameSession when making changes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 42232260-aeab-42e4-8637-47126c83352c

📥 Commits

Reviewing files that changed from the base of the PR and between 6590f86 and 66989b2.

📒 Files selected for processing (2)
  • Maple2.Graphics.Interface/Maple2.Graphics.Interface.csproj
  • Maple2.Server.Game/GameServer.cs
💤 Files with no reviewable changes (1)
  • Maple2.Graphics.Interface/Maple2.Graphics.Interface.csproj

@AngeloTadeucci AngeloTadeucci merged commit 819ff6f into master Mar 6, 2026
3 checks passed
@AngeloTadeucci AngeloTadeucci deleted the concurrent-sessions-dict branch March 6, 2026 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant