Skip to content

Commit 366b569

Browse files
Make testRunawayProcess() deterministic (#295)
On cancellation, the test's teardown sent the child `SIGINT`, escalating to `SIGKILL` after 100ms; the `INT` trap was meant to kill the runaway grandchild and `exit 0`, and the child otherwise blocked in `wait "$child_pid"`. It failed intermittently in two ways, both from trapping an asynchronous signal around a blocking `wait`. First, `signaled(11)`: bash interrupted the `wait` for `SIGINT` via `siglongjmp`, and on Darwin's system bash (3.2, arm64e) that `longjmp` occasionally failed pointer authentication and crashed the child with `SIGSEGV`. Second, `signaled(9)`: if the signal landed after bash last checked pending traps, but before it blocked in `wait`, the trap was recorded but never run. A `wait` on a child that never exited offers no further safe point, so the child lived until teardown escalated to `SIGKILL`. Send `SIGTERM` instead. Its trap runs at a deferred safe point rather than `longjmp`-ing out of the handler, so the crash cannot occur. Poll with `kill -0`/`sleep` rather than blocking in `wait`, so a deferred trap runs within one short interval; also widen the grace period so it comfortably exceeds that interval and the trap is serviced before escalation.
1 parent d15a7a0 commit 366b569

1 file changed

Lines changed: 17 additions & 6 deletions

File tree

Tests/SubprocessTests/UnixTests.swift

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -473,8 +473,12 @@ extension SubprocessUnixTests {
473473
group.addTask {
474474
var platformOptions = PlatformOptions()
475475
platformOptions.teardownSequence = [
476-
// Send SIGINT for child to catch
477-
.send(signal: .interrupt, allowedDurationToNextStep: .milliseconds(100))
476+
// SIGTERM for the child to catch (paired with the poll
477+
// loop in the script). The grace period must comfortably
478+
// exceed the poll interval so the trap is serviced
479+
// before escalation. The grace period is a ceiling;
480+
// teardown returns as soon as the child exits.
481+
.send(signal: .terminate, allowedDurationToNextStep: .seconds(1))
478482
]
479483
let result = try await Subprocess.run(
480484
.name("bash"),
@@ -486,11 +490,18 @@ extension SubprocessUnixTests {
486490
# It runs in the background forever until this script kills it
487491
/usr/bin/yes "Runaway process from \(#function), please file a SwiftSubprocess bug." > /dev/null &
488492
child_pid=$! # Retrieve the grand child yes pid
489-
# When SIGINT is sent to the script, kill grand child now
490-
trap "echo >&2 'child: received signal, killing grand child ($child_pid)'; kill -s KILL $child_pid; exit 0" INT
493+
# When SIGTERM is sent to the script, kill grand child now
494+
trap "echo >&2 'child: received signal, killing grand child ($child_pid)'; kill -s KILL $child_pid; exit 0" TERM
491495
echo "$child_pid" # communicate the child pid to our parent
492496
echo "child: waiting for grand child, pid: $child_pid" >&2
493-
wait $child_pid # wait for runaway child to exit
497+
# Poll rather than `wait "$child_pid"`. A blocking wait on a child that never
498+
# exits leaves bash no point at which to service a deferred trap, so a signal
499+
# landing in the window just before waitpid blocks is recorded but never run,
500+
# and teardown escalates to SIGKILL. A short sleep loop returns to a
501+
# trap-checking safe point each iteration, bounding trap latency.
502+
while kill -0 "$child_pid" 2>/dev/null; do
503+
sleep 0.05
504+
done
494505
""",
495506
],
496507
platformOptions: platformOptions,
@@ -499,7 +510,7 @@ extension SubprocessUnixTests {
499510
error: .fileDescriptor(.standardError, closeAfterSpawningProcess: false)
500511
) { execution in
501512
// Read stdout incrementally. Once we see the PID line,
502-
// we know the trap is set up and it's safe to send SIGINT.
513+
// we know the trap is set up and it's safe to send SIGTERM.
503514
var grandChildPid: pid_t?
504515
for try await line in execution.standardOutput.strings() {
505516
let trimmed = line.trimmingCharacters(in: .whitespacesAndNewlines)

0 commit comments

Comments
 (0)