Skip to content

Commit 06784c6

Browse files
icecrasher321claude
andcommitted
fix(realtime): round-robin the revocation scan so hung checks cannot starve later sockets
Resume each scan pass after the last target the previous pass processed, so a fixed prefix of hanging authorization checks can never repeatedly consume the pass budget and leave sockets behind it unexamined. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ead74af commit 06784c6

2 files changed

Lines changed: 94 additions & 42 deletions

File tree

apps/realtime/src/access-revalidation.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,37 @@ describe('access-revalidation sweep', () => {
319319
}
320320
})
321321

322+
it('rotates the scan start so hung checks cannot starve later sockets', async () => {
323+
vi.useFakeTimers()
324+
try {
325+
// Four hung authorization checks consume exactly the 20s pass budget
326+
// (4 × 5s per-socket timeout); the revoked socket sits behind them.
327+
const hungSockets = [1, 2, 3, 4].map((i) => makeSocket(`sock-${i}`, `user-${i}`, 'wf-1'))
328+
const revoked = makeSocket('sock-5', 'user-5', 'wf-1')
329+
const manager = makeManager([...hungSockets, revoked])
330+
mockResolveRole.mockImplementation(async (userId: string) => {
331+
if (userId === 'user-5') return null
332+
return new Promise(() => {})
333+
})
334+
335+
const sweep = startAccessRevalidationSweep(manager)
336+
337+
// First pass burns its whole budget on the hung prefix.
338+
await vi.advanceTimersByTimeAsync(ACCESS_REVALIDATION_SWEEP_INTERVAL_MS)
339+
await vi.advanceTimersByTimeAsync(25_000)
340+
expect(revoked.leave).not.toHaveBeenCalled()
341+
342+
// Second pass resumes after the last processed socket, so the revoked
343+
// socket is examined first and evicted.
344+
await vi.advanceTimersByTimeAsync(10_000)
345+
sweep.stop()
346+
347+
expect(revoked.leave).toHaveBeenCalledWith('wf-1')
348+
} finally {
349+
vi.useRealTimers()
350+
}
351+
})
352+
322353
it('keeps scanning on later ticks while a deferred cleanup hangs', async () => {
323354
vi.useFakeTimers()
324355
try {

apps/realtime/src/access-revalidation.ts

Lines changed: 63 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -55,28 +55,33 @@ export interface AccessRevalidationSweep {
5555
runOnce: () => Promise<void>
5656
}
5757

58+
interface ScanTarget {
59+
workflowId: string
60+
socket: AuthenticatedSocket
61+
userId: string
62+
}
63+
5864
/**
59-
* Groups this pod's local sockets by the workflow room each has joined.
65+
* Collects this pod's authenticated sockets with the workflow room each has
66+
* joined, in stable socket order.
6067
*
6168
* The workflow room is derived from the socket's own `rooms` set (pod-local, no
6269
* Redis round-trips): a socket joins exactly one workflow room, so its rooms are
6370
* `{ ownSocketId, workflowId }`. Only local sockets are evaluated — sockets are
6471
* sticky to a pod, so every socket is swept by exactly one pod using that pod's
6572
* warm role cache (mirroring the per-pod reasoning of the write-path cache).
6673
*/
67-
function collectLocalMemberships(io: IRoomManager['io']): Map<string, AuthenticatedSocket[]> {
68-
const byWorkflow = new Map<string, AuthenticatedSocket[]>()
74+
function collectScanTargets(io: IRoomManager['io']): ScanTarget[] {
75+
const targets: ScanTarget[] = []
6976
for (const socket of io.sockets.sockets.values()) {
7077
const authed = socket as AuthenticatedSocket
7178
if (!authed.userId) continue
7279
for (const room of socket.rooms) {
7380
if (room === socket.id) continue
74-
const existing = byWorkflow.get(room)
75-
if (existing) existing.push(authed)
76-
else byWorkflow.set(room, [authed])
81+
targets.push({ workflowId: room, socket: authed, userId: authed.userId })
7782
}
7883
}
79-
return byWorkflow
84+
return targets
8085
}
8186

8287
/**
@@ -104,6 +109,13 @@ export function startAccessRevalidationSweep(roomManager: IRoomManager): AccessR
104109
const io = roomManager.io
105110
let scanRunning = false
106111
let cleanupRunning = false
112+
/**
113+
* Round-robin cursor: the `${socketId}:${workflowId}` key of the last target
114+
* the previous pass processed. Each pass resumes after it, so a fixed prefix
115+
* of hanging authorization checks can never starve the sockets behind it —
116+
* every target is examined within a bounded number of passes.
117+
*/
118+
let scanCursorKey: string | null = null
107119

108120
/**
109121
* Room-state cleanups owed for evicted sockets, keyed
@@ -199,48 +211,57 @@ export function startAccessRevalidationSweep(roomManager: IRoomManager): AccessR
199211
}
200212

201213
async function scanMemberships(): Promise<void> {
202-
const memberships = collectLocalMemberships(io)
214+
const targets = collectScanTargets(io)
215+
if (targets.length === 0) return
216+
217+
let startIndex = 0
218+
if (scanCursorKey !== null) {
219+
const cursorIndex = targets.findIndex(
220+
({ socket, workflowId }) => `${socket.id}:${workflowId}` === scanCursorKey
221+
)
222+
if (cursorIndex !== -1) {
223+
startIndex = (cursorIndex + 1) % targets.length
224+
}
225+
}
226+
203227
const deadline = Date.now() + SCAN_PASS_BUDGET_MS
204228

205-
for (const [workflowId, sockets] of memberships) {
206-
for (const socket of sockets) {
207-
const userId = socket.userId
208-
if (!userId) continue
229+
for (let offset = 0; offset < targets.length; offset++) {
230+
const { workflowId, socket, userId } = targets[(startIndex + offset) % targets.length]
209231

210-
const remainingBudget = deadline - Date.now()
211-
if (remainingBudget <= 0) {
212-
logger.warn(
213-
'Access re-validation scan budget exhausted; remaining sockets defer to the next pass'
214-
)
215-
return
216-
}
232+
const remainingBudget = deadline - Date.now()
233+
if (remainingBudget <= 0) {
234+
logger.warn(
235+
'Access re-validation scan budget exhausted; remaining sockets defer to the next pass'
236+
)
237+
return
238+
}
217239

218-
try {
219-
// Bounded wait: a hanging authorization query skips this socket for
220-
// the pass instead of wedging the scan lane. The single-flighted
221-
// resolution keeps running in the background and is re-raced next
222-
// pass, so it is acted on once it settles.
223-
const role = await Promise.race([
224-
resolveCurrentWorkflowRole(userId, workflowId, FALLBACK_ROLE),
225-
sleep(Math.min(SCAN_SOCKET_TIMEOUT_MS, remainingBudget)).then(() => SCAN_TIMED_OUT),
226-
])
227-
if (role === SCAN_TIMED_OUT) {
228-
logger.warn(
229-
`Authorization check timed out for user ${userId} on workflow ${workflowId}; skipping this pass`
230-
)
231-
continue
232-
}
233-
if (role === null) {
234-
revokeSocket(socket, workflowId)
235-
}
236-
} catch (error) {
237-
// Never evict on an unexpected error — only a definitive `null` role
238-
// evicts, so a failure here leaves the socket's access intact.
240+
try {
241+
// Bounded wait: a hanging authorization query skips this socket for
242+
// the pass instead of wedging the scan lane. The single-flighted
243+
// resolution keeps running in the background and is re-raced when the
244+
// rotation returns to this socket, so it is acted on once it settles.
245+
const role = await Promise.race([
246+
resolveCurrentWorkflowRole(userId, workflowId, FALLBACK_ROLE),
247+
sleep(Math.min(SCAN_SOCKET_TIMEOUT_MS, remainingBudget)).then(() => SCAN_TIMED_OUT),
248+
])
249+
if (role === SCAN_TIMED_OUT) {
239250
logger.warn(
240-
`Access re-validation failed for user ${userId} on workflow ${workflowId}; leaving membership intact`,
241-
error
251+
`Authorization check timed out for user ${userId} on workflow ${workflowId}; skipping this pass`
242252
)
253+
} else if (role === null) {
254+
revokeSocket(socket, workflowId)
243255
}
256+
} catch (error) {
257+
// Never evict on an unexpected error — only a definitive `null` role
258+
// evicts, so a failure here leaves the socket's access intact.
259+
logger.warn(
260+
`Access re-validation failed for user ${userId} on workflow ${workflowId}; leaving membership intact`,
261+
error
262+
)
263+
} finally {
264+
scanCursorKey = `${socket.id}:${workflowId}`
244265
}
245266
}
246267
}

0 commit comments

Comments
 (0)