Skip to content

Commit f7d6387

Browse files
committed
fix(realtime): keep the join generation guard after the access re-check
The access re-resolve added in the previous commit sat AFTER the generation / superseded guard in the table and workspace-list joins, so a leave or a newer join landing during that await no longer cancelled the stale join — it would go on to leave the room the client had switched to and commit the abandoned one. The guard is now the last thing before the commit in all three handlers, as it already was for file-doc and workflow.
1 parent 7323300 commit f7d6387

3 files changed

Lines changed: 58 additions & 8 deletions

File tree

apps/realtime/src/handlers/tables.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,6 @@ export function setupTablesHandlers(socket: AuthenticatedSocket, roomManager: IR
256256
}
257257
}
258258

259-
// Final re-check before the membership commit: a LEAVE or a newer JOIN enqueued during the
260-
// awaits above bumped the generation, or the socket disconnected. Abort before registering.
261-
if (superseded()) return
262-
263259
// Re-check access too: the access re-validation sweep records a revocation BEFORE
264260
// it evicts, so a join that authorized just before the revocation must not
265261
// complete afterwards and put the socket back in the room. RE-RESOLVES rather
@@ -277,6 +273,12 @@ export function setupTablesHandlers(socket: AuthenticatedSocket, roomManager: IR
277273
return
278274
}
279275

276+
// Final re-check before the membership commit: a LEAVE or a newer JOIN enqueued during the
277+
// awaits above — including the access re-resolve — bumped the generation, or the socket
278+
// disconnected. This is the LAST await before registering, so nothing can interleave
279+
// between it and the commit.
280+
if (superseded()) return
281+
280282
socket.join(roomName(room))
281283

282284
const presence: UserPresence = {

apps/realtime/src/handlers/workspace-invalidation-room.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,53 @@ describe.each([ROOM_TYPES.WORKSPACE_FILES, ROOM_TYPES.WORKSPACE_TABLES] as const
158158
expect(roomManager.broadcastPresenceUpdate).not.toHaveBeenCalled()
159159
})
160160

161+
it('aborts a join superseded during the access re-check await', async () => {
162+
// The access re-resolve is an await like any other: a leave landing during it must
163+
// still cancel this join, or the stale join would leave the room the client
164+
// switched to and commit the abandoned one. Forced down the re-resolve's DB path
165+
// by expiring the cached decision mid-join, so the interleaving is deterministic
166+
// rather than dependent on microtask ordering.
167+
vi.useFakeTimers()
168+
try {
169+
const { handlers, socket } = createSocket({ id: 'socket-sup', userId: 'user-sup' })
170+
setupWorkspaceInvalidationRoom(
171+
socket as unknown as Parameters<typeof setupWorkspaceInvalidationRoom>[0],
172+
createRoomManager(),
173+
roomType
174+
)
175+
176+
let call = 0
177+
mockAuthorizeRoom.mockImplementation(async () => {
178+
call += 1
179+
if (call === 1) {
180+
// A later-started read commits, so this join's own decision is dropped; then
181+
// the join stalls past the TTL so that decision is expired by re-check time.
182+
commitRoomPermission(
183+
'user-sup',
184+
{ type: roomType, id: 'ws-sup' },
185+
'admin',
186+
beginRoomPermissionRead()
187+
)
188+
await new Promise((resolve) => setTimeout(resolve, 31_000))
189+
} else {
190+
// Second call is the re-check's re-resolve: the client leaves during it.
191+
handlers[leaveEvent]({ workspaceId: 'ws-sup' })
192+
}
193+
return { allowed: true, status: 200, workspaceId: 'ws-sup', workspacePermission: 'admin' }
194+
})
195+
196+
const joining = handlers[joinEvent]({ workspaceId: 'ws-sup' })
197+
await vi.advanceTimersByTimeAsync(31_000)
198+
await joining
199+
200+
expect(call).toBe(2)
201+
expect(socket.join).not.toHaveBeenCalled()
202+
expect(socket.emit).not.toHaveBeenCalledWith(successEvent, expect.anything())
203+
} finally {
204+
vi.useRealTimers()
205+
}
206+
})
207+
161208
it('does not join when access was revoked while the join was in flight', async () => {
162209
// The sweep records a revocation before it evicts, so a join whose authorize
163210
// completed just before that must not put the socket back in the room.

apps/realtime/src/handlers/workspace-invalidation-room.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,6 @@ export function setupWorkspaceInvalidationRoom(
104104
})
105105
if (!authorized) return
106106

107-
// A newer join started on this socket during authorize (or it dropped): abort so a
108-
// stale join can't leave the room the client has since switched to.
109-
if (joinGeneration !== joinAttempt || socket.disconnected) return
110-
111107
// Re-check access before committing: the access re-validation sweep records a
112108
// revocation BEFORE it evicts, so a join that authorized just before the
113109
// revocation must not complete afterwards and put the socket back in the room.
@@ -130,6 +126,11 @@ export function setupWorkspaceInvalidationRoom(
130126
return
131127
}
132128

129+
// A newer join started on this socket during the awaits above — including the access
130+
// re-resolve — or it dropped: abort so a stale join can't leave the room the client has
131+
// since switched to. Last await before the commit, so nothing interleaves after it.
132+
if (joinGeneration !== joinAttempt || socket.disconnected) return
133+
133134
// Leave any previously-joined room of this type (workspace switch), read straight from the
134135
// socket's native room membership so there's no presence store to keep in sync.
135136
const target = roomName(ref)

0 commit comments

Comments
 (0)