Skip to content

Commit d18d6aa

Browse files
authoredSep 20, 2024··
fix(scheduler): ensure recursive jobs can't be queued twice (#11955)
1 parent 7257e6a commit d18d6aa

File tree

2 files changed

+152
-2
lines changed

2 files changed

+152
-2
lines changed
 

‎packages/runtime-core/__tests__/scheduler.spec.ts

+146
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,45 @@ describe('scheduler', () => {
517517
await nextTick()
518518
})
519519

520+
test('jobs can be re-queued after an error', async () => {
521+
const err = new Error('test')
522+
let shouldThrow = true
523+
524+
const job1: SchedulerJob = vi.fn(() => {
525+
if (shouldThrow) {
526+
shouldThrow = false
527+
throw err
528+
}
529+
})
530+
job1.id = 1
531+
532+
const job2: SchedulerJob = vi.fn()
533+
job2.id = 2
534+
535+
queueJob(job1)
536+
queueJob(job2)
537+
538+
try {
539+
await nextTick()
540+
} catch (e: any) {
541+
expect(e).toBe(err)
542+
}
543+
expect(
544+
`Unhandled error during execution of scheduler flush`,
545+
).toHaveBeenWarned()
546+
547+
expect(job1).toHaveBeenCalledTimes(1)
548+
expect(job2).toHaveBeenCalledTimes(0)
549+
550+
queueJob(job1)
551+
queueJob(job2)
552+
553+
await nextTick()
554+
555+
expect(job1).toHaveBeenCalledTimes(2)
556+
expect(job2).toHaveBeenCalledTimes(1)
557+
})
558+
520559
test('should prevent self-triggering jobs by default', async () => {
521560
let count = 0
522561
const job = () => {
@@ -558,6 +597,113 @@ describe('scheduler', () => {
558597
expect(count).toBe(5)
559598
})
560599

600+
test('recursive jobs can only be queued once non-recursively', async () => {
601+
const job: SchedulerJob = vi.fn()
602+
job.id = 1
603+
job.flags = SchedulerJobFlags.ALLOW_RECURSE
604+
605+
queueJob(job)
606+
queueJob(job)
607+
608+
await nextTick()
609+
610+
expect(job).toHaveBeenCalledTimes(1)
611+
})
612+
613+
test('recursive jobs can only be queued once recursively', async () => {
614+
let recurse = true
615+
616+
const job: SchedulerJob = vi.fn(() => {
617+
if (recurse) {
618+
queueJob(job)
619+
queueJob(job)
620+
recurse = false
621+
}
622+
})
623+
job.id = 1
624+
job.flags = SchedulerJobFlags.ALLOW_RECURSE
625+
626+
queueJob(job)
627+
628+
await nextTick()
629+
630+
expect(job).toHaveBeenCalledTimes(2)
631+
})
632+
633+
test(`recursive jobs can't be re-queued by other jobs`, async () => {
634+
let recurse = true
635+
636+
const job1: SchedulerJob = () => {
637+
if (recurse) {
638+
// job2 is already queued, so this shouldn't do anything
639+
queueJob(job2)
640+
recurse = false
641+
}
642+
}
643+
job1.id = 1
644+
645+
const job2: SchedulerJob = vi.fn(() => {
646+
if (recurse) {
647+
queueJob(job1)
648+
queueJob(job2)
649+
}
650+
})
651+
job2.id = 2
652+
job2.flags = SchedulerJobFlags.ALLOW_RECURSE
653+
654+
queueJob(job2)
655+
656+
await nextTick()
657+
658+
expect(job2).toHaveBeenCalledTimes(2)
659+
})
660+
661+
test('jobs are de-duplicated correctly when calling flushPreFlushCbs', async () => {
662+
let recurse = true
663+
664+
const job1: SchedulerJob = vi.fn(() => {
665+
queueJob(job3)
666+
queueJob(job3)
667+
flushPreFlushCbs()
668+
})
669+
job1.id = 1
670+
job1.flags = SchedulerJobFlags.PRE
671+
672+
const job2: SchedulerJob = vi.fn(() => {
673+
if (recurse) {
674+
// job2 does not allow recurse, so this shouldn't do anything
675+
queueJob(job2)
676+
677+
// job3 is already queued, so this shouldn't do anything
678+
queueJob(job3)
679+
recurse = false
680+
}
681+
})
682+
job2.id = 2
683+
job2.flags = SchedulerJobFlags.PRE
684+
685+
const job3: SchedulerJob = vi.fn(() => {
686+
if (recurse) {
687+
queueJob(job2)
688+
queueJob(job3)
689+
690+
// The jobs are already queued, so these should have no effect
691+
queueJob(job2)
692+
queueJob(job3)
693+
}
694+
})
695+
job3.id = 3
696+
job3.flags = SchedulerJobFlags.ALLOW_RECURSE | SchedulerJobFlags.PRE
697+
698+
queueJob(job1)
699+
700+
await nextTick()
701+
702+
expect(job1).toHaveBeenCalledTimes(1)
703+
expect(job2).toHaveBeenCalledTimes(1)
704+
expect(job3).toHaveBeenCalledTimes(2)
705+
})
706+
561707
// #1947 flushPostFlushCbs should handle nested calls
562708
// e.g. app.mount inside app.mount
563709
test('flushPostFlushCbs', async () => {

‎packages/runtime-core/src/scheduler.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ export function flushPreFlushCbs(
162162
cb.flags! &= ~SchedulerJobFlags.QUEUED
163163
}
164164
cb()
165-
cb.flags! &= ~SchedulerJobFlags.QUEUED
165+
if (!(cb.flags! & SchedulerJobFlags.ALLOW_RECURSE)) {
166+
cb.flags! &= ~SchedulerJobFlags.QUEUED
167+
}
166168
}
167169
}
168170
}
@@ -239,7 +241,9 @@ function flushJobs(seen?: CountMap) {
239241
job.i,
240242
job.i ? ErrorCodes.COMPONENT_UPDATE : ErrorCodes.SCHEDULER,
241243
)
242-
job.flags! &= ~SchedulerJobFlags.QUEUED
244+
if (!(job.flags! & SchedulerJobFlags.ALLOW_RECURSE)) {
245+
job.flags! &= ~SchedulerJobFlags.QUEUED
246+
}
243247
}
244248
}
245249
} finally {

0 commit comments

Comments
 (0)
Please sign in to comment.