Skip to content

Commit

Permalink
Refactor execute command cancellation
Browse files Browse the repository at this point in the history
  • Loading branch information
Or-Geva committed Nov 27, 2023
1 parent 695a5c0 commit ed787b5
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 16 deletions.
10 changes: 2 additions & 8 deletions src/main/scanLogic/scanRunners/analyzerManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,9 @@ export class AnalyzerManager {
* @param args - the arguments for the command
* @param executionLogDirectory - the directory to save the execution log in
*/
public async run(args: string[], executionLogDirectory?: string): Promise<any> {
public async run(args: string[], checkCancel: () => void, executionLogDirectory?: string): Promise<void> {
await AnalyzerManager.FINISH_UPDATE_PROMISE;
let std: any = await this._binary.run(args, this.createEnvForRun(executionLogDirectory));
if (std.stdout && std.stdout.length > 0) {
this._logManager.logMessage('Done executing with log, log:\n' + std.stdout, 'DEBUG');
}
if (std.stderr && std.stderr.length > 0) {
this._logManager.logMessage('Done executing with log, log:\n' + std.stderr, 'ERR');
}
await this._binary.run(args, checkCancel, this.createEnvForRun(executionLogDirectory));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/main/scanLogic/scanRunners/jasRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export abstract class JasRunner {
*/
protected async runAnalyzerManager(checkCancel: () => void, args: string[], executionLogDirectory?: string): Promise<void> {
checkCancel();
await this._analyzerManager.run(args, executionLogDirectory);
await this._analyzerManager.run(args, checkCancel, executionLogDirectory);
}

protected logStartScanning(request: AnalyzeScanRequest): void {
Expand Down
12 changes: 9 additions & 3 deletions src/main/utils/resource.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fs from 'fs';
import * as path from 'path';

import * as exec from 'child_process';
import { IChecksumResult, JfrogClient } from 'jfrog-client-js';
import { LogManager } from '../log/logManager';
import { Utils } from './utils';
Expand Down Expand Up @@ -155,10 +155,16 @@ export class Resource {
return ScanUtils.Hash('SHA256', fileBuffer);
}

public async run(args: string[], env?: NodeJS.ProcessEnv | undefined): Promise<any> {
public async run(args: string[], checkCancel: () => void, env?: NodeJS.ProcessEnv | undefined): Promise<void> {
let command: string = '"' + this.fullPath + '" ' + args.join(' ');
this._logManager.debug("Executing '" + command + "' in directory '" + this._targetDir + "'");
return await ScanUtils.executeCmdAsync(command, this._targetDir, env);
const child: exec.ChildProcess = await ScanUtils.executeCmdAsync(command, checkCancel, this._targetDir, env);
if (child.stdout) {
this._logManager.logMessage('Done executing with log, log:\n' + child.stdout, 'DEBUG');
}
if (child.stderr) {
this._logManager.logMessage('Done executing with log, log:\n' + child.stderr, 'ERR');
}
}

public get fullPath(): string {
Expand Down
44 changes: 40 additions & 4 deletions src/main/utils/scanUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import * as fse from 'fs-extra';
import * as os from 'os';
import * as path from 'path';
import * as tmp from 'tmp';
import * as util from 'util';
import * as vscode from 'vscode';
import { ContextKeys } from '../constants/contextKeys';
import { LogManager } from '../log/logManager';
Expand All @@ -21,6 +20,8 @@ export class ScanUtils {

public static readonly RESOURCES_DIR: string = ScanUtils.getResourcesDir();
public static readonly SPAWN_PROCESS_BUFFER_SIZE: number = 104857600;
// every 0.1 sec
private static readonly CANCELLATION_CHECK_INTERVAL_MS: number = 100;

public static async scanWithProgress(
scanCbk: (progress: vscode.Progress<{ message?: string; increment?: number }>, checkCanceled: () => void) => Promise<void>,
Expand Down Expand Up @@ -158,8 +159,43 @@ export class ScanUtils {
return exec.execSync(command, { cwd: cwd, maxBuffer: ScanUtils.SPAWN_PROCESS_BUFFER_SIZE, env: env });
}

public static async executeCmdAsync(command: string, cwd?: string, env?: NodeJS.ProcessEnv | undefined): Promise<any> {
return await util.promisify(exec.exec)(command, { cwd: cwd, maxBuffer: ScanUtils.SPAWN_PROCESS_BUFFER_SIZE, env: env });
public static async executeCmdAsync(
command: string,
checkCancel: () => void,
cwd?: string,
env?: NodeJS.ProcessEnv | undefined
): Promise<exec.ChildProcess> {
return new Promise<exec.ChildProcess>((resolve, reject) => {
const childProcess: exec.ChildProcess = exec.exec(command, { cwd: cwd, maxBuffer: ScanUtils.SPAWN_PROCESS_BUFFER_SIZE, env: env });
const checkCancellationInterval: NodeJS.Timer = setInterval(() => {
ScanUtils.cancelProcess(childProcess, checkCancellationInterval, checkCancel, reject);
}, ScanUtils.CANCELLATION_CHECK_INTERVAL_MS);

childProcess.on('exit', code => {
clearInterval(checkCancellationInterval);
code === 0 ? resolve(childProcess) : reject(new Error(`Process exited with code ${code}`));
});

childProcess.on('error', err => {
clearInterval(checkCancellationInterval);
reject(err);
});
});
}

public static cancelProcess(
childProcess: exec.ChildProcess,
checkCancellationInterval: NodeJS.Timer,
checkCancel: () => void,
reject: (reason?: any) => void
): void {
try {
checkCancel();
} catch (error) {
clearInterval(checkCancellationInterval);
childProcess.kill('SIGTERM');
reject(error);
}
}

public static setScanInProgress(state: boolean) {
Expand Down Expand Up @@ -300,4 +336,4 @@ export class FileScanError extends Error {

export class ScanCancellationError extends Error {
message: string = 'Scan was cancelled';
}
}
83 changes: 83 additions & 0 deletions src/test/tests/scanUtils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { assert, expect } from 'chai';
import * as exec from 'child_process';
import { ScanUtils } from '../../main/utils/scanUtils';
import sinon from 'sinon';

describe('ScanUtils', () => {
describe('executeCmdAsync', () => {
let clearIntervalSpy: sinon.SinonSpy;
beforeEach(() => {
clearIntervalSpy = sinon.spy(global, 'clearInterval');
});

afterEach(() => {
clearIntervalSpy.restore();
});

const dummyCheckError: () => void = () => {
return;
};
it('should execute a command successfully', async () => {
const command: string = 'echo Hello, World!';
const result: exec.ChildProcess | undefined = await ScanUtils.executeCmdAsync(command, () => dummyCheckError, undefined, undefined);
assert.instanceOf(result, exec.ChildProcess);
assert.isTrue(clearIntervalSpy.calledOnce);
});

it('should reject with an error if the command fails', async () => {
const command: string = 'invalid_command';
try {
await ScanUtils.executeCmdAsync(command, dummyCheckError, undefined, undefined);
// If the above line doesn't throw an error, the test should fail
expect.fail('The command should have failed.');
} catch (error) {
assert.instanceOf(error, Error);
assert.isTrue(clearIntervalSpy.calledOnce);
}
});

it('should reject with a cancellation error if canceled', async () => {
const cancelSignal: () => void = () => {
throw new Error('Cancellation requested.');
};

try {
await ScanUtils.executeCmdAsync('sleep 2', cancelSignal, undefined, undefined);
// If the above line doesn't throw an error, the test should fail
expect.fail('The command should have been canceled.');
} catch (error) {
if (error instanceof Error) {
assert.equal(error.message, 'Cancellation requested.');
} else {
assert.fail('The error should have been an instance of Error.');
}
}
});

describe('cancelProcess', () => {
it('should call childProcess.kill when cancellation is requested', () => {
// Arrange
const killStub: sinon.SinonStub<any[], any> = sinon.stub();
const fakeChildProcess: Partial<exec.ChildProcess> = {
kill: killStub
};

const checkCancellation: () => never = () => {
throw new Error('Cancellation requested.');
};

// Act
// Simulating an interval, actual logic not included for this test
const checkCancellationInterval: NodeJS.Timer = setInterval(() => {
return;
}, 100);
ScanUtils.cancelProcess(fakeChildProcess as exec.ChildProcess, checkCancellationInterval, checkCancellation, () => {
return;
});

// Assertions
assert.isTrue(killStub.calledOnceWithExactly('SIGTERM'));
});
});
});
});

0 comments on commit ed787b5

Please sign in to comment.