Skip to content

Commit e7fd6ef

Browse files
committed
fix(sandbox): self-exit on failed init and lock in error-key handling
1 parent b5e5275 commit e7fd6ef

3 files changed

Lines changed: 63 additions & 3 deletions

File tree

src/classes/child-pool.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,12 @@ export class ChildPool {
7171
// be released back into the free pool, otherwise it becomes a "zombie"
7272
// that is reused for every subsequent job and fails them instantly.
7373
// Kill and remove it so a fresh child is forked on the next retain.
74-
this.kill(child, 'SIGKILL').catch(() => {});
74+
// The child also exits itself after a failed init (see ChildProcessor),
75+
// so this is normally a no-op; log any kill failure instead of silently
76+
// swallowing it so a lingering child would not go unnoticed.
77+
this.kill(child, 'SIGKILL').catch(killErr => {
78+
console.error('Failed to kill child after init error:', killErr);
79+
});
7580
throw err;
7681
}
7782
}

src/classes/child-processor.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,18 @@ export class ChildProcessor {
5252
}
5353
} catch (err) {
5454
this.status = ChildStatus.Errored;
55-
return this.send({
55+
await this.send({
5656
cmd: ParentCommand.InitFailed,
5757
err: errorToJSON(err),
5858
});
59+
// A child that failed to initialize cannot recover, and because the open
60+
// IPC channel keeps its event loop alive it would never exit on its own.
61+
// Exit explicitly (after the InitFailed message has been flushed) so the
62+
// parent can never reuse a half-initialized "zombie" child. This is a
63+
// belt-and-braces measure: ChildPool also kills the child, but exiting
64+
// here guarantees termination even if the parent-side kill were to fail.
65+
// In a worker thread this stops only the current worker, not the process.
66+
process.exit(process.exitCode || 0);
5967
}
6068

6169
const origProcessor = processor;

tests/sandboxed_process.test.ts

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { pathToFileURL } from 'url';
22
import { default as IORedis } from 'ioredis';
33
import { after } from 'lodash';
4+
import { EventEmitter } from 'events';
45
import {
56
describe,
67
beforeEach,
@@ -20,8 +21,15 @@ import {
2021
UNRECOVERABLE_ERROR,
2122
Worker,
2223
} from '../src/classes';
24+
import sandbox from '../src/classes/sandbox';
25+
import { ParentCommand } from '../src/enums';
2326

24-
import { delay, randomUUID, removeAllQueueData } from '../src/utils';
27+
import {
28+
delay,
29+
errorToJSON,
30+
randomUUID,
31+
removeAllQueueData,
32+
} from '../src/utils';
2533
import { existsSync, unlinkSync, writeFileSync } from 'fs';
2634
const { stdout, stderr } = require('test-console');
2735

@@ -229,6 +237,45 @@ describe('Sandboxed process using worker threads', () => {
229237
});
230238
});
231239

240+
describe('Sandbox error message handling', () => {
241+
// A child that refuses a Start (e.g. 'cannot start a not idling child
242+
// process') reports the reason via ParentCommand.Error, whose payload is
243+
// carried under the `err` key — unlike ParentCommand.Failed which uses
244+
// `value`. The sandbox message handler must read from either key so the
245+
// reason is never lost as an empty-message error. This guards the
246+
// `msg.value ?? msg.err` fix independently of the child lifecycle, since the
247+
// refusal path is otherwise hard to reach once init-failed children exit.
248+
it('propagates the reason when ParentCommand.Error uses the `err` key', async () => {
249+
const reason = 'cannot start a not idling child process';
250+
251+
const fakeChild: any = new EventEmitter();
252+
fakeChild.exitCode = null;
253+
fakeChild.signalCode = null;
254+
fakeChild.processFile = 'fake-process-file';
255+
fakeChild.pid = 1;
256+
fakeChild.send = () => {
257+
// Simulate the child refusing the Start command and reporting the reason
258+
// under `err` (ParentCommand.Error), not `value` (ParentCommand.Failed).
259+
queueMicrotask(() => {
260+
fakeChild.emit('message', {
261+
cmd: ParentCommand.Error,
262+
err: errorToJSON(new Error(reason)),
263+
});
264+
});
265+
};
266+
267+
const fakeChildPool: any = {
268+
retain: async () => fakeChild,
269+
release: () => {},
270+
};
271+
272+
const processFn = sandbox('fake-process-file', fakeChildPool);
273+
const fakeJob: any = { asJSONSandbox: () => ({}) };
274+
275+
await expect(processFn(fakeJob)).rejects.toThrow(reason);
276+
});
277+
});
278+
232279
function sandboxProcessTests(
233280
{ useWorkerThreads } = { useWorkerThreads: false },
234281
) {

0 commit comments

Comments
 (0)