Skip to content

GH-97002: Prevent _PyInterpreterFrames from backing more than one PyFrameObject #97996

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

Merged
markshannon merged 4 commits into from
Oct 6, 2022
Merged

GH-97002: Prevent _PyInterpreterFrames from backing more than one PyFrameObject #97996

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce322b3
Add failing regression test
brandtbucher Oct 6, 2022
7ee32d6
Prevent frame ownership conflicts
brandtbucher Oct 6, 2022
5e0e3ad
blurb add
brandtbucher Oct 6, 2022
045658c
make patchcheck
brandtbucher Oct 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Prevent frame ownership conflicts
  • Loading branch information
@brandtbucher
brandtbucher committed Oct 6, 2022
commit 7ee32d69dea331a7b68e3636de60d7adc59ebee8
29 changes: 23 additions & 6 deletions Python/frame.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,31 @@ _PyFrame_MakeAndSetFrameObject(_PyInterpreterFrame *frame)
Py_XDECREF(error_type);
Py_XDECREF(error_value);
Py_XDECREF(error_traceback);
return NULL;
}
else {
assert(frame->owner != FRAME_OWNED_BY_FRAME_OBJECT);
assert(frame->owner != FRAME_CLEARED);
f->f_frame = frame;
frame->frame_obj = f;
PyErr_Restore(error_type, error_value, error_traceback);
PyErr_Restore(error_type, error_value, error_traceback);
if (frame->frame_obj) {
// GH-97002: How did we get into this horrible situation? Most likely,
// allocating f triggered a GC collection, which ran some code that
// *also* created the same frame... while we were in the middle of
// creating it! See test_sneaky_frame_object in test_frame.py for a
// concrete example.
//
// Regardless, just throw f away and use that frame instead, since it's
// already been exposed to user code. It's actually a bit tricky to do
// this, since we aren't backed by a real _PyInterpreterFrame anymore.
// Just pretend that we have an owned, cleared frame so frame_dealloc
// doesn't make the situation worse:
f->f_frame = (_PyInterpreterFrame *)f->_f_frame_data;
f->f_frame->owner = FRAME_CLEARED;

@andersk andersk Jun 26, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FRAME_CLEARED is a value of enum _framestate. Shouldn’t ->owner be a value of enum _frameowner instead?

GH-106156

f->f_frame->frame_obj = f;
Py_DECREF(f);
return frame->frame_obj;
}
assert(frame->owner != FRAME_OWNED_BY_FRAME_OBJECT);
assert(frame->owner != FRAME_CLEARED);
f->f_frame = frame;
frame->frame_obj = f;
return f;
}

Expand Down