Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 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
cf2dc1e
Merge branch 'main' into fix-circular-finalization
ZeroIntensity Sep 17, 2025
6ea3792
Merge branch 'main' of https://github.com/python/cpython into fix-cir…
ZeroIntensity Sep 17, 2025
8c12e6c
Fix merge conflict artifact.
ZeroIntensity Sep 17, 2025
8b87014
Improve comments.
ZeroIntensity Sep 17, 2025
e57bfde
Merge branch 'fix-circular-finalization' of https://github.com/zeroin…
ZeroIntensity Sep 17, 2025
e202848
Fix ordering of finalization calls.
ZeroIntensity Sep 17, 2025
0e66c88
Finalize subinterpreters as a pre-finalization check.
ZeroIntensity Sep 18, 2025
c8cac69
Merge branch 'main' of https://github.com/python/cpython into fix-cir…
ZeroIntensity Sep 18, 2025
2ab28b0
Test _PyEval_AddPendingCall() instead of Py_AddPendingCall()
ZeroIntensity Sep 18, 2025
c9d5f4c
Add a test for subinterpreters.
ZeroIntensity Sep 18, 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
Prev Previous commit
Next Next commit
Use an RW lock instead of a counter.
  • Loading branch information
ZeroIntensity committed Jul 3, 2025
commit 859070f1f3118042fdff6aeac5f8fa93354991b7
4 changes: 4 additions & 0 deletions Include/internal/pycore_interp_structs.h
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,10 @@ struct _is {
# endif
#endif

/* The "pre-finalization" lock, which protects against things like starting
* threads. The exclusive writer is only used when the interpreter finalizes. */
_PyRWMutex prefini_mutex;

/* the initial PyInterpreterState.threads.head */
_PyThreadStateImpl _initial_thread;
// _initial_thread should be the last field of PyInterpreterState.
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 int _PyAtExit_Call(PyInterpreterState *interp);
extern void _PyAtExit_Call(PyInterpreterState *interp);

extern int _Py_IsCoreInitialized(void);

Expand Down
2 changes: 1 addition & 1 deletion Lib/threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -1573,7 +1573,7 @@ def _shutdown():
_main_thread._os_thread_handle._set_done()

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


def main_thread():
Expand Down
23 changes: 13 additions & 10 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,13 @@ ThreadHandle_get_os_handle(ThreadHandle *handle, PyThread_handle_t *os_handle)
}

static void
add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle)
add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle, PyInterpreterState *interp)
{
_PyRWMutex_RLock(&interp->prefini_mutex);
HEAD_LOCK(&_PyRuntime);
llist_insert_tail(&state->shutdown_handles, &handle->shutdown_node);
HEAD_UNLOCK(&_PyRuntime);
_PyRWMutex_RUnlock(&interp->prefini_mutex);
}

static void
Expand All @@ -195,13 +197,16 @@ clear_shutdown_handles(thread_module_state *state)
}

static void
remove_from_shutdown_handles(ThreadHandle *handle)
remove_from_shutdown_handles(ThreadHandle *handle, PyInterpreterState *interp)
{
assert(interp != NULL);
_PyRWMutex_RLock(&interp->prefini_mutex);
HEAD_LOCK(&_PyRuntime);
if (handle->shutdown_node.next != NULL) {
llist_remove(&handle->shutdown_node);
}
HEAD_UNLOCK(&_PyRuntime);
_PyRWMutex_RUnlock(&interp->prefini_mutex);
}

static ThreadHandle *
Expand Down Expand Up @@ -309,7 +314,7 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state)
handle->mutex = (PyMutex){_Py_UNLOCKED};
_PyEvent_Notify(&handle->thread_is_exiting);
llist_remove(node);
remove_from_shutdown_handles(handle);
remove_from_shutdown_handles(handle, _PyInterpreterState_GET());
}
}

Expand Down Expand Up @@ -392,7 +397,7 @@ thread_run(void *boot_raw)

