Skip to content

Commit 347cc6c

Browse files
committed
fix(realtime): fence file joins by owner-qualified room
1 parent 78b6b71 commit 347cc6c

2 files changed

Lines changed: 106 additions & 21 deletions

File tree

‎apps/realtime/src/handlers/file-doc-project.integration.ts‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
type JoinFileDocError,
1414
type JoinFileDocPayload,
1515
type JoinFileDocSuccess,
16+
type LeaveFileDocPayload,
1617
} from '@sim/realtime-protocol/file-doc'
1718
import { createDeferred } from '@sim/testing/helpers/deferred'
1819
import { getErrorMessage } from '@sim/utils/errors'
@@ -296,6 +297,97 @@ describe('Project documents across the Socket.IO and internal HTTP boundary', ()
296297
})
297298
}
298299

300+
for (const action of ['join', 'leave'] as const) {
301+
check(
302+
action === 'join'
303+
? 'a join for another owner supersedes the pending admission for the same file'
304+
: 'a leave for another owner preserves the pending admission for the same file',
305+
async () => {
306+
const actor = `owner-intent-${generateId()}`
307+
actors.set(actor, 'read')
308+
const gate = {
309+
requests: 0,
310+
entered: createDeferred<void>(),
311+
release: createDeferred<void>(),
312+
status: 200,
313+
}
314+
accessGates.set(actor, gate)
315+
const socket = connect(socketUrl, { transports: ['websocket'], auth: { actor } })
316+
clients.push(socket)
317+
await new Promise<void>((resolve) => socket.once('connect', resolve))
318+
const server = io.sockets.sockets.get(socket.id ?? '')
319+
if (!server) throw new Error('Pending socket missing')
320+
const originalJoin = server.listeners(FILE_DOC_EVENTS.JOIN)[0]
321+
const originalLeave = server.listeners(FILE_DOC_EVENTS.LEAVE)[0]
322+
if (!originalJoin || !originalLeave) throw new Error('Document lifecycle handlers missing')
323+
const provider = new Y.Doc()
324+
const nextProvider = new Y.Doc()
325+
const done = createDeferred<void>()
326+
const actionDone = createDeferred<void>()
327+
const observeJoin = async (payload: JoinFileDocPayload) => {
328+
try {
329+
await originalJoin.call(server, payload)
330+
} finally {
331+
if (payload.clientId === provider.clientID) done.resolve()
332+
else actionDone.resolve()
333+
}
334+
}
335+
const observeLeave = async (payload: LeaveFileDocPayload) => {
336+
try {
337+
await originalLeave.call(server, payload)
338+
} finally {
339+
actionDone.resolve()
340+
}
341+
}
342+
server.off(FILE_DOC_EVENTS.JOIN, originalJoin)
343+
server.on(FILE_DOC_EVENTS.JOIN, observeJoin)
344+
server.off(FILE_DOC_EVENTS.LEAVE, originalLeave)
345+
server.on(FILE_DOC_EVENTS.LEAVE, observeLeave)
346+
const admissions: JoinFileDocSuccess[] = []
347+
const errors: JoinFileDocError[] = []
348+
socket.on(FILE_DOC_EVENTS.JOIN_SUCCESS, (payload) => admissions.push(payload))
349+
socket.on(FILE_DOC_EVENTS.JOIN_ERROR, (payload) => errors.push(payload))
350+
try {
351+
socket.emit(FILE_DOC_EVENTS.JOIN, {
352+
fileId,
353+
projectId,
354+
clientId: provider.clientID,
355+
schemaVersion: FILE_DOC_SCHEMA_VERSION,
356+
})
357+
await gate.entered.promise
358+
const otherOwner = { fileId, projectId: generateId() }
359+
if (action === 'join')
360+
socket.emit(FILE_DOC_EVENTS.JOIN, {
361+
...otherOwner,
362+
clientId: nextProvider.clientID,
363+
schemaVersion: FILE_DOC_SCHEMA_VERSION,
364+
})
365+
else socket.emit(FILE_DOC_EVENTS.LEAVE, otherOwner)
366+
await actionDone.promise
367+
gate.release.resolve()
368+
await done.promise
369+
const drained = new Promise<void>((resolve) => socket.once('fixture-drained', resolve))
370+
server.emit('fixture-drained')
371+
await drained
372+
const shouldAdmit = action === 'leave'
373+
expect(server.rooms.has(`project-file-doc:${projectId}/${fileId}`)).toBe(shouldAdmit)
374+
expect(admissions).toHaveLength(shouldAdmit ? 1 : 0)
375+
expect(errors.map((error) => error.code)).toEqual(shouldAdmit ? [] : ['NOT_FOUND'])
376+
} finally {
377+
gate.release.resolve()
378+
accessGates.delete(actor)
379+
server.off(FILE_DOC_EVENTS.JOIN, observeJoin)
380+
server.on(FILE_DOC_EVENTS.JOIN, originalJoin)
381+
server.off(FILE_DOC_EVENTS.LEAVE, observeLeave)
382+
server.on(FILE_DOC_EVENTS.LEAVE, originalLeave)
383+
socket.disconnect()
384+
provider.destroy()
385+
nextProvider.destroy()
386+
}
387+
}
388+
)
389+
}
390+
299391
for (const rejectLeave of [false, true]) {
300392
check(
301393
`a stale rejoin preserves the admitted document when delayed cleanup ${rejectLeave ? 'rejects' : 'resolves'}`,

‎apps/realtime/src/handlers/file-doc.ts‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,11 +1515,8 @@ export function setupWorkspaceFileDocHandlers(
15151515
roomManager: IRoomManager
15161516
) {
15171517
const io = roomManager.io
1518-
// The file this socket currently intends to edit (set when a join starts). A leave targeting it
1519-
// — or an unscoped leave — advances the join generation to cancel an in-flight join, so a join
1520-
// awaiting authorization can't complete after the client left and register a ghost owner. A
1521-
// leave for a DIFFERENT file must NOT cancel it (a document switch), mirroring workspace-files.
1522-
let currentFileId: string | null = null
1518+
// Owner-qualified intent fences delayed admissions and leaves across document switches.
1519+
let currentFileRoom: string | null = null
15231520
/** Co-mounted providers share invalidation membership until their last admission settles. */
15241521
const pendingMemberships = new Map<string, number>()
15251522

@@ -1594,19 +1591,18 @@ export function setupWorkspaceFileDocHandlers(
15941591
return
15951592
}
15961593

1597-
// A generation represents the socket's intended FILE, not an individual provider. Co-mounted
1598-
// providers for the same file must be allowed to join concurrently; switching files advances the
1599-
// generation so every in-flight join for the old file is cancelled together.
1600-
if (currentFileId !== fileId) {
1594+
const room = fileDocRoom(fileId, projectId)
1595+
const name = roomName(room)
1596+
1597+
// Co-mounted providers share a generation only while their owner-qualified target matches.
1598+
if (currentFileRoom !== name) {
16011599
generation = (joinGeneration.get(socket.id) ?? 0) + 1
16021600
joinGeneration.set(socket.id, generation)
1603-
currentFileId = fileId
1601+
currentFileRoom = name
16041602
} else {
16051603
generation = joinGeneration.get(socket.id) ?? 0
16061604
}
16071605

1608-
const room = fileDocRoom(fileId, projectId)
1609-
const name = roomName(room)
16101606
const admissionName = fileDocAdmissionRoom(fileId, projectId)
16111607

16121608
const authorizeJoin = () =>
@@ -1957,23 +1953,20 @@ export function setupWorkspaceFileDocHandlers(
19571953

19581954
socket.on(FILE_DOC_EVENTS.LEAVE, async (payload?: LeaveFileDocPayload) => {
19591955
try {
1960-
// Cancel an in-flight join whose file the client is now leaving (or an unscoped leave): a
1961-
// join still awaiting authorization would otherwise complete after the client left, register
1962-
// as an owner, and broadcast a ghost collaborator until disconnect. Guard on the current
1963-
// file intent so a stale/deferred leave for a DIFFERENT file can't abort the join the client
1964-
// has since switched to (bumping the generation blindly caused that regression in #5941).
1965-
if (!payload?.fileId || payload.fileId === currentFileId) {
1956+
const leavingRoom = payload?.fileId
1957+
? roomName(fileDocRoom(payload.fileId, payload.projectId))
1958+
: undefined
1959+
if (!leavingRoom || leavingRoom === currentFileRoom) {
19661960
joinGeneration.set(socket.id, (joinGeneration.get(socket.id) ?? 0) + 1)
1967-
currentFileId = null
1961+
currentFileRoom = null
19681962
}
19691963
// Tear down membership only for a REGISTERED room; a leave that raced ahead of an in-flight
19701964
// join (nothing registered yet) has already cancelled it above.
19711965
const name = socketToRoomName.get(socket.id)
19721966
if (!name) return
19731967
// Scope the leave to the named file when provided: a deferred leave from a
19741968
// prior document must not evict the socket from one it has since opened.
1975-
if (payload?.fileId && roomName(fileDocRoom(payload.fileId, payload.projectId)) !== name)
1976-
return
1969+
if (leavingRoom && leavingRoom !== name) return
19771970
cleanupFileDocForSocket(socket.id, io)
19781971
await leaveUnclaimedRoom(name)
19791972
} catch (error) {

0 commit comments

Comments
 (0)