Skip to content

Commit dee847b

Browse files
committed
fix(realtime): guard table join leave-prior against superseding join
Round 2 on #5991: a superseded join's leave-prior could still run — during its getRoomForSocket await a newer join commits to its room, so currentRoom is that newer room and the superseded join would leave/remove/broadcast it before the final guard aborts. Re-check the generation immediately after the lookup await, before the leave mutation. Extended the post-authorize-window test to assert the superseded join never tears down the newer join's room.
1 parent 0383d79 commit dee847b

2 files changed

Lines changed: 17 additions & 6 deletions

File tree

apps/realtime/src/handlers/tables.test.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { ROOM_TYPES } from '@sim/realtime-protocol/rooms'
4+
import { ROOM_TYPES, type RoomRef } from '@sim/realtime-protocol/rooms'
55
import { TABLE_PRESENCE_EVENTS } from '@sim/realtime-protocol/table-presence'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77
import type { IRoomManager } from '@/rooms'
@@ -293,13 +293,13 @@ describe('setupTablesHandlers', () => {
293293
const aAtLookup = new Promise<void>((resolve) => {
294294
aReachedLookup = resolve
295295
})
296-
let releaseLookup: (value: unknown) => void = () => {}
297-
const pendingLookup = new Promise((resolve) => {
296+
let releaseLookup: (value: RoomRef | null) => void = () => {}
297+
const pendingLookup = new Promise<RoomRef | null>((resolve) => {
298298
releaseLookup = resolve
299299
})
300300
let lookupCalls = 0
301301
const roomManager = createRoomManager({
302-
getRoomForSocket: vi.fn(() => {
302+
getRoomForSocket: vi.fn((): Promise<RoomRef | null> => {
303303
lookupCalls += 1
304304
if (lookupCalls === 1) {
305305
aReachedLookup()
@@ -319,9 +319,16 @@ describe('setupTablesHandlers', () => {
319319
const joinA = handlers[TABLE_PRESENCE_EVENTS.JOIN]({ tableId: 'table-A' })
320320
await aAtLookup // A has passed its post-authorize recheck and is hung on the leave-prior lookup
321321
await handlers[TABLE_PRESENCE_EVENTS.JOIN]({ tableId: 'table-B' }) // bumps generation, completes
322-
releaseLookup(null)
322+
// A resumes with the socket now registered on table-B (the newer join committed there).
323+
releaseLookup({ type: ROOM_TYPES.TABLE, id: 'table-B' })
323324
await joinA
324325

326+
// A must NOT tear down B's room via its leave-prior, nor register itself on table-A.
327+
expect(socket.leave).not.toHaveBeenCalledWith('table:table-B')
328+
expect(roomManager.removeUserFromRoom).not.toHaveBeenCalledWith(
329+
{ type: ROOM_TYPES.TABLE, id: 'table-B' },
330+
'socket-1'
331+
)
325332
expect(socket.join).toHaveBeenCalledWith('table:table-B')
326333
expect(socket.join).not.toHaveBeenCalledWith('table:table-A')
327334
expect(roomManager.addUserToRoom).not.toHaveBeenCalledWith(

apps/realtime/src/handlers/tables.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,12 @@ export function setupTablesHandlers(socket: AuthenticatedSocket, roomManager: IR
139139
// switch), a LEAVE, or a disconnect. Registering below would strand the socket.
140140
if (joinGeneration !== joinAttempt || socket.disconnected) return
141141

142-
// Leave a previously-joined table room if switching tables.
142+
// Leave a previously-joined table room if switching tables. Re-check the generation
143+
// after the lookup await: if a newer join committed to a room during it, `currentRoom`
144+
// is now that room, and leaving it here would tear down the join the client actually
145+
// holds. A superseded join must abort before this mutation.
143146
const currentRoom = await roomManager.getRoomForSocket(socket.id, ROOM_TYPES.TABLE)
147+
if (joinGeneration !== joinAttempt || socket.disconnected) return
144148
if (currentRoom && currentRoom.id !== tableId) {
145149
socket.leave(roomName(currentRoom))
146150
await roomManager.removeUserFromRoom(currentRoom, socket.id)

0 commit comments

Comments
 (0)