Commit 94cd891
authored
Make pty kill verify and finish the kill, read the process table in one place, and stop waiting to say a name is taken (#170)
* Say what the kill verified, not what it hoped
`pty kill` signals the daemon and waits for that one pid. It then prints
`Session "X" killed.` The child, and everything the child started, is never
looked at. The word is a claim about a session made on evidence about a
daemon.
This takes a snapshot of the daemon's process tree before the signal, and
re-checks it after the daemon exits. The success line now appears only when
every process in the snapshot is gone. Otherwise the command prints
`Session "X" daemon stopped.`, which is the part it verified, and names the
survivors on standard error.
The snapshot must come first. After the daemon exits its children reparent
away, so the links that identify them are gone. The pre-kill snapshot is also
taken at a calm moment, while the daemon takes its own during shutdown, so the
command can see a process the daemon's teardown skipped.
A pid is reported as surviving only when its start token still matches. A
token that cannot be read on a process that has not exited is reported
separately as undecided, rather than being folded into either answer.
A zombie is not a survivor. It answers `kill(pid, 0)` and keeps its start
token, so the check reads the process state through `hasProcessExitedForReap`.
Two supporting changes:
A daemon that could not kill a descendant now appends
`session_descendants_survived` to the session event log, and names the pids in
its stderr warning. The daemon's stderr has no reader. The Rust tool already
writes this event; this closes the gap.
On macOS, an empty `ps -o stat=` field no longer counts as a dead process. An
empty field means the process is gone or `ps` did not answer, and under load
`ps` is the thing that goes quiet. The new check depends on this predicate, so
it had to stop reading silence as death.
This sends no additional signals.
* Make the exit status agree with the words
`pty kill` printed a survivor report and exited 0. A caller that reads only the
status reached the opposite conclusion from one that reads the output, which
leaves the honest line as decoration.
It now exits non-zero when anything survived, and when a start token could not
be read so the outcome is undecided. "I could not confirm the tree is empty" is
not success.
This is a compatibility break. A script that checks the status of `pty kill`
will fail where it used to pass, because it was passing on a false success. The
fix for such a caller is to stop treating an unverified kill as a completed one.
Nathan decided the survivor case on 2026-09-03. Silber.cos decided the
undecidable one.
* Finish the kill instead of reporting that it did not
The daemon tears down the child's tree on its way out, so the teardown races
its own exit. Whatever it does not manage is nobody's work after that, and the
command that outlives it does nothing about it.
`pty kill` now re-reads the process table after the daemon has gone. If
anything from its pre-kill snapshot is still alive, it signals the process
groups the session left behind, waits, escalates to SIGKILL, reads the table
again, and reports what is still there. It never reports the sending.
Process groups rather than pids, because a group signal needs no identity. The
snapshot drops a descendant whose start token cannot be read, and that process
is then never signalled; groups are collected from the raw listing, so it is
reached anyway. The blind spot is not solved, it is made irrelevant. A sweep
also costs one `ps` in total, against one per descendant for tokens.
The daemon's own group is never a target. The pty child calls setsid, so the
daemon sits alone in its group and signalling it reaches the daemon only. The
running process's own group is never a target either, so the command survives
to print its result.
A zombie is not a group member. `ps` lists it with its group, so counting it
makes the sweep report a group it has already emptied. A test against a real
process group found this; inspection did not.
* Do not call a tree empty while the escalation left something running
The success line and the exit status read only the pre-kill snapshot. That
snapshot drops a descendant whose start token could not be read, and it never
contained a process spawned after it was taken. So a process the sweep found
and could not kill can be absent from it entirely, and the command would print
`killed` and exit 0 over a process that had just survived SIGKILL.
That is the defect this command exists to stop making, reintroduced by the
escalation that was supposed to end it. Both halves are now required: the
snapshot must be clear AND the sweep must have left nothing behind.
Found by reading the diff rather than by a failing test, so the test came
after; it fails without the fix.
* Read the process table in one place, and stop spawning ps on Linux
Every caller that needed a fact about a process ran its own `ps` and treated
the output as fact. A subprocess can be slow, truncated or silent, and all
three look exactly like "the process is gone".
Node cannot make the syscalls the Rust tool uses, so this does what Node can.
On Linux there is now no subprocess at all: `/proc` carries ppid, pgid, state
and starttime, which is every fact the callers ask for. On macOS `ps` stays,
but the table is read once per operation rather than once per process per
poll. In the teardown loop that is the difference between 240 spawns inside a
1500 ms deadline and 60.
Silence is a third answer everywhere. Every query separates the fact from "the
table was read and this process is not in it" from "I could not find out",
with no default and no conversion that turns the last into the middle by
accident. Treating silence as death requires calling
`orAbsentWhenUnknown`, which greps in one command. That makes the mistake
visible rather than impossible.
A listing that does not contain the process that read it was truncated, not
empty. `ps` always lists itself.
`recovery.processStartToken` is untouched and still comes from `ps -o lstart=`.
Its exact text is a contract with the Rust tool through a shared registry, so
the parser takes the tail verbatim rather than re-joining split fields, which
would have rewritten `Wed Sep 3` as `Wed Sep 3`. The in-memory identity is a
separate branded type so the two can never be compared.
Production `ps` call sites: six to three, none in a per-process poll loop.
* Do not count a corpse as a surviving descendant
An unreaped descendant keeps its `/proc` row and its identity on Linux, so
matching on identity alone counted it as alive. The teardown would wait out its
whole TERM budget for a process that had already died, and then report it as
having survived a SIGKILL. That is the kill over-claiming again, in the other
direction.
macOS never had this, because `ps` stops listing a process the moment it exits.
The two platforms disagreeing is what exposed it.
Two test fixes from a real Mac run, reported by Silber.pty:
macOS has the `setsid` system call but no `setsid` executable. The real
process-group test spawned the binary, so on the one platform where process
groups are the whole escalation story, the test could not run at all. It now
uses `detached: true`, which is the same thing without the command.
The zombie test asserted the Linux mechanism rather than the conclusion. On
Linux the corpse keeps a row with state Z; on macOS it is dropped from the
listing at once. The test now asserts what every caller depends on, which is
the same on both.
A single-pid query no longer reads the whole table. `hasProcessExitedForReap`
sits inside poll loops that run every 25 ms, and asking about one pid should
not pay for every process on the machine.
* Parse what ps writes, not what the parser expected
The single-process read handed the subprocess's raw stdout to the single-line
parser. `ps` ends its output with a newline and that parser's `$` does not match
before one, so on macOS a live process read as "field-empty" and a zombie read
as NOT EXITED. The teardown would then have waited out its whole budget for a
corpse.
That is the corpse defect a third time, reintroduced by a second parsing path
that Linux never exercised, because on Linux this read goes to `/proc`.
There is one parser now. The single-process read goes through
`parsePsListing`, which splits lines first and checks that the row for the pid
it asked about is actually present, so both jobs are done by the code that was
already tested.
The other two `ps` call sites were checked for the same seam. `server.ts` and
`recovery.ts` both trim before parsing; only the path added by this branch did
not.
Nothing caught it because every test built its input the way the parser
expected. They all agreed with each other and none of them agreed with `ps`. So
there is now a test that runs the real command with the real arguments and feeds
the parser exactly what the subprocess wrote, newline and all. Reverting to the
old behaviour fails it.
Found on a real Mac by Silber.pty.
* Say "is already running" at once instead of after thirty seconds
A `pty run` that loses a creation race waited out the whole start budget and
then reported a generic publication timeout.
`wait_for_publication` compares the published metadata against its OWN pid, so
for a loser that check is false for the rest of the budget. Its only other way
out of the loop is noticing its own daemon die. When that is slow — a loaded
machine, a daemon still starting up — the loop spends the entire thirty second
default and reports a timeout, when the true answer was on disk in the first
iteration.
Measured on a Mac by Silber.pty on 2026-09-03: 30.06 s against a 30 s budget,
saying "Timed out waiting for daemon publication" instead of "is already
running".
The loop now checks whether the name is published by a live process that is not
us, and stops with the sentence `pty run` already prints when it sees a running
session before it spawns. Losing the race later should not produce a different
explanation of the same situation.
All three conditions matter and each is tested. Published, or a name whose
metadata is still being written would be refused. A different pid, or a
successful spawn would refuse itself. A live one, or stale metadata from a dead
daemon would make the name permanently unusable.
The safety property was never in question. The test that found this asserts
exactly one winner before it checks the loser message, and that assertion
passed every time. What was wrong is what the loser said, and how long it took.
* Do not call a zombie daemon a live owner
The new check refuses a session name when it is published by a live process
that is not us. It asked `pid_alive` / `isProcessAlive`, and a zombie answers
`kill(pid, 0)`. So an unreaped daemon would have counted as live and the name
would have been refused for as long as the corpse went unreaped.
That is the exact failure the liveness condition exists to prevent, arrived at
by the check that was supposed to prevent it. An unreaped daemon is the precise
case that matters here: dead, not reaped, still in the process list.
Both now ask `has_process_exited_for_reap` / `hasProcessExitedForReap`, which
reads the process state and counts a zombie as gone.
Tested against a real corpse in both languages, with the predicate the
production path actually passes. Reverting to the cheap predicate fails both.
Node also exports `hasProcessExitedForReap`, which was private.
Silber.cos asked what this check does with a zombie daemon on macOS. The answer
was worse than the question: it was wrong on both platforms.
* Do not call a name in use when nothing alive is using it
`pty run --id X` refused when a session record named X existed on disk, without
asking whether anything was answering for it. A record whose owner is a zombie
has no socket, so the name was unusable until something reaped the corpse.
That is the same failure the liveness check in the spawner exists to prevent,
one command earlier and still shipping.
The Rust tool already asks `session_exists(name) && client::is_alive(name)`
here. This adds the missing half: `isSessionAlive`, a faithful port of
`client::is_alive`, which is a socket connect.
Measured on a Mac by Silber.pty on 2026-09-03 with a real zombie owner: Rust
created a replacement in 186 ms, Node exited 1 with "is already in use".
Verified end to end on Linux with a real zombie-owned record and no socket:
Node now creates the session, and reverting the liveness condition refuses it
again.1 parent f990e38 commit 94cd891
18 files changed
Lines changed: 1598 additions & 82 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
5 | 72 | | |
6 | 73 | | |
7 | 74 | | |
| |||
28 | 95 | | |
29 | 96 | | |
30 | 97 | | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
31 | 104 | | |
32 | 105 | | |
33 | 106 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
133 | 133 | | |
134 | 134 | | |
135 | 135 | | |
| 136 | + | |
136 | 137 | | |
137 | 138 | | |
138 | 139 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
8 | 10 | | |
9 | 11 | | |
10 | 12 | | |
| |||
42 | 44 | | |
43 | 45 | | |
44 | 46 | | |
| 47 | + | |
45 | 48 | | |
46 | 49 | | |
47 | 50 | | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
48 | 57 | | |
49 | 58 | | |
50 | 59 | | |
| |||
295 | 304 | | |
296 | 305 | | |
297 | 306 | | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
298 | 315 | | |
299 | 316 | | |
300 | 317 | | |
| |||
935 | 952 | | |
936 | 953 | | |
937 | 954 | | |
938 | | - | |
| 955 | + | |
| 956 | + | |
| 957 | + | |
| 958 | + | |
| 959 | + | |
| 960 | + | |
| 961 | + | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
| 965 | + | |
939 | 966 | | |
940 | 967 | | |
941 | 968 | | |
| |||
2615 | 2642 | | |
2616 | 2643 | | |
2617 | 2644 | | |
| 2645 | + | |
| 2646 | + | |
| 2647 | + | |
| 2648 | + | |
| 2649 | + | |
| 2650 | + | |
| 2651 | + | |
| 2652 | + | |
| 2653 | + | |
| 2654 | + | |
| 2655 | + | |
| 2656 | + | |
| 2657 | + | |
| 2658 | + | |
| 2659 | + | |
| 2660 | + | |
| 2661 | + | |
2618 | 2662 | | |
2619 | 2663 | | |
2620 | 2664 | | |
| |||
2637 | 2681 | | |
2638 | 2682 | | |
2639 | 2683 | | |
| 2684 | + | |
| 2685 | + | |
| 2686 | + | |
| 2687 | + | |
| 2688 | + | |
| 2689 | + | |
| 2690 | + | |
| 2691 | + | |
| 2692 | + | |
| 2693 | + | |
2640 | 2694 | | |
2641 | 2695 | | |
2642 | 2696 | | |
| |||
2662 | 2716 | | |
2663 | 2717 | | |
2664 | 2718 | | |
2665 | | - | |
| 2719 | + | |
| 2720 | + | |
| 2721 | + | |
| 2722 | + | |
| 2723 | + | |
| 2724 | + | |
| 2725 | + | |
| 2726 | + | |
| 2727 | + | |
| 2728 | + | |
| 2729 | + | |
| 2730 | + | |
| 2731 | + | |
| 2732 | + | |
| 2733 | + | |
| 2734 | + | |
| 2735 | + | |
| 2736 | + | |
2666 | 2737 | | |
2667 | 2738 | | |
2668 | 2739 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
20 | 43 | | |
21 | 44 | | |
22 | 45 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
| |||
85 | 86 | | |
86 | 87 | | |
87 | 88 | | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
88 | 104 | | |
89 | 105 | | |
90 | 106 | | |
| |||
185 | 201 | | |
186 | 202 | | |
187 | 203 | | |
| 204 | + | |
188 | 205 | | |
189 | 206 | | |
190 | 207 | | |
| |||
0 commit comments