Repository navigation
Shim frames can be accessed during frame teardown (via tstate->cframe->current_frame) #100126
Description
Activity
Tests of urllib3 detected a segmentation fault on 3.12.0a3 too.
https://github.com/urllib3/urllib3/actions/runs/3720502510/jobs/6310016874Fatal Python error: Segmentation faulton Linux andWindows fatal exception: access violationon Windows happened in one test function.The function succeeded on macOS, but
Fatal Python error: Bus errorhappened in a different one.b72014c is the first commit where the segfault in urllib3's tests happens.
Cc. @brandtbucher
I'll look into this too. No promises that I'll have it figured out before the holiday weekend, but it'll be my main priority starting next Tuesday.
Not sure if #99110 is related... hard to tell without a reproducer.
Here is a reproducer with only
pytest:# pytest crasher.py import socket def test_foo(): _ = socket.socket()
Indeed, this looks eerily similar to #99729.
Found the issue.
PyEval_GetGlobalsreturnststate->cframe->current_frame->f_globals. When tearing a frame down,current_framemay be a shim frame. Shim frames have no globals, so the code crashes.Looks like in general, it is not safe for
current_frameto be a shim frame. Too many things can break.Minimal reproducer, no
pytest:# python crasher.py import operator class DelRaises: def __del__(self): assert False def f(): local = DelRaises() # Call from C, so there is a shim frame above f: operator.call(f)
- changed the title
[-]Regression in 3.12a3 - segfault in interpreter trampoline[/-][+]Shim frames can be accessed during frame teardown (via `tstate->cframe->current_frame`)[/+]on Dec 29, 2022 I'm honestly not sure what to do here.
Perhaps, before tearing down a frame, if our previous frame is a shim frame, we should set
tstate->cframeto becframe.previous. But that's adding code to a perf-sensitive path, and it also makes the interaction withINTERPRETER_EXITawkward, since it tries to do the same thing.The best option is probably just to add more
while (frame && _PyFrame_IsIncomplete(frame)) {frame = frame->previous;}loops to all of the affected areas. It probably makes sense to just add a helper function for this (_PyThreadState_GetInterpreterFrame?), rather than continuing to sprinkle them everywhere.Thoughts, @markshannon? (I don't think you're out this week... feel free to ignore if so.)
Also going to tag as
release-blocker(for now).@brandtbucher what's the state of this issue (and the attached fix for it)? Should this hold up 3.12.0a4?
Sorry, just got back from vacation and thought it was merged while I was gone. I'm in a meeting right now, but I have one small review comment to apply, then I'll merge.
If it's not too much trouble, I'd like this to make it into the next release. It's not too hard to trigger using frameworks like
pytest, so it would be nice to have non-crashy test coverage of the next alpha out in the wild.@Yhg1s, this is done!
Reacted by Illia Volochii- moved this from Todo to Done in Release and Deferred blockers 🚫
on Jan 9, 2023
Crash report
In jaraco/jaraco.net#5, I've captured a failure that's emerged as Github updated Python 3.12 from a2 to a3 on Linux. The calling code is using ctypes to call libc functions.
I don't yet have a minimal reproducer. I'm unable to replicate the issue locally as I don't yet have Linux with a3.
Linked PRs