Skip to content

Commit 40a6220

Browse files
Bojan131tabcatclaude
authored
fix(utils): guard onProgress cycles in Job dispatch (#3485)
* fix(utils): guard against re-entrant progress dispatch in Job When two jobs share progress callbacks that form a cycle, a single progress event would recurse synchronously through the recipients forEach loop until V8 hit `Maximum call stack size exceeded` and the process crashed. The cycle most often appears in DialQueue, where two parallel dials of the same peer share a job via `join()` and propagate progress events back to each other. Add a per-Job `dispatchingProgress` flag so a synchronous re-entry into the same job's synthesised onProgress short-circuits instead of recursing. Non-cyclic dispatches behave identically; only the cycle is broken. Fixes #3484 Co-authored-by: tabcat <tabcat00@proton.me> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 81e8dcc commit 40a6220

2 files changed

Lines changed: 243 additions & 3 deletions

File tree

packages/utils/src/queue/job.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export class Job <JobOptions extends AbortOptions & ProgressOptions = AbortOptio
2727
public status: JobStatus
2828
public readonly timeline: JobTimeline
2929
private readonly controller: AbortController
30+
private dispatchingProgress: boolean
3031

3132
constructor (fn: (options: JobOptions) => Promise<JobReturnType>, options: any) {
3233
this.id = randomId()
@@ -41,6 +42,8 @@ export class Job <JobOptions extends AbortOptions & ProgressOptions = AbortOptio
4142
this.controller = new AbortController()
4243
setMaxListeners(Infinity, this.controller.signal)
4344

45+
this.dispatchingProgress = false
46+
4447
this.onAbort = this.onAbort.bind(this)
4548
}
4649

@@ -80,9 +83,21 @@ export class Job <JobOptions extends AbortOptions & ProgressOptions = AbortOptio
8083
...(this.options ?? {}),
8184
signal: this.controller.signal,
8285
onProgress: (evt: any): void => {
83-
this.recipients.forEach(recipient => {
84-
recipient.onProgress?.(evt)
85-
})
86+
// Recipients can transitively re-enter this dispatcher; without
87+
// this guard a single event recurses until the stack overflows.
88+
if (this.dispatchingProgress) {
89+
return
90+
}
91+
92+
this.dispatchingProgress = true
93+
94+
try {
95+
this.recipients.forEach(recipient => {
96+
recipient.onProgress?.(evt)
97+
})
98+
} finally {
99+
this.dispatchingProgress = false
100+
}
86101
}
87102
}), this.controller.signal)
88103

