Add signal handler for process execution feature flag (FF) for enabli…

…ng kill child process
microsoft · Jun 3, 2024 · 8f8dc8f · 8f8dc8f
1 parent 7c0de0d
commit 8f8dc8f
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 7 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', 15, 2];
+
 describe('Toolrunner Tests', function () {
 
     before(function (done) {
@@ -487,6 +489,74 @@ describe('Toolrunner Tests', function () {
                 delete process.env['TASKLIB_TEST_TOOLRUNNER_EXITDELAY'];
             });
     })
+    signals.forEach(signal => {
+        it(`Handle child process killing with ${signal} signal`, function (done) {
+            this.timeout(10000);
+
+            let semaphorePath = path.join(testutil.getTestTemp(), 'child-process-semaphore.txt');
+            fs.writeFileSync(semaphorePath, '');
+
+            let nodePath = tl.which('node', true);
+            let scriptPath = path.join(__dirname, 'scripts', 'wait-for-file.js');
+            let shell: trm.ToolRunner;
+            if (os.platform() == 'win32') {
+                shell = tl.tool(tl.which('cmd.exe', true))
+                    .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(`"start "" /B "${nodePath}" "${scriptPath}" "file=${semaphorePath}""`);
+            }
+            else {
+                shell = tl.tool(tl.which('bash', true))
+                    .arg('-c')
+                    .arg(`node '${scriptPath}' 'file=${semaphorePath}' &`);
+            }
+
+            let toolRunnerDebug = [];
+            shell.on('debug', function (data) {
+                toolRunnerDebug.push(data);
+            });
+
+            process.env['TASKLIB_TEST_TOOLRUNNER_EXITDELAY'] = "500"; // 0.5 seconds
+
+            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 (err) {
+                    if (typeof signal === 'number') {
+                        const convertedSignal = Object.keys(os.constants.signals).find(x => os.constants.signals[x] == signal);
+                        assert(toolRunnerDebug.filter(x => x.indexOf(`Signal ${convertedSignal} received from tool`) >= 0).length > 0);
+                    } else {
+                        assert(toolRunnerDebug.filter(x => x.indexOf(`Signal ${signal} received from tool`) >= 0).length > 0);
+                    }
+                    done();
+                })
+                .catch(function (err) {
+                    done(err);
+                })
+                .finally(function () {
+                    fs.unlinkSync(semaphorePath);
+                    delete process.env['TASKLIB_TEST_TOOLRUNNER_EXITDELAY'];
+                });
+
+            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
@@ -1180,18 +1180,20 @@ 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(`Signal ${signal} received from 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(`Signal ${signal} received from tool '${this.toolPath}'`);
             state.CheckComplete();
         });
 
@@ -1312,18 +1314,20 @@ 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(`Signal ${signal} received from 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 for tool '${this.toolPath}'`);
+            this._debug(`Signal ${signal} received from tool '${this.toolPath}'`);
             state.CheckComplete();
         });
 
@@ -1374,9 +1378,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 = "SIGINT"): void {
         if (this.childProcess) {
-            this.childProcess.kill();
+            this._debug(`[killChildProcess] Signal ${signal} received`);
+            this.childProcess.kill(signal);
         }
     }
 }