From dccef0c5016d600b057fe7a30e438436ba41aab2 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 1 Oct 2026 16:09:04 -0400 Subject: [PATCH] Fix a macOS cell stopping when it kills a worker's tools On macOS, `Process.kill` raised `Errno::EPERM`, not `Errno::ESRCH`, for a process group whose only members were zombies. A tool killed with its worker stays a zombie until `launchd` reaps it, so the supervisor's reap could raise, stop the cell, and leave the caller with no response. `test_a_deadline_kills_what_the_worker_started_too` failed intermittently on the macOS CI job for this reason. Rescue `Errno::EPERM` from the group kill in `sweep_group` and `kill_group`. --- CHANGELOG.md | 4 +++ hotcell-server/lib/hot_cell/supervisor.rb | 11 +++++-- hotcell-server/test/scheduling_test.rb | 39 +++++++++++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c28020e..c1ac95d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,10 @@ Some actions that application developers should consider taking when upgrading f * A request that finds an idle worker no longer waits for `fork`. The supervisor forks a worker into each free slot at boot, and forks a replacement as soon as it reaps a worker that served a request. A request that waits in the queue still waits for `fork`. The supervisor runs `Process.warmup` once, at boot, before the first fork. +#### Fixed + +* A cell on macOS no longer stops when it kills a worker that started another process. Previously, `Process.kill` on the worker's process group could raise `Errno::EPERM` while that process was a zombie. The supervisor did not rescue the error, so the cell stopped and the caller received no response. + ### HotCell::Client #### Added diff --git a/hotcell-server/lib/hot_cell/supervisor.rb b/hotcell-server/lib/hot_cell/supervisor.rb index 11c2164..29d8e61 100644 --- a/hotcell-server/lib/hot_cell/supervisor.rb +++ b/hotcell-server/lib/hot_cell/supervisor.rb @@ -872,18 +872,23 @@ def enforce_retirements # the sweep must not signal it. An empty group is the common case, so ESRCH is expected. It is safe to # kill the group by the leader's pid even though the leader is reaped, because the supervisor is single # threaded and mints group leaders only in `spawn`, which cannot run between the `wait2` above and here. + # + # macOS answers EPERM rather than ESRCH for a group whose only members are zombies, and a tool killed + # with its worker stays one until launchd reaps it. Raised, it unwound `run` and ended the cell before + # this reap answered the caller. EPERM means no member received the signal, the same outcome as ESRCH. def sweep_group(child) Process.kill :KILL, -child.pid - rescue Errno::ESRCH + rescue Errno::ESRCH, Errno::EPERM nil end # The whole process group, which is the worker and everything it started. Negative pid is the group. # Falls back to the worker alone if the group is already gone, so a worker that died between the check - # and the signal is not an error. + # and the signal is not an error. A group answering EPERM falls back as well, for the reason + # `sweep_group` gives; the bare pid does not, because macOS signals a zombie pid without complaint. def kill_group(child) Process.kill :KILL, -child.pid - rescue Errno::ESRCH + rescue Errno::ESRCH, Errno::EPERM begin Process.kill :KILL, child.pid rescue Errno::ESRCH diff --git a/hotcell-server/test/scheduling_test.rb b/hotcell-server/test/scheduling_test.rb index 745aa52..141f81c 100644 --- a/hotcell-server/test/scheduling_test.rb +++ b/hotcell-server/test/scheduling_test.rb @@ -59,6 +59,27 @@ def test_a_deadline_kills_what_the_worker_started_too ENV.delete "HOTCELL_SPAWNED_PID_PATH" end + # macOS refuses a signal to a process group whose only members are zombies, with EPERM rather than ESRCH. + # A killed worker's tool is one such zombie until launchd reaps it, so the reap's sweep raised, unwound the + # run loop and ended the cell, and the caller read end of stream instead of the verdict. + def test_a_deadline_kill_is_answered_when_the_reap_sweep_is_refused + refuse_group_signals(after: 1) do + TestCell.boot(deadline: 0.3, concurrency: 1) do |cell| + assert_failed "killed", cell.call("test.uninterruptible", timeout: 20), cause: "deadline" + end + end + end + + # The deadline kill meets the same refusal when the worker has exited unreaped. Refused, it falls back to + # the worker's own pid. Here the worker is still alive, so the fallback is what kills it. + def test_a_deadline_kill_is_answered_when_the_group_kill_is_refused + refuse_group_signals do + TestCell.boot(deadline: 0.3, concurrency: 1) do |cell| + assert_failed "killed", cell.call("test.uninterruptible", timeout: 20), cause: "deadline" + end + end + end + # A reused worker is not one operation. Serving A, then B, then A left the shared library configured by B # while A ran, because the memo asked "has this ever booted" rather than "is this what it is set up for". def test_a_reused_worker_reconfigures_when_the_operation_changes @@ -260,4 +281,22 @@ def alive?(pid) rescue Errno::ESRCH false end + + # The cell forks from this process, so a stub installed here is inherited by the supervisor. + def refuse_group_signals(after: 0) + original = Process.method(:kill) + allowed = after + Process.define_singleton_method(:kill) do |signal, *pids| + if pids.any?(&:negative?) + raise Errno::EPERM, "induced" if allowed.zero? + + allowed -= 1 + end + + original.call signal, *pids + end + yield + ensure + Process.define_singleton_method(:kill, original) + end end