Repository navigation
fix(cli): renders on Windows stop cancelling when an old parent's pid is reused - #5358
Merged
Merged
Conversation
miguel-heygen
marked this pull request as ready for review
October 9, 2026 17:51
Contributor
Edit accuracy: accurate 2059 (base branch 2059), smooth 1542 of thoseThe gate passes. Quarantined, measured but not gated (0) |
jrusso1020
approved these changes
Oct 9, 2026
jrusso1020
left a comment
Collaborator
There was a problem hiding this comment.
Approve at 066eb11e. The walk now stops at a "parent" created after its child, and only a reused pid can look like that. Real parents are always created before their children, so the real chain is kept, and a terminal that really closes still cancels the render.
Checked
- The cause: on Windows the ancestor chain comes from one
Win32_Processlisting, followingParentProcessId. Windows doesn't reparent orphans, so a dead parent's pid can belong to an unrelated, newer process. The old walk followed it, and the render cancelled when that process exited. - The guard is complete for reuse. A reused pid's new process can only start after the original parent died. That parent was alive when it created the child, so the new process always starts after the child, and
parent.startedAt > child.startedAtcatches it. The one exception is a clock step, which the body already covers under Limits. - The time source: the listing uses
CreationDate.ToFileTimeUtc(), which is an integer tick count. So/^\d+$/plusBigIntparses it, and comparing BigInts avoids float precision loss. - A missing or unparseable time leaves
startedAtundefined, and the guard does nothing. That fails toward the old behaviour (watch the parent), never toward dropping a real one. - Closing the terminal still cancels:
- The truncated chain only loses entries above a reused pid, which were never real ancestors.
- The shell and every real parent below it stay watched.
- Closing the console also delivers SIGHUP, which aborts on its own.
- Scope:
recordsToAncestorsruns only for the off-Linux snapshot, so Linux is untouched. macOS reparents orphans to launchd, so a dead parent's pid isn't followed there.
Nits (non-blocking)
- The body says the macOS
pslisting "carries no start time".lstartis a start time; it's only a string. The conclusion still holds, because macOS reparents orphans. - Pre-existing, opposite direction: off Linux the watchdog only checks
isAlive(pid), never identity (renderCancellation.ts:125-133). On Windows, if the real parent exits and its pid is reused within the 2 s poll, the render keeps running orphaned until that new process exits. The snapshot already recordswindows:<creation>identities, so comparing them in the off-Linux check would close this the same way Linux does.
Tests:
orphanCleanup(all three files),renderCancellation.test.tsandprocessTree.test.tspass 28/28.renderCancellation.process.test.tsfails 4 cases on this box, both at this head and at its base, so it's environmental; CI runs it.- Mutants:
>→>=is caught by the same-tick test.- Dropping the guard is caught.
- Never parsing
startedAtis caught.
- Every check on this head passed before this review was posted.
— Rames
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.
What changes for a user
On Windows, a local render could cancel itself a moment after starting, or partway through, with
Render cancelled: render_cancelled_parent_exited, while the terminal that started it was still open. Renders now keep going unless a real parent exits.Why
At start, a render records its chain of parent processes and cancels if any of them exits. On Windows it builds that chain from one
Win32_Processlisting by following each process's parent pid. Windows never reparents an orphan, so a parent pid can point at a parent that died long ago, and Windows may have handed that pid to an unrelated process. The walk followed it past the real chain onto that unrelated process; when it exited, the render cancelled.recordsToAncestorsnow stops when a "parent" was created after its child, which can only mean its pid was reused. On macOS thepslisting carries no start time and Linux does not build this list at all, so nothing changes there.This is the failure behind intermittent
Windows render verificationreds on main, the merge queue and unrelated PRs since the check was added.Limits
The check trusts process creation times. If the system clock steps backwards between a parent's start and its child's, a real parent can look newer and the walk stops there. That render then stops watching its parents from that point up. A backwards step after the child starts can still let a reused pid look older, which is the old behaviour for that render, no worse.
Tests
orphanCleanup.windowsHide.test.ts: with400 → 300 → 200 → 999where 999 started after 200, the chain is300, 200. Without the fix it also includes 999, and the test fails. A parent created in the same tick as its child stays an ancestor (fails if the check becomes>=).tsc --noEmitclean for the CLI.Standalone on purpose: one root in the CLI's process-ancestry walk, unrelated to other open changes.