Skip to content

Commit dee85ff

Browse files
committed
gh-157519: Don't defer deallocation on an unknown stack
`_Py_RecursionLimit_GetMargin()` now reports plenty of margin when the stack pointer is far outside the stack limits, the same rule `_Py_CheckRecursiveCall()` uses to detect stack switching. Previously `_Py_Dealloc()` deferred every GC object to the trash queue on such a stack and the chain was never destroyed.
1 parent ac86e12 commit dee85ff

3 files changed

Lines changed: 60 additions & 0 deletions

File tree

Include/internal/pycore_pystate.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,17 @@ _Py_RecursionLimit_GetMargin(PyThreadState *tstate)
339339
assert(_tstate->c_stack_hard_limit != 0);
340340
intptr_t here_addr = _Py_get_machine_stack_pointer();
341341
#if _Py_STACK_GROWS_DOWN
342+
if (here_addr < (intptr_t)(_tstate->c_stack_hard_limit - _PyOS_STACK_MARGIN_BYTES)) {
343+
// Far out of bounds -> assume stack switching has occurred.
344+
// Report plenty of margin so that _Py_Dealloc() does not defer
345+
// objects to a chain that would never be destroyed.
346+
return _PyOS_STACK_MARGIN;
347+
}
342348
return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, here_addr - (intptr_t)_tstate->c_stack_soft_limit, _PyOS_STACK_MARGIN_SHIFT);
343349
#else
350+
if (here_addr > (intptr_t)(_tstate->c_stack_hard_limit + _PyOS_STACK_MARGIN_BYTES)) {
351+
return _PyOS_STACK_MARGIN;
352+
}
344353
return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, (intptr_t)_tstate->c_stack_soft_limit - here_addr, _PyOS_STACK_MARGIN_SHIFT);
345354
#endif
346355
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix a memory leak when Python runs on a C stack the interpreter does not know
2+
about, such as user-space threads, fibers or coroutines. :c:func:`Py_DECREF`
3+
deferred the deallocation of every garbage-collected object to the trashcan
4+
and never freed them. Objects are now deallocated immediately on such stacks,
5+
matching how the C recursion check already treats them.

Modules/_testinternalcapi.c

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3082,6 +3082,51 @@ test_threadstate_set_stack_protection(PyObject *self, PyObject *Py_UNUSED(args))
30823082
Py_RETURN_NONE;
30833083
}
30843084

3085+
3086+
// gh-157519: _Py_Dealloc() must not defer objects to tstate->delete_later
3087+
// when the stack pointer is outside the known stack (user-space threads),
3088+
// since the chain would never be destroyed.
3089+
static PyObject *
3090+
test_dealloc_on_unknown_stack(PyObject *self, PyObject *Py_UNUSED(args))
3091+
{
3092+
PyThreadState *tstate = PyThreadState_GET();
3093+
assert(!PyErr_Occurred());
3094+
if (tstate->delete_later != NULL) {
3095+
PyErr_SetString(PyExc_AssertionError,
3096+
"delete_later is not empty before the test");
3097+
return NULL;
3098+
}
3099+
3100+
// Set the stack limits far away from the actual stack pointer
3101+
size_t size = _PyOS_MIN_STACK_SIZE;
3102+
uintptr_t here_addr = _Py_get_machine_stack_pointer();
3103+
#if _Py_STACK_GROWS_DOWN
3104+
void *start = (void *)(here_addr + 64 * _PyOS_STACK_MARGIN_BYTES);
3105+
#else
3106+
void *start = (void *)(here_addr - 64 * _PyOS_STACK_MARGIN_BYTES - size);
3107+
#endif
3108+
if (PyUnstable_ThreadState_SetStackProtection(tstate, start, size) < 0) {
3109+
return NULL;
3110+
}
3111+
3112+
PyObject *result = NULL;
3113+
PyObject *list = PyList_New(0);
3114+
if (list == NULL) {
3115+
goto done;
3116+
}
3117+
Py_DECREF(list);
3118+
if (tstate->delete_later != NULL) {
3119+
PyErr_SetString(PyExc_AssertionError,
3120+
"_Py_Dealloc() deferred an object on an unknown stack");
3121+
goto done;
3122+
}
3123+
result = Py_NewRef(Py_None);
3124+
3125+
done:
3126+
PyUnstable_ThreadState_ResetStackProtection(tstate);
3127+
return result;
3128+
}
3129+
30853130
#define NUM_GUARDS 100
30863131

30873132
static PyObject *
@@ -3389,6 +3434,7 @@ static PyMethodDef module_functions[] = {
33893434
{"module_get_gc_hooks", module_get_gc_hooks, METH_O},
33903435
{"test_threadstate_set_stack_protection",
33913436
test_threadstate_set_stack_protection, METH_NOARGS},
3437+
{"test_dealloc_on_unknown_stack", test_dealloc_on_unknown_stack, METH_NOARGS},
33923438
{"_pyerr_setkeyerror", _pyerr_setkeyerror, METH_O},
33933439
{"test_interp_guard_countdown", test_interp_guard_countdown, METH_NOARGS},
33943440
{"test_interp_view_countdown", test_interp_view_countdown, METH_NOARGS},

0 commit comments

Comments
 (0)