Skip to content

gh-136003: Execute pre-finalization callbacks in a loop #136004

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
354e6b5
Execute early finalization handlers in a loop
ZeroIntensity Jun 26, 2025
3090524
Store Py_MAX result in a variable.
ZeroIntensity Jun 26, 2025
43038b8
Add a test in test_atexit.
ZeroIntensity Jun 26, 2025
8d4151c
Deal with it in Py_EndInterpreter() as well.
ZeroIntensity Jun 26, 2025
add4d33
Add a blurb entry.
ZeroIntensity Jun 26, 2025
3edc3a8
Check the return code in the test.
ZeroIntensity Jun 26, 2025
ec57918
Add a test for pending calls.
ZeroIntensity Jun 26, 2025
970153b
Fix tests on Windows.
ZeroIntensity Jun 26, 2025
cfd62b8
Use os.linesep.
ZeroIntensity Jul 3, 2025
859070f
Use an RW lock instead of a counter.
ZeroIntensity Jul 3, 2025
37098a0
Merge branch 'main' of https://github.com/python/cpython into fix-cir…
ZeroIntensity Jul 3, 2025
8a1aa13
Remove more counters.
ZeroIntensity Jul 3, 2025
cbcd552
Remove old artifacts (again).
ZeroIntensity Jul 9, 2025
54613a1
Final time removing artifacts.
ZeroIntensity Jul 9, 2025
9ccdb5f
(I lied)
ZeroIntensity Jul 9, 2025
9cd75b7
Atomically check if there are threads, atexit callbacks, or pending c…
ZeroIntensity Jul 9, 2025
1360059
Remove stray newline change.
ZeroIntensity Jul 9, 2025
475538a
Add a test for atexit with subinterpreters.
ZeroIntensity Jul 10, 2025
a794188
Check for os.pipe() in the test.
ZeroIntensity Jul 10, 2025
2dda7a4
Rely on stop-the-world and the GIL instead of a dedicated RW mutex.
ZeroIntensity Jul 10, 2025
f1460af
Serialize pending calls via the ceval mutex.
ZeroIntensity Jul 10, 2025
1e1301d
Only check for non-daemon threads at finalization.
ZeroIntensity Jul 10, 2025
51a20d4
Fix assertion failures on the GILful build.
ZeroIntensity Jul 24, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Include/internal/pycore_ceval.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ PyAPI_FUNC(int) _PyEval_MakePendingCalls(PyThreadState *);
# define Py_DEFAULT_RECURSION_LIMIT 1000
#endif

extern void _Py_FinishPendingCalls(PyThreadState *tstate);
extern int _Py_FinishPendingCalls(PyThreadState *tstate);
extern void _PyEval_InitState(PyInterpreterState *);
extern void _PyEval_SignalReceived(void);

Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ extern void _PyErr_DisplayException(PyObject *file, PyObject *exc);

extern void _PyThreadState_DeleteCurrent(PyThreadState *tstate);

extern void _PyAtExit_Call(PyInterpreterState *interp);
extern int _PyAtExit_Call(PyInterpreterState *interp);

extern int _Py_IsCoreInitialized(void);

Expand Down
24 changes: 24 additions & 0 deletions Lib/test/test_atexit.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,30 @@ def thready():
# want them to affect the rest of the tests.
script_helper.assert_python_ok("-c", textwrap.dedent(source))

@threading_helper.requires_working_threading()
def test_thread_created_in_atexit(self):
Comment on lines +82 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would probably make sense to extend this with a subtest to check it in a subinterpreter.

source = """
import atexit
import threading
import time


def run():
print(24)
time.sleep(1)
print(42)

@atexit.register
def start_thread():
threading.Thread(target=run).start()
"""

return_code, stdout, stderr = script_helper.assert_python_ok("-c", textwrap.dedent(source))
self.assertEqual(return_code, 0)
end = "\r\n" if os.name == "nt" else "\n"
self.assertEqual(stdout, f"24{end}42{end}".encode("utf-8"))
self.assertEqual(stderr, b"")


@support.cpython_only
class SubinterpreterTest(unittest.TestCase):
Expand Down
32 changes: 32 additions & 0 deletions Lib/test/test_capi/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from test import support
from test.support import MISSING_C_DOCSTRINGS
from test.support import import_helper
from test.support import script_helper
from test.support import threading_helper
from test.support import warnings_helper
from test.support import requires_limited_api
Expand Down Expand Up @@ -1641,6 +1642,37 @@ def subthread():

self.assertEqual(actual, int(interpid))

@threading_helper.requires_working_threading()
def test_pending_call_creates_thread(self):
Comment on lines +1645 to +1646
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, it would probably make sense to extend this with a subtest to check it in a subinterpreter.

