GH-116098: Remove dead frame object creation code (GH-116687)

gaogaotiantian · web-flow · commit a53cc3f49463 · 2024-03-12T23:35:28.000Z
diff --git 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
@@ -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;
