Skip to content

Fix lobby race: single-transaction gated add - #46

Open
wasita wants to merge 1 commit into
mainfrom
fix-lobby-race
Open

Fix lobby race: single-transaction gated add#46
wasita wants to merge 1 commit into
mainfrom
fix-lobby-race

Conversation

@wasita

@wasita wasita commented Apr 7, 2026

Copy link
Copy Markdown

Summary

  • Collapse `reqMetaDocChange` add + capacity check + reset into a single Firestore transaction, closing the two-transaction gap that allowed `meta.counter` to overflow past `groupSize` and deadlock the lobby whenever 3+ users joined near-simultaneously.
  • Gate the add on capacity so the invariant `counter.length < groupSize` holds after every committed write. Overflow becomes structurally impossible, and the `=== groupSize` check on the closing user is safe again.
  • Return the dyad from the transaction callback instead of capturing it in a closure variable, so Firestore retries can't leak stale state across attempts.
  • Throw `new Error(...)` instead of string literals; use a status-tagged return value (`duplicate` / `full` / `waiting` / `closed`) so `LOBBY_FULL` is a loud assertion rather than a silent deadlock.
  • Delete `checkIfResetCounter` and the commented-out pairing loop that was never viable (non-transactional `initGroup` side effects).

How it works

Each `reqMetaDocChange` call runs one transaction that either:

  • duplicate: user already in counter → no-op
  • full: counter is already at capacity (should never fire under the new invariant; surfaced as a warning so external writes or bugs are visible)
  • waiting: user appended, counter still below capacity
  • closed: user appended, counter hit capacity, reset to `[]` atomically, dyad returned

Only the closed caller runs `initGroup`. Non-closing users are notified via the existing `onSnapshot` listener on their participant doc in `App.svelte:188`, which reactively subscribes them to the group doc once `initGroup` writes their `groupId` at `utils.js:1161`. (Listener chain verified in this PR.)

Test plan

  • 2 browser windows: confirm the existing happy path still forms a dyad and both users advance.
  • 3 browser windows opened nearly simultaneously: first two should pair, third should remain queued until a 4th joins.
  • 4+ browser windows opened nearly simultaneously: two dyads form, no users stranded, no duplicate groups.
  • Double-click / re-entry on a user already in the counter: dedupe path hits, no spurious state change.
  • Manually set `meta.counter` to length `>= groupSize` in the Firebase console, then have a user join: `LOBBY_FULL` warning should fire and the user should not be added.

…action

Previously reqMetaDocChange and checkIfResetCounter ran as two separate
Firestore transactions. Between them the counter could grow past groupSize,
and the === groupSize check would then fall through to a log-only branch,
permanently deadlocking the lobby whenever 3+ users joined near-simultaneously.

Collapse everything into one gated-add transaction that maintains the
invariant counter.length < groupSize after every committed write. The
closing user captures the dyad via the transaction's return value (not a
closure variable, which would leak across retries) and calls initGroup;
other users learn of the group via their participant-doc onSnapshot listener.
@github-actions

github-actions Bot commented Apr 7, 2026

Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit 78f2617):

https://shared-reality-chat--pr46-fix-lobby-race-qbwlkh70.web.app

(expires Tue, 14 Apr 2026 20:46:35 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 610727926b0a3107426ed7cfe5bfddf4ee1485a4

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