Skip to content

Commit 2182a71

Browse files
authored
[3.11] GH-99729: Unlink frames before clearing them (#100047)
1 parent 3fae04b commit 2182a71

File tree

4 files changed

+56
-10
lines changed

4 files changed

+56
-10
lines changed

Lib/test/test_frame.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22
import re
33
import sys
44
import textwrap
5+
import threading
56
import types
67
import unittest
78
import weakref
89

910
from test import support
11+
from test.support import threading_helper
1012
from test.support.script_helper import assert_python_ok
1113

1214

@@ -325,6 +327,46 @@ def f():
325327
if old_enabled:
326328
gc.enable()
327329

330+
@support.cpython_only
331+
@threading_helper.requires_working_threading()
332+
def test_sneaky_frame_object_teardown(self):
333+
334+
class SneakyDel:
335+
def __del__(self):
336+
"""
337+
Stash a reference to the entire stack for walking later.
338+
339+
It may look crazy, but you'd be surprised how common this is
340+
when using a test runner (like pytest). The typical recipe is:
341+
ResourceWarning + -Werror + a custom sys.unraisablehook.
342+
"""
343+
nonlocal sneaky_frame_object
344+
sneaky_frame_object = sys._getframe()
345+
346+
class SneakyThread(threading.Thread):
347+
"""
348+
A separate thread isn't needed to make this code crash, but it does
349+
make crashes more consistent, since it means sneaky_frame_object is
350+
backed by freed memory after the thread completes!
351+
"""
352+
353+
def run(self):
354+
"""Run SneakyDel.__del__ as this frame is popped."""
355+
ref = SneakyDel()
356+
357+
sneaky_frame_object = None
358+
t = SneakyThread()
359+
t.start()
360+
t.join()
361+
# sneaky_frame_object can be anything, really, but it's crucial that
362+
# SneakyThread.run's frame isn't anywhere on the stack while it's being
363+
# torn down:
364+
self.assertIsNotNone(sneaky_frame_object)
365+
while sneaky_frame_object is not None:
366+
self.assertIsNot(
367+
sneaky_frame_object.f_code, SneakyThread.run.__code__
368+
)
369+
sneaky_frame_object = sneaky_frame_object.f_back
328370

329371
if __name__ == "__main__":
330372
unittest.main()
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix an issue that could cause frames to be visible to Python code as they
2+
are being torn down, possibly leading to memory corruption or hard crashes
3+
of the interpreter.

Python/ceval.c

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,14 +1617,6 @@ trace_function_exit(PyThreadState *tstate, _PyInterpreterFrame *frame, PyObject
16171617
return 0;
16181618
}
16191619

1620-
static _PyInterpreterFrame *
1621-
pop_frame(PyThreadState *tstate, _PyInterpreterFrame *frame)
1622-
{
1623-
_PyInterpreterFrame *prev_frame = frame->previous;
1624-
_PyEvalFrameClearAndPop(tstate, frame);
1625-
return prev_frame;
1626-
}
1627-
16281620
/* It is only between the PRECALL instruction and the following CALL,
16291621
* that this has any meaning.
16301622
*/
@@ -2441,7 +2433,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
24412433
DTRACE_FUNCTION_EXIT();
24422434
_Py_LeaveRecursiveCallTstate(tstate);
24432435
if (!frame->is_entry) {
2444-
frame = cframe.current_frame = pop_frame(tstate, frame);
2436+
// GH-99729: We need to unlink the frame *before* clearing it:
2437+
_PyInterpreterFrame *dying = frame;
2438+
frame = cframe.current_frame = dying->previous;
2439+
_PyEvalFrameClearAndPop(tstate, dying);
24452440
_PyFrame_StackPush(frame, retval);
24462441
goto resume_frame;
24472442
}
@@ -5833,7 +5828,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
58335828
assert(tstate->cframe->current_frame == frame->previous);
58345829
return NULL;
58355830
}
5836-
frame = cframe.current_frame = pop_frame(tstate, frame);
5831+
// GH-99729: We need to unlink the frame *before* clearing it:
5832+
_PyInterpreterFrame *dying = frame;
5833+
frame = cframe.current_frame = dying->previous;
5834+
_PyEvalFrameClearAndPop(tstate, dying);
58375835

58385836
resume_with_error:
58395837
SET_LOCALS_FROM_FRAME();

Python/frame.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ _PyFrame_Clear(_PyInterpreterFrame *frame)
123123
* to have cleared the enclosing generator, if any. */
124124
assert(frame->owner != FRAME_OWNED_BY_GENERATOR ||
125125
_PyFrame_GetGenerator(frame)->gi_frame_state == FRAME_CLEARED);
126+
// GH-99729: Clearing this frame can expose the stack (via finalizers). It's
127+
// crucial that this frame has been unlinked, and is no longer visible:
128+
assert(_PyThreadState_GET()->cframe->current_frame != frame);
126129
if (frame->frame_obj) {
127130
PyFrameObject *f = frame->frame_obj;
128131
frame->frame_obj = NULL;

0 commit comments

Comments
 (0)