Skip to content

[API] Matchmaking confirm race creates duplicate matches #545

Description

@Flegma

The final confirming players can each independently pass the threshold check and call createMatch, double-booking servers and lineups. The non-transactional createMatch writes (see the sibling P2 issue) make the resulting half-created state hard to clean up.

Location: src/matchmaking/matchmake.service.ts:807 (api)

What: playerConfirmMatchmaking() writes the player's confirmation with hset (line 798), then re-reads the whole confirmation hash and, if confirmed.length == team1.length+team2.length, calls createMatch() (lines 804-814). There is no atomic election of a single creator: createMatch() (line 817) has no idempotency guard and is invoked once per confirming socket message. When the last two still-unconfirmed players confirm nearly simultaneously (two independent WebSocket handlers), both hsets land, then both handlers read a confirmed count equal to the team total, and both call createMatch().

Impact: A 5v5 confirmation has 8 of 10 players confirmed; players A and B (the final two) each hit 'confirm' within the same event-loop window. Handler A hsets A, handler B hsets B; both then hgetall and see 10/10 confirmed; both run createMatch(). Result: two matches are created via matchAssistant.createMatchBasedOnType, two dedicated/on-demand servers are reserved, two sets of match_lineup_players are inserted, both are set to Live, and matches:confirmation:<matchId> is written twice (last write wins), so cancelMatchMakingByMatchId later can only clean one. The same class of race also occurs between createMatch() and the delayed CancelMatchMaking job at the 30s boundary, double-booking the same players.

Suggested fix: Elect a single creator atomically, e.g. gate createMatch() behind a Redis HSETNX/SET NX 'created' flag on the confirmation key (or run the confirm+threshold check in a Lua script) so exactly one caller proceeds to createMatch().

Verifier evidence

matchmake.service.ts:798-802 await this.redis.hset(${...}:confirmed, steamId, 1); :804-805 const { lobbyIds, team1, team2, confirmed } = await this.getMatchConfirmationDetails(confirmationId); :807 if (confirmed.length != team1.length + team2.length) { ... return; }; :814 await this.createMatch(confirmationId). getMatchConfirmationDetails :712-724 derives confirmed via plain hgetall + Object.keys(confirmed) (no CAS). createMatch :826 await this.matchAssistant.createMatchBasedOnType(...) (fresh match each call, no confirmationId dedup), :839-861 lineup inserts, :863 updateMatchStatus Live, :866-870 hset matchId (last-write-wins), :872 set matches:confirmation:${match.id}. gateway matchmaking.gateway.ts:300-318 confirm handler calls playerConfirmMatchmaking with no lock (grep: 0 lock occurrences in handler); contrast join-queue lock at gateway:227 and lock impl cache.service.ts:113-133 set(lockKey,1,"EX",expires,"NX").


Found by the 2026-07 multi-agent code audit; adversarially verified (CONFIRMED, P1). One of a batch of findings from that pass.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P1-highStability & reliabilityaudit-2026-07Findings from the 2026-07 code audit / review passrace-conditionConcurrency or race conditionservice:api5stackgg/api service

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions