Skip to content

Conversation

@tudddorrr
Copy link
Collaborator

No description provided.

@tudddorrr tudddorrr added the enhancement New feature or request label Dec 3, 2025
@claude
Copy link

claude bot commented Dec 3, 2025

🔴 Critical

Concurrent EntityManager usage creates race condition

Location: src/lib/queues/game-metrics/flush-metrics-queue-handler.ts:27-28

The change removes em.fork() when calling checkGroupMemberships in parallel:

// Before (sequential with isolated EMs)
for (const player of players) {
  await player.checkGroupMemberships(em.fork())
}

// After (parallel with shared EM)
const promises = players.map((player) => player.checkGroupMemberships(em))
await Promise.all(promises)

Problem: checkGroupMemberships modifies the EntityManager's UnitOfWork by:

  • Calling player.groups.add() / player.groups.remove() (checkGroupMemberships.ts:41-43)
  • Calling em.flush() internally (checkGroupMemberships.ts:80)

Sharing the same em across parallel operations can cause race conditions where multiple concurrent flushes and entity modifications interfere with each other.

Fix: Keep the parallelization for performance, but restore em.fork() for isolation:

const promises = players.map((player) => player.checkGroupMemberships(em.fork()))
await Promise.all(promises)

🟡 Major

No issues found


🔵 Minor

No issues found


Overall Assessment

The singleton pattern for queue handlers is a reasonable optimization since services are instantiated once. However, the concurrent EntityManager change introduces a critical bug. After fixing the EM issue, this PR will be good to merge.

@codecov
Copy link

codecov bot commented Dec 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.21%. Comparing base (330f5bc) to head (a8376db).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #720   +/-   ##
========================================
  Coverage    99.21%   99.21%           
========================================
  Files          225      225           
  Lines        16602    16607    +5     
  Branches      1536     1535    -1     
========================================
+ Hits         16471    16476    +5     
  Misses         128      128           
  Partials         3        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tudddorrr tudddorrr merged commit 97b1519 into develop Dec 3, 2025
6 checks passed
@tudddorrr tudddorrr deleted the metrics-queue-handler-singleton branch December 3, 2025 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants