Add signal handler for process execution (#1008)

* Add signal handler for process execution * Add default SIGKILL signal to killChildProcess
microsoft · Aug 19, 2024 · cf531c6 · cf531c6
1 parent 69ea14e
commit cf531c6
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 12 deletions.
diff --git 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 = <trm.IExecOptions>{
+                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
@@ -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);
         }
     }
 }