Skip to content

Commit

Permalink
feat(util/exec): enable process group handling on process termination (
Browse files Browse the repository at this point in the history
  • Loading branch information
Gabriel-Ladzaretti committed Aug 28, 2022
1 parent dafda2e commit 21ab4ba
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 17 deletions.
4 changes: 4 additions & 0 deletions docs/usage/self-hosted-experimental.md
Expand Up @@ -58,3 +58,7 @@ If set, Renovate will enable `forcePathStyle` when instantiating the AWS s3 clie
> Whether to force path style URLs for S3 objects (e.g., `https://s3.amazonaws.com//` instead of `https://.s3.amazonaws.com/`
Source: [AWS s3 documentation - Interface BucketEndpointInputConfig](https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/clients/client-s3/interfaces/bucketendpointinputconfig.html)

## `RENOVATE_X_EXEC_GPID_HANDLE`

If set, Renovate will terminate the whole process group of a terminated child process spawned by Renovate.
47 changes: 47 additions & 0 deletions lib/util/exec/common.spec.ts
Expand Up @@ -33,6 +33,7 @@ interface StubArgs {
stdout?: string;
stderr?: string;
timeout?: number;
pid?: number;
}

function getReadable(
Expand Down Expand Up @@ -66,6 +67,7 @@ function getSpawnStub(args: StubArgs): ChildProcess {
stderr,
encoding,
timeout,
pid = 31415,
} = args;
const listeners: Events = {};

Expand Down Expand Up @@ -140,6 +142,7 @@ function getSpawnStub(args: StubArgs): ChildProcess {
emit,
unref,
kill,
pid,
} as ChildProcess;
}

Expand Down Expand Up @@ -282,4 +285,48 @@ describe('util/exec/common', () => {
});
});
});

describe('handle gpid', () => {
const killSpy = jest.spyOn(process, 'kill');

afterEach(() => {
delete process.env.RENOVATE_X_EXEC_GPID_HANDLE;
jest.restoreAllMocks();
});

it('calls process.kill on the gpid', async () => {
process.env.RENOVATE_X_EXEC_GPID_HANDLE = 'true';
const cmd = 'ls -l';
const exitSignal = 'SIGTERM';
const stub = getSpawnStub({ cmd, exitCode: null, exitSignal });
spawn.mockImplementationOnce((cmd, opts) => stub);
killSpy.mockImplementationOnce((pid, signal) => true);
await expect(
exec(cmd, partial<RawExecOptions>({ encoding: 'utf8' }))
).rejects.toMatchObject({
cmd,
signal: exitSignal,
message: `Command failed: ${cmd}\nInterrupted by ${exitSignal}`,
});
expect(process.kill).toHaveBeenCalledWith(-stub.pid!, exitSignal);
});

it('handles process.kill call on non existent gpid', async () => {
process.env.RENOVATE_X_EXEC_GPID_HANDLE = 'true';
const cmd = 'ls -l';
const exitSignal = 'SIGTERM';
const stub = getSpawnStub({ cmd, exitCode: null, exitSignal });
spawn.mockImplementationOnce((cmd, opts) => stub);
killSpy.mockImplementationOnce((pid, signal) => {
throw new Error();
});
await expect(
exec(cmd, partial<RawExecOptions>({ encoding: 'utf8' }))
).rejects.toMatchObject({
cmd,
signal: exitSignal,
message: `Command failed: ${cmd}\nInterrupted by ${exitSignal}`,
});
});
});
});
34 changes: 17 additions & 17 deletions lib/util/exec/common.ts
Expand Up @@ -122,25 +122,25 @@ export function exec(cmd: string, opts: RawExecOptions): Promise<ExecResult> {

function kill(cp: ChildProcess, signal: NodeJS.Signals): boolean {
try {
// TODO: will be enabled in #16654
/**
* If `pid` is negative, but not `-1`, signal shall be sent to all processes
* (excluding an unspecified set of system processes),
* whose process group ID (pgid) is equal to the absolute value of pid,
* and for which the process has permission to send a signal.
*/
// process.kill(-(cp.pid as number), signal);

// destroying stdio is needed for unref to work
// https://nodejs.org/api/child_process.html#subprocessunref
// https://github.com/nodejs/node/blob/4d5ff25a813fd18939c9f76b17e36291e3ea15c3/lib/child_process.js#L412-L426
cp.stderr?.destroy();
cp.stdout?.destroy();
cp.unref();
return cp.kill(signal);
if (cp.pid && process.env.RENOVATE_X_EXEC_GPID_HANDLE) {
/**
* If `pid` is negative, but not `-1`, signal shall be sent to all processes
* (excluding an unspecified set of system processes),
* whose process group ID (pgid) is equal to the absolute value of pid,
* and for which the process has permission to send a signal.
*/
return process.kill(-cp.pid, signal);
} else {
// destroying stdio is needed for unref to work
// https://nodejs.org/api/child_process.html#subprocessunref
// https://github.com/nodejs/node/blob/4d5ff25a813fd18939c9f76b17e36291e3ea15c3/lib/child_process.js#L412-L426
cp.stderr?.destroy();
cp.stdout?.destroy();
cp.unref();
return cp.kill(signal);
}
} catch (err) {
// cp is a single node tree, therefore -pid is invalid as there is no such pgid,
// istanbul ignore next: will be covered once we use process.kill
return false;
}
}
Expand Down

0 comments on commit 21ab4ba

Please sign in to comment.