packages/utils/test/queue.spec.ts

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -934,4 +934,229 @@ describe('queue', () => {
934934

935935
expect(events).to.have.lengthOf(2)
936936
})
937+
938+
it('should not recurse infinitely when two jobs progress-feed each other', async () => {
939+
interface ProgressJobOptions extends AbortOptions, ProgressOptions {
940+
941+
}
942+
943+
const queueA = new Queue<string, ProgressJobOptions>({ concurrency: 1 })
944+
const queueB = new Queue<string, ProgressJobOptions>({ concurrency: 1 })
945+
946+
let aSynthOP: ((evt: any) => void) | undefined
947+
let bSynthOP: ((evt: any) => void) | undefined
948+
949+
const aReady = pDefer<void>()
950+
const bReady = pDefer<void>()
951+
const aHold = pDefer<void>()
952+
const bHold = pDefer<void>()
953+
954+
const eventsA: any[] = []
955+
const eventsB: any[] = []
956+
957+
const pA = queueA.add(async (options) => {
958+
aSynthOP = options.onProgress
959+
aReady.resolve()
960+
await aHold.promise
961+
return 'a'
962+
}, {
963+
onProgress: (evt) => {
964+
eventsA.push(evt)
965+
bSynthOP?.(evt)
966+
}
967+
})
968+
969+
const pB = queueB.add(async (options) => {
970+
bSynthOP = options.onProgress
971+
bReady.resolve()
972+
await bHold.promise
973+
return 'b'
974+
}, {
975+
onProgress: (evt) => {
976+
eventsB.push(evt)
977+
aSynthOP?.(evt)
978+
}
979+
})
980+
981+
await Promise.all([aReady.promise, bReady.promise])
982+
983+
expect(() => {
984+
aSynthOP?.(new CustomProgressEvent('kick'))
985+
}).to.not.throw()
986+
987+
expect(eventsA).to.have.lengthOf(1)
988+
expect(eventsB).to.have.lengthOf(1)
989+
990+
aHold.resolve()
991+
bHold.resolve()
992+
993+
await Promise.all([pA, pB])
994+
})
995+
996+
it('should not recurse infinitely on a 3-job triangle (A -> B -> C -> A)', async () => {
997+
interface ProgressJobOptions extends AbortOptions, ProgressOptions {
998+
999+
}
1000+
1001+
const queueA = new Queue<string, ProgressJobOptions>({ concurrency: 1 })
1002+
const queueB = new Queue<string, ProgressJobOptions>({ concurrency: 1 })
1003+
const queueC = new Queue<string, ProgressJobOptions>({ concurrency: 1 })
1004+
1005+
let aSynthOP: ((evt: any) => void) | undefined
1006+
let bSynthOP: ((evt: any) => void) | undefined
1007+
let cSynthOP: ((evt: any) => void) | undefined
1008+
1009+
const aReady = pDefer<void>()
1010+
const bReady = pDefer<void>()
1011+
const cReady = pDefer<void>()
1012+
const hold = pDefer<void>()
1013+
1014+
const eventsA: any[] = []
1015+
const eventsB: any[] = []
1016+
const eventsC: any[] = []
1017+
1018+
const pA = queueA.add(async (options) => {
1019+
aSynthOP = options.onProgress
1020+
aReady.resolve()
1021+
await hold.promise
1022+
return 'a'
1023+
}, {
1024+
onProgress: (evt) => {
1025+
eventsA.push(evt)
1026+
bSynthOP?.(evt)
1027+
}
1028+
})
1029+
1030+
const pB = queueB.add(async (options) => {
1031+
bSynthOP = options.onProgress
1032+
bReady.resolve()
1033+
await hold.promise
1034+
return 'b'
1035+
}, {
1036+
onProgress: (evt) => {
1037+
eventsB.push(evt)
1038+
cSynthOP?.(evt)
1039+
}
1040+
})
1041+
1042+
const pC = queueC.add(async (options) => {
1043+
cSynthOP = options.onProgress
1044+
cReady.resolve()
1045+
await hold.promise
1046+
return 'c'
1047+
}, {
1048+
onProgress: (evt) => {
1049+
eventsC.push(evt)
1050+
aSynthOP?.(evt)
1051+
}
1052+
})
1053+
1054+
await Promise.all([aReady.promise, bReady.promise, cReady.promise])
1055+
1056+
expect(() => {
1057+
aSynthOP?.(new CustomProgressEvent('kick'))
1058+
}).to.not.throw()
1059+
1060+
expect(eventsA).to.have.lengthOf(1)
1061+
expect(eventsB).to.have.lengthOf(1)
1062+
expect(eventsC).to.have.lengthOf(1)
1063+
1064+
hold.resolve()
1065+
await Promise.all([pA, pB, pC])
1066+
})
1067+
1068+
it('should keep dispatching after a previous cycle completes', async () => {
1069+
interface ProgressJobOptions extends AbortOptions, ProgressOptions {
1070+
1071+
}
1072+
1073+
const queueA = new Queue<string, ProgressJobOptions>({ concurrency: 1 })
1074+
const queueB = new Queue<string, ProgressJobOptions>({ concurrency: 1 })
1075+
1076+
let aSynthOP: ((evt: any) => void) | undefined
1077+
let bSynthOP: ((evt: any) => void) | undefined
1078+
1079+
const aReady = pDefer<void>()
1080+
const bReady = pDefer<void>()
1081+
const hold = pDefer<void>()
1082+
1083+
const eventsA: any[] = []
1084+
const eventsB: any[] = []
1085+
1086+
const pA = queueA.add(async (options) => {
1087+
aSynthOP = options.onProgress
1088+
aReady.resolve()
1089+
await hold.promise
1090+
return 'a'
1091+
}, {
1092+
onProgress: (evt) => {
1093+
eventsA.push(evt)
1094+
bSynthOP?.(evt)
1095+
}
1096+
})
1097+
1098+
const pB = queueB.add(async (options) => {
1099+
bSynthOP = options.onProgress
1100+
bReady.resolve()
1101+
await hold.promise
1102+
return 'b'
1103+
}, {
1104+
onProgress: (evt) => {
1105+
eventsB.push(evt)
1106+
aSynthOP?.(evt)
1107+
}
1108+
})
1109+
1110+
await Promise.all([aReady.promise, bReady.promise])
1111+
1112+
aSynthOP?.(new CustomProgressEvent('first'))
1113+
aSynthOP?.(new CustomProgressEvent('second'))
1114+
1115+
expect(eventsA.map(e => e.type)).to.deep.equal(['first', 'second'])
1116+
expect(eventsB.map(e => e.type)).to.deep.equal(['first', 'second'])
1117+
1118+
hold.resolve()
1119+
await Promise.all([pA, pB])
1120+
})
1121+
1122+
it('resets the dispatch flag even if a recipient throws', async () => {
1123+
interface ProgressJobOptions extends AbortOptions, ProgressOptions {
1124+
1125+
}
1126+
1127+
const queue = new Queue<string, ProgressJobOptions>({ concurrency: 1 })
1128+
1129+
let synthOP: ((evt: any) => void) | undefined
1130+
1131+
const ready = pDefer<void>()
1132+
const hold = pDefer<void>()
1133+
1134+
const seen: any[] = []
1135+
let throwOnce = true
1136+
1137+
const p = queue.add(async (options) => {
1138+
synthOP = options.onProgress
1139+
ready.resolve()
1140+
await hold.promise
1141+
return 'done'
1142+
}, {
1143+
onProgress: (evt) => {
1144+
seen.push(evt)
1145+
if (throwOnce) {
1146+
throwOnce = false
1147+
throw new Error('boom')
1148+
}
1149+
}
1150+
})
1151+
1152+
await ready.promise
1153+
1154+
expect(() => synthOP?.(new CustomProgressEvent('first'))).to.throw('boom')
1155+
expect(() => synthOP?.(new CustomProgressEvent('second'))).to.not.throw()
1156+
1157+
expect(seen.map(e => e.type)).to.deep.equal(['first', 'second'])
1158+
1159+
hold.resolve()
1160+
await p
1161+
})
9371162
})

0 commit comments

Comments
 (0)