fix: exclude parent process chain from concurrent instance detection on Windows#31712
Closed
Strontvod wants to merge 1 commit into
Closed
fix: exclude parent process chain from concurrent instance detection on Windows#31712Strontvod wants to merge 1 commit into
Strontvod wants to merge 1 commit into
Conversation
…on Windows On Windows, the setuptools-generated hermes.exe launcher is a separate native process that spawns python.exe (the interpreter running the update code). os.getpid() returns the Python PID, but the launcher (which holds the file lock) is the parent. Without walking the parent chain, every 'hermes update' reports its own launcher as a concurrent instance - a false positive. This patch builds an exclusion set containing the Python process and its entire ancestor chain, so the running invocation never reports itself.
Collaborator
Contributor
Author
|
Hey @alt-glitch was this meant for a different PR? #31712 is about the Windows concurrent instance false-positive fix, not kanban scratch cleanup. Just flagging in case it landed here by accident. |
Contributor
|
Merged via #31806 — your commit was cherry-picked onto current main with authorship preserved (323cce7e9). Thanks for the fix! Follow-up commit (46f8948ba) on top:
|
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.
Bug Description
On Windows, every
hermes updatereports a false positive concurrent instance error and aborts, because the setuptools-generatedhermes.exelauncher is detected as a separate running instance of Hermes.Root Cause
The setuptools launcher (
hermes.exe) is a separate native process that spawnspython.exe(the interpreter that runs the update code).os.getpid()returns the Python PID, but the launcher (which holds the file lock that prevents the update from renaming/quarantining the .exe) is the parent process. The old code only excluded the Python PID itself viaexclude_pid = os.getpid(), so the launcher process was always flagged as a concurrent instance.Fix
Build an exclusion set containing the Python process and its entire ancestor chain by walking
psutil.Process.parent()upward. This ensures the runninghermes updateinvocation never reports any process in its own tree — whether it's the Python interpreter, the .exe launcher, or any shell in between.How to Verify
hermes updatewithout this fix — it fails with a concurrent-instance error.hermes updatewith a second Hermes process (e.g. desktop app) still correctly blocks, since the other process is outside this process's parent chain.Test Plan
Risk Assessment
Low — Only affects the Windows concurrent-instance detection path. The change expands the exclusion set from a single PID to the full parent chain; all other logic (shim comparison, process enumeration) is unchanged. Off-Windows the function returns
[]early, so no effect on Linux/macOS.