Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(scheduler): ensure recursive jobs can't be queued twice #11955

Merged
merged 1 commit into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions packages/runtime-core/__tests__/scheduler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,45 @@ describe('scheduler', () => {
await nextTick()
})

test('jobs can be re-queued after an error', async () => {
const err = new Error('test')
let shouldThrow = true

const job1: SchedulerJob = vi.fn(() => {
if (shouldThrow) {
shouldThrow = false
throw err
}
})
job1.id = 1

const job2: SchedulerJob = vi.fn()
job2.id = 2

queueJob(job1)
queueJob(job2)

try {
await nextTick()
} catch (e: any) {
expect(e).toBe(err)
}
expect(
`Unhandled error during execution of scheduler flush`,
).toHaveBeenWarned()

expect(job1).toHaveBeenCalledTimes(1)
expect(job2).toHaveBeenCalledTimes(0)

queueJob(job1)
queueJob(job2)

await nextTick()

expect(job1).toHaveBeenCalledTimes(2)
expect(job2).toHaveBeenCalledTimes(1)
})

test('should prevent self-triggering jobs by default', async () => {
let count = 0
const job = () => {
Expand Down Expand Up @@ -558,6 +597,113 @@ describe('scheduler', () => {
expect(count).toBe(5)
})

test('recursive jobs can only be queued once non-recursively', async () => {
const job: SchedulerJob = vi.fn()
job.id = 1
job.flags = SchedulerJobFlags.ALLOW_RECURSE

queueJob(job)
queueJob(job)

await nextTick()

expect(job).toHaveBeenCalledTimes(1)
})

test('recursive jobs can only be queued once recursively', async () => {
let recurse = true

const job: SchedulerJob = vi.fn(() => {
if (recurse) {
queueJob(job)
queueJob(job)
recurse = false
}
})
job.id = 1
job.flags = SchedulerJobFlags.ALLOW_RECURSE

queueJob(job)

await nextTick()

expect(job).toHaveBeenCalledTimes(2)
})

test(`recursive jobs can't be re-queued by other jobs`, async () => {
let recurse = true

const job1: SchedulerJob = () => {
if (recurse) {
// job2 is already queued, so this shouldn't do anything
queueJob(job2)
recurse = false
}
}
job1.id = 1

const job2: SchedulerJob = vi.fn(() => {
if (recurse) {
queueJob(job1)
queueJob(job2)
}
})
job2.id = 2
job2.flags = SchedulerJobFlags.ALLOW_RECURSE

queueJob(job2)

await nextTick()

expect(job2).toHaveBeenCalledTimes(2)
})

test('jobs are de-duplicated correctly when calling flushPreFlushCbs', async () => {
let recurse = true

const job1: SchedulerJob = vi.fn(() => {
queueJob(job3)
queueJob(job3)
flushPreFlushCbs()
})
job1.id = 1
job1.flags = SchedulerJobFlags.PRE

const job2: SchedulerJob = vi.fn(() => {
if (recurse) {
// job2 does not allow recurse, so this shouldn't do anything
queueJob(job2)

// job3 is already queued, so this shouldn't do anything
queueJob(job3)
recurse = false
}
})
job2.id = 2
job2.flags = SchedulerJobFlags.PRE

const job3: SchedulerJob = vi.fn(() => {
if (recurse) {
queueJob(job2)
queueJob(job3)

// The jobs are already queued, so these should have no effect
queueJob(job2)
queueJob(job3)
}
})
job3.id = 3
job3.flags = SchedulerJobFlags.ALLOW_RECURSE | SchedulerJobFlags.PRE

queueJob(job1)

await nextTick()

expect(job1).toHaveBeenCalledTimes(1)
expect(job2).toHaveBeenCalledTimes(1)
expect(job3).toHaveBeenCalledTimes(2)
})

// #1947 flushPostFlushCbs should handle nested calls
// e.g. app.mount inside app.mount
test('flushPostFlushCbs', async () => {
Expand Down
8 changes: 6 additions & 2 deletions packages/runtime-core/src/scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ export function flushPreFlushCbs(
cb.flags! &= ~SchedulerJobFlags.QUEUED
}
cb()
cb.flags! &= ~SchedulerJobFlags.QUEUED
if (!(cb.flags! & SchedulerJobFlags.ALLOW_RECURSE)) {
cb.flags! &= ~SchedulerJobFlags.QUEUED
}
}
}
}
Expand Down Expand Up @@ -239,7 +241,9 @@ function flushJobs(seen?: CountMap) {
job.i,
job.i ? ErrorCodes.COMPONENT_UPDATE : ErrorCodes.SCHEDULER,
)
job.flags! &= ~SchedulerJobFlags.QUEUED
if (!(job.flags! & SchedulerJobFlags.ALLOW_RECURSE)) {
job.flags! &= ~SchedulerJobFlags.QUEUED
}
}
}
} finally {
Expand Down