8000 gh-116522: Stop the world before fork() and during shutdown (#116607) · python/cpython@e728303 · GitHub
[go: up one dir, main page]
Skip to content

Commit e728303

Browse files
colesburycolesbury
authored
gh-116522: Stop the world before fork() and during shutdown (#116607)
This changes the free-threaded build to perform a stop-the-world pause before deleting other thread states when forking and during shutdown. This fixes some crashes when using multiprocessing and during shutdown when running with `PYTHON_GIL=0`. This also changes `PyOS_BeforeFork` to acquire the runtime lock (i.e., `HEAD_LOCK(&_PyRuntime)`) before forking to ensure that data protected by the runtime lock (and not just the GIL or stop-the-world) is in a consistent state before forking.
1 parent 1f8b24e commit e728303

File tree

3 files changed

+30
-7
lines changed

3 files changed

+30
-7
lines changed

‎Modules/posixmodule.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -613,11 +613,16 @@ PyOS_BeforeFork(void)
613613
run_at_forkers(interp->before_forkers, 1);
614614

615615
_PyImport_AcquireLock(interp);
616+
_PyEval_StopTheWorldAll(&_PyRuntime);
617+
HEAD_LOCK(&_PyRuntime);
616618
}
617619

618620
void
619621
PyOS_AfterFork_Parent(void)
620622
{
623+
HEAD_UNLOCK(&_PyRuntime);
624+
_PyEval_StartTheWorldAll(&_PyRuntime);
625+
621626
PyInterpreterState *interp = _PyInterpreterState_GET();
622627
if (_PyImport_ReleaseLock(interp) <= 0) {
623628
Py_FatalError("failed releasing import lock after fork");
@@ -632,6 +637,7 @@ PyOS_AfterFork_Child(void)
632637
PyStatus status;
633638
_PyRuntimeState *runtime = &_PyRuntime;
634639

640+
// re-creates runtime->interpreters.mutex (HEAD_UNLOCK)
635641
status = _PyRuntimeState_ReInitThreads(runtime);
636642
if (_PyStatus_EXCEPTION(status)) {
637643
goto fatal_error;
@@ -7731,10 +7737,15 @@ os_register_at_fork_impl(PyObject *module, PyObject *before,
77317737
// running in the process. Best effort, silent if unable to count threads.
77327738
// Constraint: Quick. Never overcounts. Never leaves an error set.
77337739
//
7734-
// This code might do an import, thus acquiring the import lock, which
7735-
// PyOS_BeforeFork() also does. As this should only be called from
7736-
// the parent process, it is in the same thread so that works.
7737-
static void warn_about_fork_with_threads(const char* name) {
7740+
// This should only be called from the parent process after
7741+
// PyOS_AfterFork_Parent().
7742+
static void
7743+
warn_about_fork_with_threads(const char* name)
7744+
{
7745+
// It's not safe to issue the warning while the world is stopped, because
7746+
// other threads might be holding locks that we need, which would deadlock.
7747+
assert(!_PyRuntime.stoptheworld.world_stopped);
7748+
77387749
// TODO: Consider making an `os` module API to return the current number
77397750
// of threads in the process. That'd presumably use this platform code but
77407751
// raise an error rather than using the inaccurate fallback.
@@ -7858,9 +7869,10 @@ os_fork1_impl(PyObject *module)
78587869
/* child: this clobbers and resets the import lock. */
78597870
PyOS_AfterFork_Child();
78607871
} else {
7861-
warn_about_fork_with_threads("fork1");
78627872
/* parent: release the import lock. */
78637873
PyOS_AfterFork_Parent();
7874+
// After PyOS_AfterFork_Parent() starts the world to avoid deadlock.
7875+
warn_about_fork_with_threads("fork1");
78647876
}
78657877
if (pid == -1) {
78667878
errno = saved_errno;
@@ -7906,9 +7918,10 @@ os_fork_impl(PyObject *module)
79067918
/* child: this clobbers and resets the import lock. */
79077919
PyOS_AfterFork_Child();
79087920
} else {
7909-
warn_about_fork_with_threads("fork");
79107921
/* parent: release the import lock. */
79117922
PyOS_AfterFork_Parent();
7923+
// After PyOS_AfterFork_Parent() starts the world to avoid deadlock.
7924+
warn_about_fork_with_threads("fork");
79127925
}
79137926
if (pid == -1) {
79147927
errno = saved_errno;
@@ -8737,9 +8750,10 @@ os_forkpty_impl(PyObject *module)
87378750
/* child: this clobbers and resets the import lock. */
87388751
PyOS_AfterFork_Child();
87398752
} else {
8740-
warn_about_fork_with_threads("forkpty");
87418753
/* parent: release the import lock. */
87428754
PyOS_AfterFork_Parent();
8755+
// After PyOS_AfterFork_Parent() starts the world to avoid deadlock.
8756+
warn_about_fork_with_threads("forkpty");
87438757
}
87448758
if (pid == -1) {
87458759
return posix_error();

‎Python/pylifecycle.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1911,6 +1911,9 @@ Py_FinalizeEx(void)
19111911
int malloc_stats = tstate->interp->config.malloc_stats;
19121912
#endif
19131913

1914+
/* Ensure that remaining threads are detached */
1915+
_PyEval_StopTheWorldAll(runtime);
1916+
19141917
/* Remaining daemon threads will automatically exit
19151918
when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
19161919
_PyInterpreterState_SetFinalizing(tstate->interp, tstate);

‎Python/pystate.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1692,6 +1692,10 @@ _PyThreadState_DeleteExcept(PyThreadState *tstate)
16921692
PyInterpreterState *interp = tstate->interp;
16931693
_PyRuntimeState *runtime = interp->runtime;
16941694

1695+
#ifdef Py_GIL_DISABLED
1696+
assert(runtime->stoptheworld.world_stopped);
1697+
#endif
1698+
16951699
HEAD_LOCK(runtime);
16961700
/* Remove all thread states, except tstate, from the linked list of
16971701
thread states. This will allow calling PyThreadState_Clear()
@@ -1710,6 +1714,8 @@ _PyThreadState_DeleteExcept(PyThreadState *tstate)
17101714
interp->threads.head = tstate;
17111715
HEAD_UNLOCK(runtime);
17121716

1717+
_PyEval_StartTheWorldAll(runtime);
1718+
17131719
/* Clear and deallocate all stale thread states. Even if this
17141720
executes Python code, we should be safe since it executes
17151721
in the current thread, not one of the stale threads. */

0 commit comments

Comments
 (0)
0