exit:
// Don't need to wait for this thread anymore
remove_from_shutdown_handles(handle);
remove_from_shutdown_handles(handle, _PyInterpreterState_GET());

_PyEvent_Notify(&handle->thread_is_exiting);
ThreadHandle_decref(handle);
Expand Down Expand Up @@ -1863,12 +1868,12 @@ do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args,
// Add the handle before starting the thread to avoid adding a handle
// to a thread that has already finished (i.e. if the thread finishes
// before the call to `ThreadHandle_start()` below returns).
add_to_shutdown_handles(state, handle);
add_to_shutdown_handles(state, handle, interp);
}

if (ThreadHandle_start(handle, func, args, kwargs) < 0) {
if (!daemon) {
remove_from_shutdown_handles(handle);
remove_from_shutdown_handles(handle, _PyInterpreterState_GET());
}
return -1;
}
Expand Down Expand Up @@ -2345,7 +2350,6 @@ 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 @@ -2354,7 +2358,6 @@ 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 @@ -2375,13 +2378,13 @@ thread_shutdown(PyObject *self, PyObject *args)
PyErr_FormatUnraisable("Exception ignored while joining a thread "
"in _thread._shutdown()");
ThreadHandle_decref(handle);
return PyBool_FromLong(found_thread);
Py_RETURN_NONE;
}

ThreadHandle_decref(handle);
}

return PyBool_FromLong(found_thread);
Py_RETURN_NONE;
}

