Skip to content

Commit

Permalink
fix: prevent possible race condition when killing tasks on failure
Browse files Browse the repository at this point in the history
  • Loading branch information
iiroj committed Jun 8, 2022
1 parent cb8a432 commit bc92aff
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 8 deletions.
18 changes: 10 additions & 8 deletions lib/resolveTaskFn.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,23 +71,25 @@ const killExecaProcess = async (execaProcess) => {
*
* @param {Object} ctx
* @param {execa.ExecaChildProcess<string>} execaChildProcess
* @returns {() => void} Function that clears the interval that
* @returns {() => Promise<void>} Function that clears the interval that
* checks the context.
*/
const interruptExecutionOnError = (ctx, execaChildProcess) => {
let loopIntervalId
let intervalId, killPromise

const loop = async () => {
if (ctx.errors.size > 0) {
clearInterval(loopIntervalId)
await killExecaProcess(execaChildProcess)
clearInterval(intervalId)
killPromise = killExecaProcess(execaChildProcess)
await killPromise
}
}

loopIntervalId = setInterval(loop, ERROR_CHECK_INTERVAL)
intervalId = setInterval(loop, ERROR_CHECK_INTERVAL)

return () => {
clearInterval(loopIntervalId)
return async () => {
clearInterval(intervalId)
await killPromise
}
}

Expand Down Expand Up @@ -155,7 +157,7 @@ export const resolveTaskFn = ({

const quitInterruptCheck = interruptExecutionOnError(ctx, execaChildProcess)
const result = await execaChildProcess
quitInterruptCheck()
await quitInterruptCheck()

if (result.failed || result.killed || result.signal != null) {
throw makeErr(command, result, ctx)
Expand Down
25 changes: 25 additions & 0 deletions test/resolveTaskFn.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,31 @@ describe('resolveTaskFn', () => {
`)
})

it('should not kill long running tasks without errors in context', async () => {
execa.mockImplementationOnce(() =>
createExecaReturnValue(
{
stdout: 'a-ok',
stderr: '',
code: 0,
cmd: 'mock cmd',
failed: false,
killed: false,
signal: null,
},
1000
)
)

const context = getInitialState()
const taskFn = resolveTaskFn({ command: 'node' })
const taskPromise = taskFn(context)

jest.runOnlyPendingTimers()

await expect(taskPromise).resolves.toEqual()
})

it('should kill a long running task when an error is added to the context', async () => {
execa.mockImplementationOnce(() =>
createExecaReturnValue(
Expand Down

0 comments on commit bc92aff

Please sign in to comment.