Also, it would be good to test adding a pending call to a different interpreter during its finalization.

source = """
import _testcapi
import threading
import time


def output():
print(24)
time.sleep(1)
print(42)


def callback():
threading.Thread(target=output).start()


def create_pending_call():
time.sleep(1)
_testcapi.simple_pending_call(callback)


threading.Thread(target=create_pending_call).start()
"""
return_code, stdout, stderr = script_helper.assert_python_ok('-c', textwrap.dedent(source))
self.assertEqual(return_code, 0)
end = "\r\n" if os.name == "nt" else "\n"
self.assertEqual(stdout, f"24{end}42{end}".encode("utf-8"))
self.assertEqual(stderr, b"")


class SubinterpreterTest(unittest.TestCase):

Expand Down
7 changes: 4 additions & 3 deletions Lib/threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -1557,8 +1557,9 @@ def _shutdown():
# normally - that won't happen until the interpreter is nearly dead. So
# mark it done here.
if _main_thread._os_thread_handle.is_done() and _is_main_interpreter():
# _shutdown() was already called
return
# _shutdown() was already called, but threads might have started
# in the meantime.
return _thread_shutdown()

global _SHUTTING_DOWN
_SHUTTING_DOWN = True
Expand All @@ -1572,7 +1573,7 @@ def _shutdown():
_main_thread._os_thread_handle._set_done()

# Wait for all non-daemon threads to exit.
_thread_shutdown()
return _thread_shutdown()


def main_thread():
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix :class:`threading.Thread` objects becoming incorrectly daemon when
created from an :mod:`atexit` callback or a pending call
(:c:func:`Py_AddPendingCall`).
11 changes: 11 additions & 0 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2546,6 +2546,16 @@ toggle_reftrace_printer(PyObject *ob, PyObject *arg)
Py_RETURN_NONE;
}

static PyObject *
simple_pending_call(PyObject *self, PyObject *callable)
{
if (Py_AddPendingCall(_pending_callback, Py_NewRef(callable)) < 0) {
return NULL;
}

Py_RETURN_NONE;
}

static PyMethodDef TestMethods[] = {
{"set_errno", set_errno, METH_VARARGS},
{"test_config", test_config, METH_NOARGS},
Expand Down Expand Up @@ -2640,6 +2650,7 @@ static PyMethodDef TestMethods[] = {
{"test_atexit", test_atexit, METH_NOARGS},
{"code_offset_to_line", _PyCFunction_CAST(code_offset_to_line), METH_FASTCALL},
{"toggle_reftrace_printer", toggle_reftrace_printer, METH_O},
{"simple_pending_call", simple_pending_call, METH_O},
{NULL, NULL} /* sentinel */
};

Expand Down
6 changes: 4 additions & 2 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,7 @@ thread_shutdown(PyObject *self, PyObject *args)
{
PyThread_ident_t ident = PyThread_get_thread_ident_ex();
thread_module_state *state = get_thread_state(self);
int found_thread = 0;

for (;;) {
ThreadHandle *handle = NULL;
Expand All @@ -2353,6 +2354,7 @@ thread_shutdown(PyObject *self, PyObject *args)
HEAD_LOCK(&_PyRuntime);
struct llist_node *node;
llist_for_each_safe(node, &state->shutdown_handles) {
found_thread = 1;
ThreadHandle *cur = llist_data(node, ThreadHandle, shutdown_node);
if (cur->ident != ident) {
ThreadHandle_incref(cur);
Expand All @@ -2373,13 +2375,13 @@ thread_shutdown(PyObject *self, PyObject *args)
PyErr_FormatUnraisable("Exception ignored while joining a thread "
"in _thread._shutdown()");
ThreadHandle_decref(handle);
Py_RETURN_NONE;
return PyBool_FromLong(found_thread);
}

ThreadHandle_decref(handle);
}

Py_RETURN_NONE;
return PyBool_FromLong(found_thread);
}

PyDoc_STRVAR(shutdown_doc,
Expand Down
12 changes: 8 additions & 4 deletions Modules/atexitmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ _PyAtExit_Fini(PyInterpreterState *interp)
}
}

static void
static int
atexit_callfuncs(struct atexit_state *state)
{
assert(!PyErr_Occurred());
Expand All @@ -112,10 +112,13 @@ atexit_callfuncs(struct atexit_state *state)
{
PyErr_FormatUnraisable("Exception ignored while "
"copying atexit callbacks");
return;
return 0;
}

int called = 0;

for (Py_ssize_t i = 0; i < PyList_GET_SIZE(copy); ++i) {
called = 1;
// We don't have to worry about evil borrowed references, because
// no other threads can access this list.
PyObject *tuple = PyList_GET_ITEM(copy, i);
Expand All @@ -141,14 +144,15 @@ atexit_callfuncs(struct atexit_state *state)
atexit_cleanup(state);

assert(!PyErr_Occurred());
return called;
}


void
int
_PyAtExit_Call(PyInterpreterState *interp)
{
struct atexit_state *state = &interp->atexit;
atexit_callfuncs(state);
return atexit_callfuncs(state);
}


Expand Down
35 changes: 29 additions & 6 deletions Python/ceval_gil.c
Original file line number Diff line number Diff line change
Expand Up @@ -837,10 +837,11 @@ handle_signals(PyThreadState *tstate)
}

static int
_make_pending_calls(struct _pending_calls *pending, int32_t *p_npending)
_make_pending_calls(struct _pending_calls *pending, int32_t *p_npending, int *p_called)
{
int res = 0;
int32_t npending = -1;
int called = 0;

assert(sizeof(pending->max) <= sizeof(size_t)
&& ((size_t)pending->max) <= Py_ARRAY_LENGTH(pending->calls));
Expand Down Expand Up @@ -868,6 +869,8 @@ _make_pending_calls(struct _pending_calls *pending, int32_t *p_npending)
break;
}

called = 1;

/* having released the lock, perform the callback */
res = func(arg);
if ((flags & _Py_PENDING_RAWFREE) && arg != NULL) {
Expand All @@ -881,6 +884,7 @@ _make_pending_calls(struct _pending_calls *pending, int32_t *p_npending)

finally:
*p_npending = npending;
*p_called = called;
return res;
}

Expand Down Expand Up @@ -917,7 +921,7 @@ clear_pending_handling_thread(struct _pending_calls *pending)
}

