Skip to content

Commit cf531c6

Browse files
Add signal handler for process execution (#1008)
* Add signal handler for process execution * Add default SIGKILL signal to killChildProcess
1 parent 69ea14e commit cf531c6

File tree

2 files changed

+77
-12
lines changed

2 files changed

+77
-12
lines changed

node/test/toolrunnertests.ts

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import * as trm from '../_build/toolrunner';
1212

1313
import testutil = require('./testutil');
1414

15+
const signals: (number | NodeJS.Signals)[] = ['SIGTERM', 'SIGINT', 'SIGKILL', 15, 2, 9];
16+
1517
describe('Toolrunner Tests', function () {
1618

1719
before(function (done) {
@@ -486,7 +488,67 @@ describe('Toolrunner Tests', function () {
486488
fs.unlinkSync(semaphorePath);
487489
delete process.env['TASKLIB_TEST_TOOLRUNNER_EXITDELAY'];
488490
});
489-
})
491+
});
492+
493+
signals.forEach(signal => {
494+
it(`Handle child process killing with ${signal} signal`, function (done) {
495+
this.timeout(10000);
496+
497+
let shell: trm.ToolRunner;
498+
let tool;
499+
if (os.platform() == 'win32') {
500+
tool = tl.which('cmd.exe', true);
501+
shell = tl.tool(tool)
502+
.arg('/D') // Disable execution of AutoRun commands from registry.
503+
.arg('/E:ON') // Enable command extensions. Note, command extensions are enabled by default, unless disabled via registry.
504+
.arg('/V:OFF') // Disable delayed environment expansion. Note, delayed environment expansion is disabled by default, unless enabled via registry.
505+
.arg('/S') // Will cause first and last quote after /C to be stripped.
506+
.arg('/C')
507+
.arg("waitfor 3");
508+
}
509+
else {
510+
tool = tl.which('bash', true);
511+
shell = tl.tool(tool)
512+
.arg('-c')
513+
.arg("sleep 3");
514+
}
515+
516+
let toolRunnerDebug = [];
517+
shell.on('debug', function (data) {
518+
toolRunnerDebug.push(data);
519+
});
520+
521+
let options = <trm.IExecOptions>{
522+
cwd: __dirname,
523+
env: process.env,
524+
silent: false,
525+
failOnStdErr: true,
526+
ignoreReturnCode: false,
527+
outStream: process.stdout,
528+
errStream: process.stdout,
529+
windowsVerbatimArguments: true
530+
};
531+
532+
shell.exec(options)
533+
.then(function () {
534+
done(new Error('should not have been successful'));
535+
done();
536+
})
537+
.catch(function () {
538+
if (typeof signal === 'number') {
539+
signal = Object.keys(os.constants.signals).find(x => os.constants.signals[x] == signal) as NodeJS.Signals;
540+
}
541+
assert(toolRunnerDebug.pop(), `STDIO streams have closed and received exit code null and signal ${signal} for tool '${tool}'`);
542+
done();
543+
})
544+
.catch(function (err) {
545+
done(err);
546+
})
547+
548+
shell.killChildProcess(signal);
549+
});
550+
});
551+
490552
it('Handles child process holding streams open and non-zero exit code', function (done) {
491553
this.timeout(10000);
492554

node/toolrunner.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,9 @@ export class ToolRunner extends events.EventEmitter {
678678
if (fileStream) {
679679
fileStream.write(data);
680680
}
681-
cp.stdin?.write(data);
681+
if (!cp.stdin?.destroyed) {
682+
cp.stdin?.write(data);
683+
}
682684
} catch (err) {
683685
this._debug('Failed to pipe output of ' + toolPathFirst + ' to ' + toolPath);
684686
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 {
11801182
state.CheckComplete();
11811183
});
11821184

1183-
cp.on('exit', (code: number, signal: any) => {
1185+
cp.on('exit', (code: number, signal: number | NodeJS.Signals) => {
11841186
state.processExitCode = code;
11851187
state.processExited = true;
1186-
this._debug(`Exit code ${code} received from tool '${this.toolPath}'`);
1188+
this._debug(`STDIO streams have closed and received exit code ${code} and signal ${signal} for tool '${this.toolPath}'`);
11871189
state.CheckComplete()
11881190
});
11891191

1190-
cp.on('close', (code: number, signal: any) => {
1192+
cp.on('close', (code: number, signal: number | NodeJS.Signals) => {
11911193
state.processExitCode = code;
11921194
state.processExited = true;
11931195
state.processClosed = true;
1194-
this._debug(`STDIO streams have closed for tool '${this.toolPath}'`)
1196+
this._debug(`STDIO streams have closed and received exit code ${code} and signal ${signal} for tool '${this.toolPath}'`);
11951197
state.CheckComplete();
11961198
});
11971199

@@ -1312,18 +1314,18 @@ export class ToolRunner extends events.EventEmitter {
13121314
state.CheckComplete();
13131315
});
13141316

1315-
cp.on('exit', (code: number, signal: any) => {
1317+
cp.on('exit', (code: number, signal: number | NodeJS.Signals) => {
13161318
state.processExitCode = code;
13171319
state.processExited = true;
1318-
this._debug(`Exit code ${code} received from tool '${this.toolPath}'`);
1320+
this._debug(`STDIO streams have closed and received exit code ${code} and signal ${signal} for tool '${this.toolPath}'`);
13191321
state.CheckComplete()
13201322
});
13211323

1322-
cp.on('close', (code: number, signal: any) => {
1324+
cp.on('close', (code: number, signal: number | NodeJS.Signals) => {
13231325
state.processExitCode = code;
13241326
state.processExited = true;
13251327
state.processClosed = true;
1326-
this._debug(`STDIO streams have closed for tool '${this.toolPath}'`)
1328+
this._debug(`STDIO streams have closed and received exit code ${code} and signal ${signal} for tool '${this.toolPath}'`);
13271329
state.CheckComplete();
13281330
});
13291331

@@ -1374,9 +1376,10 @@ export class ToolRunner extends events.EventEmitter {
13741376
* Used to close child process by sending SIGNINT signal.
13751377
* It allows executed script to have some additional logic on SIGINT, before exiting.
13761378
*/
1377-
public killChildProcess(): void {
1379+
public killChildProcess(signal: number | NodeJS.Signals = "SIGTERM"): void {
13781380
if (this.childProcess) {
1379-
this.childProcess.kill();
1381+
this._debug(`[killChildProcess] Signal ${signal} received`);
1382+
this.childProcess.kill(signal);
13801383
}
13811384
}
13821385
}

0 commit comments

Comments
 (0)