Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[3.10] bpo-44449: faulthandler don't modify frame refcnt #27850

Merged
merged 1 commit into from
Aug 30, 2021
Merged

[3.10] bpo-44449: faulthandler don't modify frame refcnt #27850

merged 1 commit into from
Aug 30, 2021

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Aug 20, 2021 •
edited by bedevere-bot
Loading

Fix a crash in the signal handler of the faulthandler module: no
longer modify the reference count of frame objects.

https://bugs.python.org/issue44449

@vstinner
bpo-44449: faulthandler don't modify frame refcnt
f8dfd90 
Fix a crash in the signal handler of the faulthandler module: no
longer modify the reference count of frame objects.
@vstinner vstinner added the needs backport to 3.9 only security fixes label Aug 20, 2021
Fidget-Spinner
Fidget-Spinner approved these changes Aug 20, 2021
View reviewed changes
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. I'm not sure how to write a test for this though.

BTW, to make bedevere/maintenance-branch-pr — Not a valid maintenance branch PR title. test pass, you can rename issue title to [3.10] bpo...

@markshannon
Copy link
Member

I don't think there is anything wring with _Py_DumpTraceback. The problem is that we are calling it from the fault handler.
Since the VM is not in a valid state (or there wouldn't be a fault), we shouldn't call any API functions from the fault handler.

@markshannon
Copy link
Member

I hadn't realized that this is for 3.10. It should be OK it that case.

@vstinner vstinner changed the title bpo-44449: faulthandler don't modify frame refcnt [3.10] bpo-44449: faulthandler don't modify frame refcnt Aug 30, 2021
@vstinner vstinner merged commit fe997e1 into python:3.10 Aug 30, 2021
@miss-islington
Copy link
Contributor

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

@vstinner vstinner deleted the traceback_refcnt branch August 30, 2021 13:24
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 30, 2021
@vstinner @miss-islington
bpo-44449: faulthandler don't modify frame refcnt (pythonGH-27850)
cb28ba8 
Fix a crash in the signal handler of the faulthandler module: no
longer modify the reference count of frame objects.
(cherry picked from commit fe997e1)

Co-authored-by: Victor Stinner <vstinner@python.org>
@bedevere-bot
Copy link

GH-28066 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Aug 30, 2021
@vstinner
Copy link
Member Author

@markshannon: In the main branch, dump_traceback() reads tstate->frame and frame->previous.

Since the VM is not in a valid state (or there wouldn't be a fault), we shouldn't call any API functions from the fault handler.

Right. Hopefully, in my experience, faulthandler is also to dump the traceback in almost all cases, even if something really bad happens. Anyway, Python is already exiting with a "fatal error", so it's not a big deal if it triggers a new crash :-D

miss-islington added a commit that referenced this pull request Aug 30, 2021
@miss-islington @vstinner
bpo-44449: faulthandler don't modify frame refcnt (GH-27850)
720aef4 
Fix a crash in the signal handler of the faulthandler module: no
longer modify the reference count of frame objects.
(cherry picked from commit fe997e1)

Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vstinner @markshannon @miss-islington @bedevere-bot @Fidget-Spinner @the-knights-who-say-ni