PyDoc_STRVAR(shutdown_doc,
Expand Down
20 changes: 11 additions & 9 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 int
static void
atexit_callfuncs(struct atexit_state *state)
{
assert(!PyErr_Occurred());
Expand All @@ -112,13 +112,10 @@ atexit_callfuncs(struct atexit_state *state)
{
PyErr_FormatUnraisable("Exception ignored while "
"copying atexit callbacks");
return 0;
return;
}

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 @@ -144,15 +141,14 @@ atexit_callfuncs(struct atexit_state *state)
atexit_cleanup(state);

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


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


Expand Down Expand Up @@ -200,9 +196,15 @@ atexit_register(PyObject *module, PyObject *args, PyObject *kwargs)
return NULL;
}

PyInterpreterState *interp = _PyInterpreterState_GET();
assert(interp != NULL);
struct atexit_state *state = get_atexit_state();
_PyRWMutex_RLock(&interp->prefini_mutex);
// atexit callbacks go in a LIFO order
if (PyList_Insert(state->callbacks, 0, callback) < 0)
int res = PyList_Insert(state->callbacks, 0, callback);
_PyRWMutex_RUnlock(&interp->prefini_mutex);

if (res < 0)
{
Py_DECREF(callback);
return NULL;
Expand Down
27 changes: 11 additions & 16 deletions Python/ceval_gil.c
Original file line number Diff line number Diff line change
Expand Up @@ -785,8 +785,10 @@ _PyEval_AddPendingCall(PyInterpreterState *interp,
}

PyMutex_Lock(&pending->mutex);
_PyRWMutex_RLock(&interp->prefini_mutex);
_Py_add_pending_call_result result =
_push_pending_call(pending, func, arg, flags);
_PyRWMutex_RUnlock(&interp->prefini_mutex);
PyMutex_Unlock(&pending->mutex);

if (main_only) {
Expand Down Expand Up @@ -837,11 +839,10 @@ handle_signals(PyThreadState *tstate)
}

static int
_make_pending_calls(struct _pending_calls *pending, int32_t *p_npending, int *p_called)
_make_pending_calls(struct _pending_calls *pending, int32_t *p_npending)
{
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 @@ -869,8 +870,6 @@ _make_pending_calls(struct _pending_calls *pending, int32_t *p_npending, int *p_
break;
}

called = 1;

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

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

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

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

int32_t npending;
int called;
if (_make_pending_calls(pending, &npending, &called) != 0) {
if (_make_pending_calls(pending, &npending) != 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 @@ -963,9 +960,8 @@ make_pending_calls_with_count(PyThreadState *tstate)
signal_pending_calls(tstate, interp);
}

int main_called = 0;
if (_Py_IsMainThread() && _Py_IsMainInterpreter(interp)) {
if (_make_pending_calls(pending_main, &npending, &main_called) != 0) {
if (_make_pending_calls(pending_main, &npending) != 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 @@ -978,17 +974,16 @@ make_pending_calls_with_count(PyThreadState *tstate)
}

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

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

return 0;
_PyRWMutex_RLock(&tstate->interp->prefini_mutex);
int res = make_pending_calls_lock_held(tstate);
_PyRWMutex_RUnlock(&tstate->interp->prefini_mutex);
return res;
}


Expand Down
25 changes: 9 additions & 16 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ static PyStatus init_android_streams(PyThreadState *tstate);
#if defined(__APPLE__) && HAS_APPLE_SYSTEM_LOG
static PyStatus init_apple_streams(PyThreadState *tstate);
#endif
static int wait_for_thread_shutdown(PyThreadState *tstate);
static void wait_for_thread_shutdown(PyThreadState *tstate);
static void finalize_subinterpreters(void);
static void call_ll_exitfuncs(_PyRuntimeState *runtime);

Expand Down Expand Up @@ -2010,15 +2010,11 @@ make_pre_finalization_calls(PyThreadState *tstate)
* could start a thread or vice versa. To ensure that we properly clean
* call everything, we run these in a loop until none of them run anything. */
for (;;) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it make sense to add an arbitrary limit to detect infinite loop? For example, log an error after 16 attemps.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, it would prevent deadlocks in rare cases, but it would cause crashes in other equally rare cases. Maybe it would be better to emit a fatal error when there are too many iterations?

int called = 0;

// Wrap up existing "threading"-module-created, non-daemon threads.
int threads_joined = wait_for_thread_shutdown(tstate);
called = Py_MAX(called, threads_joined);
wait_for_thread_shutdown(tstate);
Comment on lines +2085 to +2086
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't necessarily need to worry about it here, but it would probably also be worth waiting here for the interpreter to not be "running main".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let's do that in a follow-up (probably tomorrow).


// Make any remaining pending calls.
int made_pending_calls = _Py_FinishPendingCalls(tstate);
called = Py_MAX(called, made_pending_calls);
_Py_FinishPendingCalls(tstate);

/* The interpreter is still entirely intact at this point, and the
* exit funcs may be relying on that. In particular, if some thread
Expand All @@ -2030,9 +2026,9 @@ make_pre_finalization_calls(PyThreadState *tstate)
* the threads created via Threading.
*/

int called_atexit = _PyAtExit_Call(tstate->interp);
called = Py_MAX(called, called_atexit);
_PyAtExit_Call(tstate->interp);

_PyRWMutex_Unlock(&tstate->interp->prefini_mutex);
if (called == 0) {
break;
}
Expand Down Expand Up @@ -3456,7 +3452,7 @@ Py_ExitStatusException(PyStatus status)
the threading module was imported in the first place.
The shutdown routine will wait until all non-daemon
"threading" threads have completed. */
static int
static void
wait_for_thread_shutdown(PyThreadState *tstate)
{
PyObject *result;
Expand All @@ -3466,19 +3462,16 @@ wait_for_thread_shutdown(PyThreadState *tstate)
PyErr_FormatUnraisable("Exception ignored on threading shutdown");
}
/* else: threading not imported */
return 0;
return;
}
int called = 0;
result = PyObject_CallMethodNoArgs(threading, &_Py_ID(_shutdown));
if (result == NULL) {
PyErr_FormatUnraisable("Exception ignored on threading shutdown");
}
else {
assert(PyBool_Check(result) && _Py_IsImmortal(result));
called = result == Py_True;
}
Py_XDECREF(result);
Py_DECREF(threading);
return called;
return;
}

int Py_AtExit(void (*func)(void))
Expand Down