Skip to content

gh-106140: Reorder some more fields to facilitate out-of-process inspection#106148

Merged
pablogsal merged 1 commit into
python:mainfrom
pablogsal:gh-106140-2
Jun 27, 2023
Merged

gh-106140: Reorder some more fields to facilitate out-of-process inspection#106148
pablogsal merged 1 commit into
python:mainfrom
pablogsal:gh-106140-2

Conversation

@pablogsal
Copy link
Copy Markdown
Member

@pablogsal pablogsal commented Jun 27, 2023 •
edited by bedevere-bot
Loading

@pablogsal pablogsal added the needs backport to 3.12 only security fixes label Jun 27, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jun 27, 2023
Closed
@pablogsal pablogsal enabled auto-merge (squash) June 27, 2023 15:45
@pablogsal pablogsal merged commit 9126a6a into python:main Jun 27, 2023
@pablogsal pablogsal deleted the gh-106140-2 branch June 27, 2023 16:09
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Jun 27, 2023

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Jun 27, 2023

Sorry, @pablogsal, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 9126a6a9ce3772d5dc785cbee159b07a1ff7d531 3.12

@pablogsal pablogsal added needs backport to 3.12 only security fixes and removed needs backport to 3.12 only security fixes labels Jun 27, 2023
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Jun 27, 2023

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Jun 27, 2023

Sorry @pablogsal, I had trouble checking out the 3.12 backport branch.
Please retry by removing and re-adding the "needs backport to 3.12" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 9126a6a9ce3772d5dc785cbee159b07a1ff7d531 3.12

@pablogsal pablogsal added needs backport to 3.12 only security fixes and removed needs backport to 3.12 only security fixes labels Jun 27, 2023
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Jun 27, 2023

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Jun 27, 2023

Sorry @pablogsal, I had trouble checking out the 3.12 backport branch.
Please retry by removing and re-adding the "needs backport to 3.12" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 9126a6a9ce3772d5dc785cbee159b07a1ff7d531 3.12

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Jun 27, 2023

GH-106155 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Jun 27, 2023
pablogsal added a commit to pablogsal/cpython that referenced this pull request Jun 27, 2023
@pablogsal
[3.12] pythongh-106140: Reorder some more fields to facilitate out-of…
c965a5d 
…-process inspection (pythonGH-106148)

(cherry picked from commit 9126a6a)

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
pablogsal added a commit to pablogsal/cpython that referenced this pull request Jun 27, 2023
@pablogsal
[3.12] pythongh-106140: Reorder some more fields to facilitate out-of…
a41b29f 
…-process inspection (pythonGH-106148)

(cherry picked from commit 9126a6a)

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
pablogsal added a commit that referenced this pull request Jun 27, 2023
@pablogsal
[3.12] gh-106140: Reorder some more fields to facilitate out-of-proce…
20315d1 
…ss inspection (GH-106148) (#106155)

(cherry picked from commit 9126a6a)
trohitg added a commit to zeenie-ai/MachinaOS that referenced this pull request May 26, 2026
@trohitg @claude
fix(cli): catch SystemError leak from os.kill on Windows
0ba137f 
``machina stop`` crashed on Windows whenever a configured port had
an actual listener::

  OSError: [WinError 87] The parameter is incorrect

  The above exception was the direct cause of the following exception:
  ...
  SystemError: <built-in function kill> returned a result with an
  exception set

CPython issue python/cpython#106148: on Windows, ``os.kill(pid,
signal.CTRL_BREAK_EVENT)`` calls ``GenerateConsoleCtrlEvent`` under
the hood. When that fails with ``ERROR_INVALID_PARAMETER`` (e.g.
target isn't in the caller's console group -- every port-bound
service that wasn't spawned by our supervisor), CPython sets an
``OSError`` AND returns success. The interpreter detects the
inconsistency and raises ``SystemError`` instead of propagating the
underlying ``OSError``.

``cli/ports.py``'s ``kill_pid`` already had a fallback to
``proc.terminate()`` for the ``OSError`` case, but the
``except (OSError, ProcessLookupError)`` clause never caught
``SystemError`` -- so the bug propagated, typer printed a traceback,
the process exited non-zero, and PowerShell (with its PS 7.4+
default ``$PSNativeCommandUseErrorActionPreference=$true`` plus a
``$ErrorActionPreference='Break'`` or ``Set-PSDebug`` in the user's
profile) dropped into the debugger at the npm shim's
``$ret=$LASTEXITCODE`` line.

Fix: add ``SystemError`` to the except tuple. The existing fallback
to ``TerminateProcess`` (via ``psutil.terminate``) now runs as
designed.

End-to-end verified against the packed npm tarball: ``machina
stop`` invoked from an installed bin shim kills a real process on
port 3010 and exits 0 cleanly, no traceback.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@pablogsal pablogsal

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pablogsal @miss-islington @bedevere-bot