From cf531c6169f90cec5a648c7f5935a84f3ae8a763 Mon Sep 17 00:00:00 2001 From: Ivan Duplenskikh <115665590+ivanduplenskikh@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:46:34 +0200 Subject: [PATCH] Add signal handler for process execution (#1008) * Add signal handler for process execution * Add default SIGKILL signal to killChildProcess --- node/test/toolrunnertests.ts | 64 +++++++++++++++++++++++++++++++++++- node/toolrunner.ts | 25 +++++++------- 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/node/test/toolrunnertests.ts b/node/test/toolrunnertests.ts index e67327a6f..1b6ecc4c4 100644 --- a/node/test/toolrunnertests.ts +++ b/node/test/toolrunnertests.ts @@ -12,6 +12,8 @@ import * as trm from '../_build/toolrunner'; import testutil = require('./testutil'); +const signals: (number | NodeJS.Signals)[] = ['SIGTERM', 'SIGINT', 'SIGKILL', 15, 2, 9]; + describe('Toolrunner Tests', function () { before(function (done) { @@ -486,7 +488,67 @@ describe('Toolrunner Tests', function () { fs.unlinkSync(semaphorePath); delete process.env['TASKLIB_TEST_TOOLRUNNER_EXITDELAY']; }); - }) + }); + + signals.forEach(signal => { + it(`Handle child process killing with ${signal} signal`, function (done) { + this.timeout(10000); + + let shell: trm.ToolRunner; + let tool; + if (os.platform() == 'win32') { + tool = tl.which('cmd.exe', true); + shell = tl.tool(tool) + .arg('/D') // Disable execution of AutoRun commands from registry. + .arg('/E:ON') // Enable command extensions. Note, command extensions are enabled by default, unless disabled via registry. + .arg('/V:OFF') // Disable delayed environment expansion. Note, delayed environment expansion is disabled by default, unless enabled via registry. + .arg('/S') // Will cause first and last quote after /C to be stripped. + .arg('/C') + .arg("waitfor 3"); + } + else { + tool = tl.which('bash', true); + shell = tl.tool(tool) + .arg('-c') + .arg("sleep 3"); + } + + let toolRunnerDebug = []; + shell.on('debug', function (data) { + toolRunnerDebug.push(data); + }); + + let options = { + cwd: __dirname, + env: process.env, + silent: false, + failOnStdErr: true, + ignoreReturnCode: false, + outStream: process.stdout, + errStream: process.stdout, + windowsVerbatimArguments: true + }; + + shell.exec(options) + .then(function () { + done(new Error('should not have been successful')); + done(); + }) + .catch(function () { + if (typeof signal === 'number') { + signal = Object.keys(os.constants.signals).find(x => os.constants.signals[x] == signal) as NodeJS.Signals; + } + assert(toolRunnerDebug.pop(), `STDIO streams have closed and received exit code null and signal ${signal} for tool '${tool}'`); + done(); + }) + .catch(function (err) { + done(err); + }) + + shell.killChildProcess(signal); + }); + }); + it('Handles child process holding streams open and non-zero exit code', function (done) { this.timeout(10000); diff --git a/node/toolrunner.ts b/node/toolrunner.ts index 48dcc2b73..6d01fa384 100644 --- a/node/toolrunner.ts +++ b/node/toolrunner.ts @@ -678,7 +678,9 @@ export class ToolRunner extends events.EventEmitter { if (fileStream) { fileStream.write(data); } - cp.stdin?.write(data); + if (!cp.stdin?.destroyed) { + cp.stdin?.write(data); + } } catch (err) { this._debug('Failed to pipe output of ' + toolPathFirst + ' to ' + toolPath); this._debug(toolPath + ' might have exited due to errors prematurely. Verify the arguments passed are valid.'); @@ -1180,18 +1182,18 @@ export class ToolRunner extends events.EventEmitter { state.CheckComplete(); }); - cp.on('exit', (code: number, signal: any) => { + cp.on('exit', (code: number, signal: number | NodeJS.Signals) => { state.processExitCode = code; state.processExited = true; - this._debug(`Exit code ${code} received from tool '${this.toolPath}'`); + this._debug(`STDIO streams have closed and received exit code ${code} and signal ${signal} for tool '${this.toolPath}'`); state.CheckComplete() }); - cp.on('close', (code: number, signal: any) => { + cp.on('close', (code: number, signal: number | NodeJS.Signals) => { state.processExitCode = code; state.processExited = true; state.processClosed = true; - this._debug(`STDIO streams have closed for tool '${this.toolPath}'`) + this._debug(`STDIO streams have closed and received exit code ${code} and signal ${signal} for tool '${this.toolPath}'`); state.CheckComplete(); }); @@ -1312,18 +1314,18 @@ export class ToolRunner extends events.EventEmitter { state.CheckComplete(); }); - cp.on('exit', (code: number, signal: any) => { + cp.on('exit', (code: number, signal: number | NodeJS.Signals) => { state.processExitCode = code; state.processExited = true; - this._debug(`Exit code ${code} received from tool '${this.toolPath}'`); + this._debug(`STDIO streams have closed and received exit code ${code} and signal ${signal} for tool '${this.toolPath}'`); state.CheckComplete() }); - cp.on('close', (code: number, signal: any) => { + cp.on('close', (code: number, signal: number | NodeJS.Signals) => { state.processExitCode = code; state.processExited = true; state.processClosed = true; - this._debug(`STDIO streams have closed for tool '${this.toolPath}'`) + this._debug(`STDIO streams have closed and received exit code ${code} and signal ${signal} for tool '${this.toolPath}'`); state.CheckComplete(); }); @@ -1374,9 +1376,10 @@ export class ToolRunner extends events.EventEmitter { * Used to close child process by sending SIGNINT signal. * It allows executed script to have some additional logic on SIGINT, before exiting. */ - public killChildProcess(): void { + public killChildProcess(signal: number | NodeJS.Signals = "SIGTERM"): void { if (this.childProcess) { - this.childProcess.kill(); + this._debug(`[killChildProcess] Signal ${signal} received`); + this.childProcess.kill(signal); } } }