GH-99729: Unlink frames before clearing them (GH-100030) · python/cpython@b72014c

GitHub

Original file line numberDiff line numberDiff line change@@ -2,6 +2,7 @@

22importre

33importsys

44importtextwrap

5+importthreading

56importtypes

67importunittest

78importweakref

@@ -11,6 +12,7 @@

1112_testcapi=None

12131314fromtestimportsupport

15+fromtest.supportimportthreading_helper

1416fromtest.support.script_helperimportassert_python_ok

15171618@@ -329,6 +331,46 @@ def f():

329331ifold_enabled:

330332gc.enable()

[email protected]_only

335+@threading_helper.requires_working_threading()

336+deftest_sneaky_frame_object_teardown(self):

337+338+classSneakyDel:

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+nonlocalsneaky_frame_object

348+sneaky_frame_object=sys._getframe()

349+350+classSneakyThread(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+defrun(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+whilesneaky_frame_objectisnotNone:

370+self.assertIsNot(

371+sneaky_frame_object.f_code, SneakyThread.run.__code__

372+ )

373+sneaky_frame_object=sneaky_frame_object.f_back

[email protected](_testcapiisNone, 'need _testcapi')

334376classTestCAPI(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.

Original file line numberDiff line numberDiff line change@@ -619,7 +619,10 @@ dummy_func(

619619DTRACE_FUNCTION_EXIT();

620620_Py_LeaveRecursiveCallPy(tstate);

621621assert(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 }

Original file line numberDiff line numberDiff line change@@ -1009,14 +1009,6 @@ trace_function_exit(PyThreadState *tstate, _PyInterpreterFrame *frame, PyObject

10091009return0;

10101010}

101110111012-static_PyInterpreterFrame*

1013-pop_frame(PyThreadState*tstate, _PyInterpreterFrame*frame)

1014-{

1015-_PyInterpreterFrame*prev_frame=frame->previous;

1016-_PyEvalFrameClearAndPop(tstate, frame);

1017-returnprev_frame;

1018-}

1019-1020101210211013int_Py_CheckRecursiveCallPy(

10221014PyThreadState*tstate)

@@ -1432,7 +1424,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int

14321424assert(_PyErr_Occurred(tstate));

14331425_Py_LeaveRecursiveCallPy(tstate);

14341426assert(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);

14361431if (frame==&entry_frame) {

14371432/* Restore previous cframe and exit */

14381433tstate->cframe=cframe.previous;

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. */

128128assert(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);

130133if (frame->frame_obj) {

131134PyFrameObject*f=frame->frame_obj;

132135frame->frame_obj=NULL;