Closing a pane right after starting it can leave its program running (stray shims from tests) #35

New issue

Open

opened 2026-10-02 02:36:47 +00:00 by jhgaylor · 0 comments

jhgaylor commented

2026-10-02 02:36:47 +00:00

Owner

Closing a pane right after starting it can leave its program running with no daemon. It's seen as stray illogicald _shim … -- bash --norc --noprofile -c sleep 600 processes (parent pid 1) piling up on jake-mini, where a CI runner runs the test suite (~/.cache/illogical-ci). Each one lives until its program exits on its own; for that test, 10 minutes.

Reproduce (macOS, main at `b6ac5be`)

cargo test -p illogicald --test api
pgrep -fl 'target/debug/illogicald _shim'     # one stray per run

It's api.rs's "a script can close what it opened, even while it's running": POST /api/run {"command": "sleep 600"}, then at once POST /api/panes/N/close, then the test ends and kills its daemon.

The stray sleep is a session leader with the pane's terminal as its controlling tty (Ss+). No process holds the PTY master any more, and a SIGHUP sent by hand kills it. So the daemon's hangup never reached it.

Why (likely)

The shim records the pid before the program has its own process group. shim::run forks, and the parent appends pid <pid> <start> at once. The child calls setsid() and then execs, but possibly after the record is written. The daemon reads the record, considers the program started, and a close arriving now runs hang_up(), which calls killpg(pid, SIGHUP). Before the child's setsid() no process group pid exists, so the signal is lost (ESRCH).
The SIGKILL backstop lives in the daemon. hang_up() sends SIGKILL to the group 3 s later from a daemon thread. If the daemon dies first (the test ends; a real daemon stops or crashes), it never happens.
Closing the master when the daemon dies should still hang the terminal up. On macOS that evidently doesn't reach this program; probably the same race, with the hangup landing before the child had made the terminal its controlling tty. To confirm.

Linux likely has the same race, but it hasn't been seen there.

Fix

Handshake in the shim. Create a close-on-exec pipe before forking. The child holds the write end through setsid() and TIOCSCTTY, and it closes on exec. The parent waits for EOF (or for an exec-failure byte) before recording the pid. Then whatever the daemon signals by pid or group exists.
Let the shim own the SIGKILL. The daemon asks for a close (SIGHUP to the group, as now), and the shim, which outlives the daemon, sends SIGKILL after a few seconds if the program is still there. That doesn't depend on the daemon living on. One way: hang_up signals the shim too, and the shim escalates.
Tests clean up after themselves. The test Daemon helpers' Drop (api.rs, attach.rs, fs.rs, agents, …) kills the process group of every pid recorded under its state dir's blocks/*/process before deleting the dir. Then no test run leaves programs behind, whatever the daemon does.

Done when

20 runs of cargo test -p illogicald --test api on macOS and Linux leave no _shim or program processes behind.
A unit or integration test closes a pane immediately after run and asserts the program is gone within a few seconds, with the daemon killed right after the close.
The CI runners on geek and jake-mini stop accumulating stray shims.

Closing a pane right after starting it can leave its program running with no daemon. It's seen as stray `illogicald _shim … -- bash --norc --noprofile -c sleep 600` processes (parent pid 1) piling up on jake-mini, where a CI runner runs the test suite (`~/.cache/illogical-ci`). Each one lives until its program exits on its own; for that test, 10 minutes. ### Reproduce (macOS, main at b6ac5be) ``` cargo test -p illogicald --test api pgrep -fl 'target/debug/illogicald _shim' # one stray per run ``` It's `api.rs`'s "a script can close what it opened, even while it's running": `POST /api/run {"command": "sleep 600"}`, then at once `POST /api/panes/N/close`, then the test ends and kills its daemon. The stray `sleep` is a session leader with the pane's terminal as its controlling tty (`Ss+`). No process holds the PTY master any more, and a SIGHUP sent by hand kills it. So the daemon's hangup never reached it. ### Why (likely) 1. **The shim records the pid before the program has its own process group.** `shim::run` forks, and the parent appends `pid <pid> <start>` at once. The child calls `setsid()` and then execs, but possibly after the record is written. The daemon reads the record, considers the program started, and a `close` arriving now runs `hang_up()`, which calls `killpg(pid, SIGHUP)`. Before the child's `setsid()` no process group `pid` exists, so the signal is lost (ESRCH). 2. **The SIGKILL backstop lives in the daemon.** `hang_up()` sends SIGKILL to the group 3 s later from a daemon thread. If the daemon dies first (the test ends; a real daemon stops or crashes), it never happens. 3. Closing the master when the daemon dies should still hang the terminal up. On macOS that evidently doesn't reach this program; probably the same race, with the hangup landing before the child had made the terminal its controlling tty. To confirm. Linux likely has the same race, but it hasn't been seen there. ### Fix - **Handshake in the shim.** Create a close-on-exec pipe before forking. The child holds the write end through `setsid()` and `TIOCSCTTY`, and it closes on `exec`. The parent waits for EOF (or for an exec-failure byte) before recording the pid. Then whatever the daemon signals by pid or group exists. - **Let the shim own the SIGKILL.** The daemon asks for a close (SIGHUP to the group, as now), and the shim, which outlives the daemon, sends SIGKILL after a few seconds if the program is still there. That doesn't depend on the daemon living on. One way: `hang_up` signals the shim too, and the shim escalates. - **Tests clean up after themselves.** The test `Daemon` helpers' `Drop` (`api.rs`, `attach.rs`, `fs.rs`, `agents`, …) kills the process group of every `pid` recorded under its state dir's `blocks/*/process` before deleting the dir. Then no test run leaves programs behind, whatever the daemon does. ### Done when - 20 runs of `cargo test -p illogicald --test api` on macOS and Linux leave no `_shim` or program processes behind. - A unit or integration test closes a pane immediately after `run` and asserts the program is gone within a few seconds, with the daemon killed right after the close. - The CI runners on geek and jake-mini stop accumulating stray shims.