Skip to content

Commit ad1e409

Browse files
committed
test(host-kit): a guarded group write answers the way process.kill does
The default mode returned `false` for a write the kernel took, and the comment above it described that as a group with no reachable members. `process.kill` has three answers to a negative pid and none of them is `false`: `true` once the write is accepted, `ESRCH` when no member is left, `EPERM` when a member belongs to someone else. The assertions survived because the seam reads the throw and ignores the return value, so nothing was wrong with the behavior — but the next test written against this helper would copy an answer the kernel never gives, and the comment would keep promising a state the seam cannot observe. The mode is now `delivered` and returns `true`, and `EPERM` is a mode of its own, which is also the test that branch never had: `signalProcessGroupBestEffort` has one catch for both errno values, and only `ESRCH` was ever exercised. Making that catch rethrow `EPERM` turns the new test red and nothing else.
1 parent 882f15c commit ad1e409

1 file changed

Lines changed: 29 additions & 11 deletions

File tree

packages/host-kit/src/internal/exec-kill-settle.test.ts

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -166,17 +166,17 @@ test.runIf(process.platform !== 'win32')(
166166
// question the callers answer, so both report to one settlement.
167167
//
168168
// The kill paths below address a process group whose leader this worker already reaped, and
169-
// the hermetic signal setup ends a worker's authority over a pid at that moment. So every
170-
// group write is answered by `guardGroupWrites` below, which is the seam that setup points a real
171-
// kill path at: it records what the kill aimed at and answers the way a real group would, either
172-
// a delivery nothing was reached for or the `ESRCH` a vanished group throws.
169+
// the hermetic signal setup ends a worker's authority over a pid at that moment. So every group
170+
// write is answered by `guardGroupWrites` below, which is the seam that setup points a real kill
171+
// path at: it records what the kill aimed at and answers the way `process.kill` does — `true` for a
172+
// write the kernel accepted, `ESRCH` for a group that is gone, `EPERM` for one that is not ours.
173173

174174
type GroupWrite = { readonly pid: number; readonly signal: string | number };
175175

176-
/** How a guarded group write answers, matching what a real group would do. */
177-
type GroupWriteAnswer = 'no-group-reached' | 'no-such-process';
176+
/** How a guarded group write answers, matching what `process.kill` does with a negative pid. */
177+
type GroupWriteAnswer = 'delivered' | 'no-such-process' | 'not-permitted';
178178

179-
function guardGroupWrites(answer: GroupWriteAnswer = 'no-group-reached'): {
179+
function guardGroupWrites(answer: GroupWriteAnswer = 'delivered'): {
180180
restore: () => void;
181181
writes: GroupWrite[];
182182
} {
@@ -185,12 +185,14 @@ function guardGroupWrites(answer: GroupWriteAnswer = 'no-group-reached'): {
185185
process.kill = ((pid: number, signal: string | number = 'SIGTERM') => {
186186
if (pid < 0) {
187187
writes.push({ pid, signal });
188-
if (answer === 'no-such-process') {
189-
const error = new Error('no such process') as NodeJS.ErrnoException;
190-
error.code = 'ESRCH';
188+
if (answer === 'no-such-process' || answer === 'not-permitted') {
189+
const error = new Error(
190+
answer === 'no-such-process' ? 'no such process' : 'operation not permitted',
191+
) as NodeJS.ErrnoException;
192+
error.code = answer === 'no-such-process' ? 'ESRCH' : 'EPERM';
191193
throw error;
192194
}
193-
return false;
195+
return true;
194196
}
195197
return original(pid, signal as NodeJS.Signals);
196198
}) as typeof process.kill;
@@ -231,6 +233,22 @@ test('group signaling reports a vanished group and never signals an invalid pid'
231233
}
232234
});
233235

236+
test('a group that is gone and a group that is not ours to signal both report nothing reached', () => {
237+
// `process.kill` answers a negative pid in exactly three ways: `true` once the kernel accepted the
238+
// write, `ESRCH` when no member is left, and `EPERM` when a member belongs to another user. The
239+
// second and third are the same answer to this seam — nothing was reached, so the caller must not
240+
// keep waiting on a pipe holder it just asked to be killed — and only the first of them was tested.
241+
for (const answer of ['no-such-process', 'not-permitted'] as const) {
242+
const groupWrites = guardGroupWrites(answer);
243+
try {
244+
assert.equal(signalProcessGroupBestEffort(101, 'SIGKILL'), false);
245+
assert.deepEqual(groupWrites.writes, [{ pid: -101, signal: 'SIGKILL' }]);
246+
} finally {
247+
groupWrites.restore();
248+
}
249+
}
250+
});
251+
234252
test.runIf(process.platform !== 'win32')(
235253
'a detached deadline still kills the group its reaped child left behind',
236254
async () => {

0 commit comments

Comments
 (0)