static int
make_pending_calls(PyThreadState *tstate)
make_pending_calls_with_count(PyThreadState *tstate)
{
PyInterpreterState *interp = tstate->interp;
struct _pending_calls *pending = &interp->ceval.pending;
Expand Down Expand Up @@ -947,7 +951,8 @@ make_pending_calls(PyThreadState *tstate)
unsignal_pending_calls(tstate, interp);

int32_t npending;
if (_make_pending_calls(pending, &npending) != 0) {
int called;
if (_make_pending_calls(pending, &npending, &called) != 0) {
clear_pending_handling_thread(pending);
/* There might not be more calls to make, but we play it safe. */
signal_pending_calls(tstate, interp);
Expand All @@ -958,8 +963,9 @@ make_pending_calls(PyThreadState *tstate)
signal_pending_calls(tstate, interp);
}

int main_called = 0;
if (_Py_IsMainThread() && _Py_IsMainInterpreter(interp)) {
if (_make_pending_calls(pending_main, &npending) != 0) {
if (_make_pending_calls(pending_main, &npending, &main_called) != 0) {
clear_pending_handling_thread(pending);
/* There might not be more calls to make, but we play it safe. */
signal_pending_calls(tstate, interp);
Expand All @@ -972,6 +978,16 @@ make_pending_calls(PyThreadState *tstate)
}

clear_pending_handling_thread(pending);
return Py_MAX(called, main_called);
}

static int
make_pending_calls(PyThreadState *tstate)
{
if (make_pending_calls_with_count(tstate) < 0) {
return -1;
}

return 0;
}

Expand All @@ -994,7 +1010,7 @@ _Py_unset_eval_breaker_bit_all(PyInterpreterState *interp, uintptr_t bit)
_Py_FOR_EACH_TSTATE_END(interp);
}

void
int
_Py_FinishPendingCalls(PyThreadState *tstate)
{
_Py_AssertHoldsTstate();
Expand All @@ -1011,13 +1027,18 @@ _Py_FinishPendingCalls(PyThreadState *tstate)
#ifndef NDEBUG
int32_t npending_prev = INT32_MAX;
#endif
int called = 0;
do {
if (make_pending_calls(tstate) < 0) {
int res = make_pending_calls_with_count(tstate);
if (res < 0) {
PyObject *exc = _PyErr_GetRaisedException(tstate);
PyErr_BadInternalCall();
_PyErr_ChainExceptions1(exc);
_PyErr_Print(tstate);
}
if (res != 0) {
called = 1;
}

npending = _Py_atomic_load_int32_relaxed(&pending->npending);
if (pending_main != NULL) {
Expand All @@ -1028,6 +1049,8 @@ _Py_FinishPendingCalls(PyThreadState *tstate)
npending_prev = npending;
#endif
} while (npending > 0);

return called;
}

int
Expand Down
Loading
Loading