Repository navigation
fix: stop the child outside the PTY lock, and four runtime fixes - #610
Merged
Merged
Conversation
- 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
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five runtime issues.
#589 —
Dropheld the global PTY lock for 50–200 ms. It called portable-pty'skill(SIGHUP, then fixed 50 ms sleeps, SIGKILL after 200 ms) while holdingPTY_LIFECYCLE, which every spawn takes. NowDropstops 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 smallMasterDupwrapper — 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 auditedunsafeblock (now a helper shared withTerminal::signal), so SECURITY.md's count is unchanged.Measured (release, Linux), before → after:
drop#590 —
Error::Writeat the deadline. A write blocked inwrite(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,000xtheny.#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 (asexecvpand std do), then along PATH as before. Test:sub/xspawns fromcurrent_dir, with and withoutenv_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_exitrecords the reap, andsendticks the watch so input long after the exit is refused. Windows-only test (100 ms waits until EOF, then a refused send). Clippy forx86_64-pc-windows-msvcpasses 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