Skip to content

mingw: avoid terminating unrelated processes during cleanup - #6493

Draft
dscho wants to merge 2 commits into
git-for-windows:mainfrom
dscho:safer-process-tree-termination
Draft

dscho wants to merge 2 commits into
git-for-windows:mainfrom
dscho:safer-process-tree-termination

Conversation

@dscho

@dscho dscho commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Windows can reuse the PID of an orphan's dead parent for a process Git is cleaning up. Our process-tree traversal currently matches only the parent PID, so it can mistake that unrelated orphan and its descendants for processes Git should terminate.

This switches to NtQuerySystemInformation to obtain creation times alongside parent PIDs. Candidates must not predate their claimed parents, and their recorded creation times must precede the snapshot-start cutoff. When opening a descendant for termination, its creation time must match the captured value, so a reused PID does not select a different process.

This should also fix a frequent flake in VFS for Git's Functional Tests.

This supersedes the process-tree cleanup proposal in the following PR: #6474. That approach opens candidates during discovery and retains a handle for every accepted descendant. Here, discovery opens no process handles, and each descendant is opened only once, at termination. Handle usage stays bounded by the root plus one temporary handle rather than growing with the tree. The independent growable-storage change is a separate commit.

This fixes #6435.

@dscho dscho self-assigned this Oct 16, 2026
dscho added 2 commits October 16, 2026 12:31
Process-tree cleanup reserves a 64 KiB PID array on the stack regardless
of the tree's size. Move that storage to the heap so memory use follows
the collected tree, retaining the existing 16,384-process traversal cap.

Assisted-by: GPT-6.1 Sol
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Process-tree cleanup can kill an older, unrelated orphan when Windows
assigns the cleanup target a PID formerly held by the orphan's parent.
Matching a parent PID alone does not prove ancestry; see
git-for-windows#6435.

Use `NtQuerySystemInformation` with `SystemProcessInformation` to obtain
creation times alongside PID and parent PID without opening processes
during discovery. Reject candidates that predate their claimed parents.

Record a precise UTC `FILETIME` immediately before each query attempt
and use the successful attempt's cutoff. Reject entries whose recorded
creation time is at or after that cutoff.

Windows records the `KeQuerySystemTime()` value at process creation and
leaves it unchanged when system time is adjusted, as documented here:
https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/nf-ntddk-psgetprocesscreatetimequadpart

These times use 100-nanosecond units, but that is not their accuracy.
Microsoft documents one-clock-tick accuracy for `KeQuerySystemTime()`
and microsecond accuracy for the precise clock in its API reference:
https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-kequerysystemtimeprecise

Only terminate a descendant if `GetProcessTimes()` on its termination
handle reports the exact creation time captured for that PID. This
rejects replacements from PID reuse after capture. The root's held
handle pins its PID.

Assisted-by: GPT-6.1 Sol
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

terminate_process_tree() kills unrelated processes when a PID has been reused (no creation-time check)

1 participant