https://github.com/python/cpython/commit/a53cc3f49463e50cb3e2b839b3a82e6bf7f73fee
commit: a53cc3f49463e50cb3e2b839b3a82e6bf7f73fee
branch: main
author: Tian Gao <[email protected]>
committer: brandtbucher <[email protected]>
date: 2024-03-12T23:35:28Z
summary:
GH-116098: Remove dead frame object creation code (GH-116687)
files:
M Lib/test/test_frame.py
M Python/frame.c
diff --git a/Lib/test/test_frame.py b/Lib/test/test_frame.py
index f8812c281c2deb..8e744a1223e86f 100644
--- a/Lib/test/test_frame.py
+++ b/Lib/test/test_frame.py
@@ -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):
diff --git a/Python/frame.c b/Python/frame.c
index ddf6ef6ba5465c..f88a8f0d73d3f8 100644
--- a/Python/frame.c
+++ b/Python/frame.c
@@ -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;
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]
