Skip to content

gh-116522: Stop the world before fork() and during shutdown #116607

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

Merged
merged 3 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 21 additions & 7 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -613,11 +613,16 @@ PyOS_BeforeFork(void)
run_at_forkers(interp->before_forkers, 1);

_PyImport_AcquireLock(interp);
_PyEval_StopTheWorldAll(&_PyRuntime);
HEAD_LOCK(&_PyRuntime);
}

void
PyOS_AfterFork_Parent(void)
{
HEAD_UNLOCK(&_PyRuntime);
_PyEval_StartTheWorldAll(&_PyRuntime);

PyInterpreterState *interp = _PyInterpreterState_GET();
if (_PyImport_ReleaseLock(interp) <= 0) {
Py_FatalError("failed releasing import lock after fork");
Expand All @@ -632,6 +637,7 @@ PyOS_AfterFork_Child(void)
PyStatus status;
_PyRuntimeState *runtime = &_PyRuntime;

// re-creates runtime->interpreters.mutex (HEAD_UNLOCK)
status = _PyRuntimeState_ReInitThreads(runtime);
if (_PyStatus_EXCEPTION(status)) {
goto fatal_error;
Expand Down Expand Up @@ -7731,10 +7737,15 @@ os_register_at_fork_impl(PyObject *module, PyObject *before,
// running in the process. Best effort, silent if unable to count threads.
// Constraint: Quick. Never overcounts. Never leaves an error set.
//
// This code might do an import, thus acquiring the import lock, which
// PyOS_BeforeFork() also does. As this should only be called from
// the parent process, it is in the same thread so that works.
static void warn_about_fork_with_threads(const char* name) {
// This should only be called from the parent process after
// PyOS_AfterFork_Parent().
static void
warn_about_fork_with_threads(const char* name)
{
// It's not safe to issue the warning while the world is stopped, because
// other threads might be holding locks that we need, which would deadlock.
assert(!_PyRuntime.stoptheworld.world_stopped);

// TODO: Consider making an `os` module API to return the current number
// of threads in the process. That'd presumably use this platform code but
// raise an error rather than using the inaccurate fallback.
Expand Down Expand Up @@ -7858,9 +7869,10 @@ os_fork1_impl(PyObject *module)
/* child: this clobbers and resets the import lock. */
PyOS_AfterFork_Child();
} else {
warn_about_fork_with_threads("fork1");
/* parent: release the import lock. */
PyOS_AfterFork_Parent();
// After PyOS_AfterFork_Parent() starts the world to avoid deadlock.
warn_about_fork_with_threads("fork1");
}
if (pid == -1) {
errno = saved_errno;
Expand Down Expand Up @@ -7906,9 +7918,10 @@ os_fork_impl(PyObject *module)
/* child: this clobbers and resets the import lock. */
PyOS_AfterFork_Child();
} else {
warn_about_fork_with_threads("fork");
/* parent: release the import lock. */
PyOS_AfterFork_Parent();
// After PyOS_AfterFork_Parent() starts the world to avoid deadlock.
warn_about_fork_with_threads("fork");
}
if (pid == -1) {
errno = saved_errno;
Expand Down Expand Up @@ -8737,9 +8750,10 @@ os_forkpty_impl(PyObject *module)
/* child: this clobbers and resets the import lock. */
PyOS_AfterFork_Child();
} else {
warn_about_fork_with_threads("forkpty");
/* parent: release the import lock. */
PyOS_AfterFork_Parent();
// After PyOS_AfterFork_Parent() starts the world to avoid deadlock.
warn_about_fork_with_threads("forkpty");
}
if (pid == -1) {
return posix_error();
Expand Down
3 changes: 3 additions & 0 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -1911,6 +1911,9 @@ Py_FinalizeEx(void)
int malloc_stats = tstate->interp->config.malloc_stats;
#endif

/* Ensure that remaining threads are detached */
_PyEval_StopTheWorldAll(runtime);

/* Remaining daemon threads will automatically exit
when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
_PyInterpreterState_SetFinalizing(tstate->interp, tstate);
Expand Down
6 changes: 6 additions & 0 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -1715,6 +1715,10 @@ _PyThreadState_DeleteExcept(PyThreadState *tstate)
PyInterpreterState *interp = tstate->interp;
_PyRuntimeState *runtime = interp->runtime;

#ifdef Py_GIL_DISABLED
assert(runtime->stoptheworld.world_stopped);
#endif

HEAD_LOCK(runtime);
/* Remove all thread states, except tstate, from the linked list of
thread states. This will allow calling PyThreadState_Clear()
Expand All @@ -1733,6 +1737,8 @@ _PyThreadState_DeleteExcept(PyThreadState *tstate)
interp->threads.head = tstate;
HEAD_UNLOCK(runtime);

_PyEval_StartTheWorldAll(runtime);

/* Clear and deallocate all stale thread states. Even if this
executes Python code, we should be safe since it executes
in the current thread, not one of the stale threads. */
Expand Down