Skip to content

Commit bc92aff

Browse files
committedJun 8, 2022
fix: prevent possible race condition when killing tasks on failure
1 parent cb8a432 commit bc92aff

File tree

2 files changed

+35
-8
lines changed

2 files changed

+35
-8
lines changed
 

‎lib/resolveTaskFn.js

+10-8
Original file line numberDiff line numberDiff line change
@@ -71,23 +71,25 @@ const killExecaProcess = async (execaProcess) => {
7171
*
7272
* @param {Object} ctx
7373
* @param {execa.ExecaChildProcess<string>} execaChildProcess
74-
* @returns {() => void} Function that clears the interval that
74+
* @returns {() => Promise<void>} Function that clears the interval that
7575
* checks the context.
7676
*/
7777
const interruptExecutionOnError = (ctx, execaChildProcess) => {
78-
let loopIntervalId
78+
let intervalId, killPromise
7979

8080
const loop = async () => {
8181
if (ctx.errors.size > 0) {
82-
clearInterval(loopIntervalId)
83-
await killExecaProcess(execaChildProcess)
82+
clearInterval(intervalId)
83+
killPromise = killExecaProcess(execaChildProcess)
84+
await killPromise
8485
}
8586
}
8687

87-
loopIntervalId = setInterval(loop, ERROR_CHECK_INTERVAL)
88+
intervalId = setInterval(loop, ERROR_CHECK_INTERVAL)
8889

89-
return () => {
90-
clearInterval(loopIntervalId)
90+
return async () => {
91+
clearInterval(intervalId)
92+
await killPromise
9193
}
9294
}
9395

@@ -155,7 +157,7 @@ export const resolveTaskFn = ({
155157

156158
const quitInterruptCheck = interruptExecutionOnError(ctx, execaChildProcess)
157159
const result = await execaChildProcess
158-
quitInterruptCheck()
160+
await quitInterruptCheck()
159161

160162
if (result.failed || result.killed || result.signal != null) {
161163
throw makeErr(command, result, ctx)

‎test/resolveTaskFn.spec.js

+25
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,31 @@ describe('resolveTaskFn', () => {
333333
`)
334334
})
335335

336+
it('should not kill long running tasks without errors in context', async () => {
337+
execa.mockImplementationOnce(() =>
338+
createExecaReturnValue(
339+
{
340+
stdout: 'a-ok',
341+
stderr: '',
342+
code: 0,
343+
cmd: 'mock cmd',
344+
failed: false,
345+
killed: false,
346+
signal: null,
347+
},
348+
1000
349+
)
350+
)
351+
352+
const context = getInitialState()
353+
const taskFn = resolveTaskFn({ command: 'node' })
354+
const taskPromise = taskFn(context)
355+
356+
jest.runOnlyPendingTimers()
357+
358+
await expect(taskPromise).resolves.toEqual()
359+
})
360+
336361
it('should kill a long running task when an error is added to the context', async () => {
337362
execa.mockImplementationOnce(() =>
338363
createExecaReturnValue(

0 commit comments

Comments
 (0)
Please sign in to comment.