Skip to content

Commit

Permalink
GH-116098: Remove dead frame object creation code (GH-116687)
Browse files Browse the repository at this point in the history
  • Loading branch information
gaogaotiantian authored Mar 12, 2024
1 parent 149f7f7 commit a53cc3f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 83 deletions.
65 changes: 0 additions & 65 deletions Lib/test/test_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,71 +293,6 @@ def gen():
""")
assert_python_ok("-c", code)

@support.cpython_only
@unittest.skipIf(Py_GIL_DISABLED, "test requires precise GC scheduling")
def test_sneaky_frame_object(self):

def trace(frame, event, arg):
"""
Don't actually do anything, just force a frame object to be created.
"""

def callback(phase, info):
"""
Yo dawg, I heard you like frames, so I'm allocating a frame while
you're allocating a frame, so you can have a frame while you have a
frame!
"""
nonlocal sneaky_frame_object
sneaky_frame_object = sys._getframe().f_back.f_back
# We're done here:
gc.callbacks.remove(callback)

def f():
while True:
yield

old_threshold = gc.get_threshold()
old_callbacks = gc.callbacks[:]
old_enabled = gc.isenabled()
old_trace = sys.gettrace()
try:
# Stop the GC for a second while we set things up:
gc.disable()
# Create a paused generator:
g = f()
next(g)
# Move all objects to the oldest generation, and tell the GC to run
# on the *very next* allocation:
gc.collect()
gc.set_threshold(1, 0, 0)
sys._clear_internal_caches()
# Okay, so here's the nightmare scenario:
# - We're tracing the resumption of a generator, which creates a new
# frame object.
# - The allocation of this frame object triggers a collection
# *before* the frame object is actually created.
# - During the collection, we request the exact same frame object.
# This test does it with a GC callback, but in real code it would
# likely be a trace function, weakref callback, or finalizer.
# - The collection finishes, and the original frame object is
# created. We now have two frame objects fighting over ownership
# of the same interpreter frame!
sys.settrace(trace)
gc.callbacks.append(callback)
sneaky_frame_object = None
gc.enable()
next(g)
# g.gi_frame should be the frame object from the callback (the
# one that was *requested* second, but *created* first):
self.assertIs(g.gi_frame, sneaky_frame_object)
finally:
gc.set_threshold(*old_threshold)
gc.callbacks[:] = old_callbacks
sys.settrace(old_trace)
if old_enabled:
gc.enable()

@support.cpython_only
@threading_helper.requires_working_threading()
def test_sneaky_frame_object_teardown(self):
Expand Down
27 changes: 9 additions & 18 deletions Python/frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,15 @@ _PyFrame_MakeAndSetFrameObject(_PyInterpreterFrame *frame)
return NULL;
}
PyErr_SetRaisedException(exc);
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;
f->f_frame->frame_obj = f;
Py_DECREF(f);
return frame->frame_obj;
}

// GH-97002: There was a time when a frame object could be created when we
// are allocating the new frame object f above, so frame->frame_obj would
// be assigned already. That path does not exist anymore. We won't call any
// Python code in this function and garbage collection will not run.
// Notice that _PyFrame_New_NoTrack() can potentially raise a MemoryError,
// but it won't allocate a traceback until the frame unwinds, so we are safe
// here.
assert(frame->frame_obj == NULL);
assert(frame->owner != FRAME_OWNED_BY_FRAME_OBJECT);
assert(frame->owner != FRAME_CLEARED);
f->f_frame = frame;
Expand Down

0 comments on commit a53cc3f

Please sign in to comment.