Skip to content

Fix lobby race: single-transaction gated add #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wasita wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix lobby race: single-transaction gated add #46

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 94 additions & 82 deletions src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1373,107 +1373,119 @@ export const reqUserStateChange = async (newState) => {
}
};

// add userId to meta counter
// ──────────────────────────────────────────────────────────────────────────
// Lobby matching — why this is structured as a single transaction
// ──────────────────────────────────────────────────────────────────────────
// A previous implementation split lobby matching into two transactions: one
// to append the user to meta.counter, and a second to check if the counter
// was full and reset it. Between those two transactions, additional users
// could join the counter, so counter.length could exceed groupSize. The
// reset check used strict equality (counter.length === groupSize), so any
// overflow state would fall through to a log-and-do-nothing branch and the
// lobby would permanently deadlock (e.g. 3+ users joining near-simultaneously
// would never get paired).
//
// This version collapses add + capacity check + reset into ONE transaction,
// and gates the add on capacity. That maintains the invariant:
//
// after every committed write, counter.length < groupSize
//
// Because the invariant holds, overflow is structurally impossible, and the
// `=== groupSize` check on the closing user is safe. When a user's add
// brings the counter to exactly groupSize, that same transaction captures
// the dyad and resets the counter to [] atomically — no other client can
// ever observe the "full" state, so there's no window for a second client
// to also try to initialize the group.
//
// Firestore's optimistic concurrency guarantees transaction retries on
// conflict, so concurrent joiners are serialized: each retry reads the
// committed post-state of the previous commit. Each attempt therefore does
// exactly one of:
// - "duplicate": user already present in counter → no-op
// - "full": counter already at capacity (invariant violation — this
// should never fire in normal operation; it's an assertion
// that surfaces external writes or bugs loudly rather than
// silently deadlocking)
// - "waiting": user appended, counter still below capacity
// - "closed": user appended, counter hit capacity, reset + dyad captured
//
// Only the "closed" caller calls initGroup. Non-closing users learn about
// their new group via the onSnapshot listener on their participant doc
// (see App.svelte:188), which fires when initGroup writes groupId onto their
// user record and then transitively subscribes them to the group doc.
//
// Note: `dyad` is returned from the transaction callback rather than being
// captured in a closure. Firestore may retry the callback on conflict, and
// a closure variable set on attempt #1 could leak stale state into the
// post-transaction code if attempt #2 takes a different branch. Returning
// the result from runTransaction avoids that class of bug entirely — each
// retry produces its own fresh result object.
// ──────────────────────────────────────────────────────────────────────────

// add userId to meta counter; if this add closes the lobby, initialize the group
export const reqMetaDocChange = async (userId) => {
const metaDocRef = doc(db, metaCollectionName, 'data');

// update meta doc
let result;
try {
await runTransaction(db, async (transaction) => {

// Get the latest data, rather than relying on the store
result = await runTransaction(db, async (transaction) => {
const document = await transaction.get(metaDocRef);
if (!document.exists()) {
throw "Document does not exist!";
throw new Error("Meta doc does not exist");
}
// Freshest data
const { counter } = document.data();
const data = { counter: [...counter, userId] };

// Add the user to the counter if they're not already in it
if (!counter.includes(userId)) {
await transaction.update(metaDocRef, data);
} else {
console.log("Ignoring duplicate request");
// Dedupe: user already in the lobby (e.g. double-click, reconnect).
if (counter.includes(userId)) {
console.log(`User ${userId} already in lobby counter; ignoring duplicate request`);
return { status: "duplicate" };
}
});
} catch (error) {
console.error(`Error updating meta doc for userId: ${userId}`, error);
}
// Use helper function to run a second transaction that checks the counter length and
// actually performs the state change if appropriate
await checkIfResetCounter(userId);
};

// helper funtion to run check for meta doc change
// every time user joins, check if counter is full
const checkIfResetCounter = async () => {
// Get latest counter
let counter;

const metaDocRef = doc(db, metaCollectionName, 'data');
try {
await runTransaction(db, async (transaction) => {
// Get the latest data, rather than relying on the store
const document = await transaction.get(metaDocRef);
if (!document.exists()) {
throw "Document does not exist!";
// Invariant check: counter should never be at or above groupSize after
// any committed write. If it is, something external has touched it.
// Refuse to add this user and surface loudly rather than silently
// deadlocking or joining a phantom group.
if (counter.length >= globalVars.groupSize) {
console.warn(
`LOBBY_FULL invariant violation: counter.length=${counter.length} >= ` +
`groupSize=${globalVars.groupSize}. Not adding user ${userId}.`
);
return { status: "full" };
}
// Get latest counter
counter = document.data().counter;

// group size met
if (counter.length === globalVars.groupSize) {
// if you're the last user to join the group,
// you initialize the group doc
console.log('Resetting counter...');
// reset counter
const obj = {};
obj["counter"] = [];
await transaction.update(metaDocRef, obj);
} else {
console.log("No need to reset. Still waiting for users to join...");
const next = [...counter, userId];

if (next.length === globalVars.groupSize) {
// This user's add closes the lobby. Atomically reset the counter
// and capture the dyad for initGroup below.
transaction.update(metaDocRef, { counter: [] });
console.log(`Lobby closed by ${userId}; dyad=[${next}]`);
return { status: "closed", dyad: next };
}

// Otherwise just append and wait for more joiners.
transaction.update(metaDocRef, { counter: next });
console.log(
`User ${userId} added to lobby; still waiting for ` +
`${globalVars.groupSize - next.length} more`
);
return { status: "waiting" };
});
} catch (error) {
console.error(`Error verifying meta doc change`, error);
console.error(`Error updating meta doc for userId: ${userId}`, error);
return;
}

if (counter.length === globalVars.groupSize) {
console.log('Initializing group...');
// initialize group
await initGroup(counter);
} else if (counter.length > globalVars.groupSize) {
console.log(`Counter length ${counter.length} exceeds group size`);
console.log("counter", counter)

// // Loop over the array two items at a time
// for (let i = 0; i < counter.length; i += 2) {
// // Check if there is only one item left
// if (counter.length === 1) {
// console.log("1 person left:", counter[0]);

// // reset counter
// // const obj = {};
// // obj["counter"] = counter;
// // await transaction.update(metaDocRef, obj);
// } else {
// // Get two items from the array
// let userA = counter[0];
// let userB = counter[1];
// console.log("2 people moving:", userA, userB)

// // Call initGroup with two items
// await initGroup([userA, userB]);
// // Remove the processed items from the array
// counter.splice(0, 2);
// console.log("updated coutner", counter);
// }
// }
} else {
console.log(`Still waiting for ${globalVars.groupSize - counter.length} requests...`);
if (result && result.status === "closed") {
// Only the closing user runs initGroup. Other users in the dyad are
// notified via their participant-doc onSnapshot listener (App.svelte)
// once initGroup writes their groupId.
await initGroup(result.dyad);
} else if (result && result.status === "full") {
// Optional: surface to caller so the UI can route to a failure/wait
// screen. For now we just return; callers can inspect logs.
return;
}

};

// Save vignette information (no chat participants only)
Expand Down
Loading