Skip to content

gh-144316: Fix missing exception in _remote_debugging with debug=False#144442

Merged
pablogsal merged 14 commits intopython:mainfrom
taegyunkim:gh-144316-get_stack_trace
Feb 26, 2026
Merged

gh-144316: Fix missing exception in _remote_debugging with debug=False#144442
pablogsal merged 14 commits intopython:mainfrom
taegyunkim:gh-144316-get_stack_trace

Conversation

@taegyunkim
Copy link
Contributor

@taegyunkim taegyunkim commented Feb 3, 2026 •
edited
Loading

  1. Adds missing PyErr_NoMemory() call when PyMem_RawMalloc() returned NULL.
  2. Adds missing PyErr_Format() calls before calling set_exception_cause macro
  3. Modifies set_exception_cause macro to assert an error is set.

@bedevere-app bedevere-app bot mentioned this pull request Feb 3, 2026
Closed
@taegyunkim taegyunkim force-pushed the gh-144316-get_stack_trace branch from 7d1bb63 to 0f2d244 Compare February 3, 2026 19:50
@taegyunkim taegyunkim force-pushed the gh-144316-get_stack_trace branch from cffe58f to 66b060d Compare February 3, 2026 20:01
picnixz
picnixz reviewed Feb 3, 2026
View reviewed changes
@taegyunkim taegyunkim marked this pull request as ready for review February 3, 2026 20:14
@taegyunkim taegyunkim requested a review from pablogsal as a code owner February 3, 2026 20:14
@pablogsal
Copy link
Member

pablogsal commented Feb 4, 2026

This macro is intended to add context to an exception not to be the actual cause of the error. Doing this will mask actual bugs in the code where we are not setting exceptions properly in places that should catch it and fail. Unfortunately this is not only is a partial fix but it will hide errors. The correct solution is finding the error paths that are not properly handling errors and fix there (and perhaps chain).

@taegyunkim
Copy link
Contributor Author

taegyunkim commented Feb 4, 2026 •
edited
Loading

@pablogsal Found PyMem_RawMalloc() call which didn't set PyErr when it returned NULL. Let me know if the changes to the macro look unnecessary.

picnixz
picnixz reviewed Feb 4, 2026
View reviewed changes
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should instead add assert(PyErr_Occurred()) in _set_debug_exception_cause instead.

@taegyunkim taegyunkim marked this pull request as draft February 4, 2026 20:37
@taegyunkim taegyunkim marked this pull request as ready for review February 4, 2026 21:29
@picnixz
Copy link
Member

picnixz commented Feb 5, 2026 •
edited
Loading

Please, stop updating the branch when there is nothing to do. It wastes CI resources and notifies everyone subscribed to the thread. See https://devguide.python.org/getting-started/pull-request-lifecycle/#update-branch-button.

@colesbury colesbury self-requested a review February 26, 2026 19:58
@colesbury
Copy link
Contributor

colesbury commented Feb 26, 2026

@pablogsal - do you have any more objections? Otherwise, I'm going to merge this.

@pablogsal
Copy link
Member

pablogsal commented Feb 26, 2026

@pablogsal - do you have any more objections? Otherwise, I'm going to merge this.

No, let's land this!

@pablogsal pablogsal merged commit c2d3d6b into python:main Feb 26, 2026
47 checks passed
@pablogsal
Copy link
Member

pablogsal commented Feb 26, 2026

Great work @taegyunkim !

@pablogsal pablogsal added the needs backport to 3.14 bugs and security fixes label Feb 26, 2026
@miss-islington-app
Copy link

miss-islington-app bot commented Feb 26, 2026

Thanks @taegyunkim for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app
Copy link

miss-islington-app bot commented Feb 26, 2026

Sorry, @taegyunkim and @pablogsal, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker c2d3d6b0dd87aaea2f3a72419e1d1488d3ab9503 3.14

pablogsal pushed a commit to pablogsal/cpython that referenced this pull request Feb 26, 2026
@taegyunkim @pablogsal
pythongh-144316: Fix missing exception in _remote_debugging with debu…
dcd1c74 
…g=False (python#144442)
pablogsal pushed a commit to pablogsal/cpython that referenced this pull request Feb 26, 2026
@taegyunkim @pablogsal
[3.14] pythongh-144316: Fix missing exception in _remote_debugging wi…
24ef36a 
…th debug=False (pythonGH-144442)

(cherry picked from commit c2d3d6b)

Co-authored-by: Taegyun Kim <taegyun.kim@datadoghq.com>
pablogsal added a commit to pablogsal/cpython that referenced this pull request Feb 26, 2026
@pablogsal
[3.14] pythongh-144316: Fix missing exception in _remote_debugging (p…
0b5eaae 
…ythonGH-144442)
@bedevere-app
Copy link

bedevere-app bot commented Feb 26, 2026

GH-145280 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Feb 26, 2026
pablogsal added a commit that referenced this pull request Feb 26, 2026
@pablogsal
[3.14] gh-144316: Fix missing exception in _remote_debugging with deb…
ded533b 
…ug=False (GH-144442) (#145280)
bkap123 pushed a commit to bkap123/cpython that referenced this pull request Feb 28, 2026
@taegyunkim @bkap123
pythongh-144316: Fix missing exception in _remote_debugging with debu…
b9b2499 
…g=False (python#144442)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixz picnixz picnixz left review comments

@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner

@colesbury colesbury Awaiting requested review from colesbury

Assignees

@pablogsal pablogsal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@taegyunkim @pablogsal @picnixz @colesbury