Skip to content

fix: stop the child outside the PTY lock, and four runtime fixes - #610

Merged
vyncint merged 2 commits into
mainfrom
fix/runtime
Oct 27, 2026
Merged

vyncint merged 2 commits into
mainfrom
fix/runtime

Conversation

@vyncint

@vyncint vyncint commented Oct 27, 2026

Copy link
Copy Markdown
Owner

Five runtime issues.

#589 — Drop held the global PTY lock for 50–200 ms. It called portable-pty's kill (SIGHUP, then fixed 50 ms sleeps, SIGKILL after 200 ms) while holding PTY_LIFECYCLE, which every spawn takes. Now Drop stops the child before taking the lock — its own SIGHUP, the reap polled from 1 ms, SIGKILL after the same 200 ms grace — and takes the lock only to close the descriptors. Safe because this terminal's master is still open while the child is stopped, so its device cannot be recycled under anyone; the lock exists for master closes. The responder threads' copies of the master (reader and writer threads, never joined) now close under the lock too, through a small MasterDup wrapper — before, the reader's copy probably closed under the lock only because the 50 ms sleep gave it time to see EOF; without the sleep that would no longer be true. kill(2) stays in its one audited unsafe block (now a helper shared with Terminal::signal), so SECURITY.md's count is unchanged.

Measured (release, Linux), before → after:

child at drop drop spawn on another thread during the drop
dies on SIGHUP 50 ms → 1 ms 45 ms → 0 ms
ignores SIGHUP 201 ms → 201 ms 197 ms → ≤ 1 ms

#590 — Error::Write at the deadline. A write blocked in write(2) cannot be portably interrupted, so it is not withdrawn; the message, Error::Write's docs, send's docs and the skill's failure table now say the bytes stay queued and reach the application if it reads again, and that later input queues behind them. A test pins it: two sends fail at a 500 ms deadline, and the application still receives 200,000 x then y.

#591 — reply ordering. Replies are now queued for the writer thread inside the state lock (a channel send: never blocks, no writer lock). They used to be queued after the lock was released, so a waiter that saw the query's screen — woken by the release or by its own poll tick — could type first. The issue's shape, 16/32 threads × 150 runs: main reordered 11 of 9,600 at 32 threads (the application read qE[1;6R); this branch 0 of 9,600 (and 0 of 4,800 at 16 threads, where main had 1). A unit test pins the order.

#592 — spawn("target/debug/app"). On Unix a name with a directory part is looked up in the working directory first (as execvp and std do), then along PATH as before. Test: sub/x spawns from current_dir, with and without env_clear(); without the fix it fails with the issue's exact "No viable candidates found in PATH".

#593 — Windows exit watch. The watch lives on the Terminal, so the 500 ms drain grace runs from the reap, not from each wait's first tick; wait_exit records the reap, and send ticks the watch so input long after the exit is refused. Windows-only test (100 ms waits until EOF, then a refused send). Clippy for x86_64-pc-windows-msvc passes locally; the Windows leg runs the test.

Verification. Each fix's test fails without it (a run with the SIGKILL step removed also left a real stray process, which is the failure the test exists for). Full workspace suite, clippy (Linux and Windows targets), rustdoc -D warnings, DCO and attribution gates pass locally. Stress is dispatched on this branch: the lock change is exactly what its comment says the stress workflow must bless.

Closes #589
Closes #590
Closes #591
Closes #592
Closes #593

- Drop stops the child before taking the lifecycle lock, with its own
  SIGHUP, a reap polled from 1 ms and SIGKILL after portable-pty's grace,
  then closes the descriptors under the lock; the responder threads'
  copies of the master close under it too (#589).
- A write that gave up at the deadline is not withdrawn, and the error
  and docs now say so (#590).
- Query replies are queued for the writer under the state lock, so no
  waiter can see a query's screen before its reply is queued (#591).
- A program named with a directory part is looked up in the working
  directory first, then along PATH (#592).
- On Windows the exit watch lives on the terminal, so the drain grace runs
  from the reap rather than from each wait's first tick, and a send asks
  it too (#593).

Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
Resolves the CHANGELOG conflict with #609: both sets of entries kept.

Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
@vyncint
vyncint merged commit f834d5f into main Oct 27, 2026
16 checks passed
@vyncint
vyncint deleted the fix/runtime branch October 27, 2026 09:46
vyncint added a commit that referenced this pull request Oct 27, 2026
Resolves the CHANGELOG conflict with #610: both sets of entries kept.

Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment