Skip to content

Commit b72014c

Browse files
authored
GH-99729: Unlink frames before clearing them (GH-100030)
1 parent 85d5a7e commit b72014c

File tree

6 files changed

+60
-11
lines changed

6 files changed

+60
-11
lines changed

Lib/test/test_frame.py

+42
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import re
33
import sys
44
import textwrap
5+
import threading
56
import types
67
import unittest
78
import weakref
@@ -11,6 +12,7 @@
1112
_testcapi = None
1213

1314
from test import support
15+
from test.support import threading_helper
1416
from test.support.script_helper import assert_python_ok
1517

1618

@@ -329,6 +331,46 @@ def f():
329331
if old_enabled:
330332
gc.enable()
331333

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

333375
@unittest.skipIf(_testcapi is None, 'need _testcapi')
334376
class TestCAPI(unittest.TestCase):
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/bytecodes.c

+4-1
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,10 @@ dummy_func(
619619
DTRACE_FUNCTION_EXIT();
620620
_Py_LeaveRecursiveCallPy(tstate);
621621
assert(frame != &entry_frame);
622-
frame = cframe.current_frame = pop_frame(tstate, frame);
622+
// GH-99729: We need to unlink the frame *before* clearing it:
623+
_PyInterpreterFrame *dying = frame;
624+
frame = cframe.current_frame = dying->previous;
625+
_PyEvalFrameClearAndPop(tstate, dying);
623626
_PyFrame_StackPush(frame, retval);
624627
goto resume_frame;
625628
}

Python/ceval.c

+4-9
Original file line numberDiff line numberDiff line change
@@ -1009,14 +1009,6 @@ trace_function_exit(PyThreadState *tstate, _PyInterpreterFrame *frame, PyObject
10091009
return 0;
10101010
}
10111011

1012-
static _PyInterpreterFrame *
1013-
pop_frame(PyThreadState *tstate, _PyInterpreterFrame *frame)
1014-
{
1015-
_PyInterpreterFrame *prev_frame = frame->previous;
1016-
_PyEvalFrameClearAndPop(tstate, frame);
1017-
return prev_frame;
1018-
}
1019-
10201012

10211013
int _Py_CheckRecursiveCallPy(
10221014
PyThreadState *tstate)
@@ -1432,7 +1424,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
14321424
assert(_PyErr_Occurred(tstate));
14331425
_Py_LeaveRecursiveCallPy(tstate);
14341426
assert(frame != &entry_frame);
1435-
frame = cframe.current_frame = pop_frame(tstate, frame);
1427+
// GH-99729: We need to unlink the frame *before* clearing it:
1428+
_PyInterpreterFrame *dying = frame;
1429+
frame = cframe.current_frame = dying->previous;
1430+
_PyEvalFrameClearAndPop(tstate, dying);
14361431
if (frame == &entry_frame) {
14371432
/* Restore previous cframe and exit */
14381433
tstate->cframe = cframe.previous;

Python/frame.c

+3
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@ _PyFrame_Clear(_PyInterpreterFrame *frame)
127127
* to have cleared the enclosing generator, if any. */
128128
assert(frame->owner != FRAME_OWNED_BY_GENERATOR ||
129129
_PyFrame_GetGenerator(frame)->gi_frame_state == FRAME_CLEARED);
130+
// GH-99729: Clearing this frame can expose the stack (via finalizers). It's
131+
// crucial that this frame has been unlinked, and is no longer visible:
132+
assert(_PyThreadState_GET()->cframe->current_frame != frame);
130133
if (frame->frame_obj) {
131134
PyFrameObject *f = frame->frame_obj;
132135
frame->frame_obj = NULL;

Python/generated_cases.c.h

+4-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)