From 5e86cca8313575ab1cde8b01abee7f20d25df568 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 11 Nov 2024 16:57:04 +0000 Subject: [PATCH 1/2] gh-126688: Reinit import lock after fork The PyMutex implementation supports unlocking after fork because we clear the list of watiers in parking_lot.c. This doesn't work as well for _PyRecursiveMutex because on some systems, such as SerenityOS, the thread id is not preserved across fork(). --- Include/internal/pycore_lock.h | 7 +++++++ .../2024-11-11-17-02-48.gh-issue-126688.QiOXUi.rst | 2 ++ Modules/posixmodule.c | 3 ++- 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-11-11-17-02-48.gh-issue-126688.QiOXUi.rst diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index 57cbce8f126aca..3f380c26cb6748 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -164,6 +164,13 @@ extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t t PyAPI_FUNC(void) _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m); extern int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m); +static inline void +_PyRecursiveMutex_at_fork_reinit(_PyRecursiveMutex *m) +{ + memset(m, 0, sizeof(*m)); +} + + // A readers-writer (RW) lock. The lock supports multiple concurrent readers or // a single writer. The lock is write-preferring: if a writer is waiting while // the lock is read-locked then, new readers will be blocked. This avoids diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-11-17-02-48.gh-issue-126688.QiOXUi.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-11-17-02-48.gh-issue-126688.QiOXUi.rst new file mode 100644 index 00000000000000..30aa5722f0ea02 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-11-17-02-48.gh-issue-126688.QiOXUi.rst @@ -0,0 +1,2 @@ +Fix a crash when calling :func:`os.fork` on some operating systems, +including SerenityOS. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 1ce2baecb8a964..a7e1c4b20b5482 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -678,7 +678,8 @@ PyOS_AfterFork_Child(void) _PyEval_StartTheWorldAll(&_PyRuntime); _PyThreadState_DeleteList(list); - _PyImport_ReleaseLock(tstate->interp); + // gh-126688: Reinit lock because thread id may differ in child process. + _PyRecursiveMutex_at_fork_reinit(&tstate->interp->imports.lock); _PySignal_AfterFork(); From 27d5db7f521ef5e1b083bec77f648463f44f4c4e Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 11 Nov 2024 19:33:37 +0000 Subject: [PATCH 2/2] Only re-initialize thread id --- Include/internal/pycore_import.h | 1 + Include/internal/pycore_lock.h | 7 ------- Modules/posixmodule.c | 4 ++-- Python/import.c | 7 +++++++ 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Include/internal/pycore_import.h b/Include/internal/pycore_import.h index 290ba95e1a0ad7..318c712bdfa174 100644 --- a/Include/internal/pycore_import.h +++ b/Include/internal/pycore_import.h @@ -21,6 +21,7 @@ extern int _PyImport_SetModuleString(const char *name, PyObject* module); extern void _PyImport_AcquireLock(PyInterpreterState *interp); extern void _PyImport_ReleaseLock(PyInterpreterState *interp); +extern void _PyImport_ReInitLock(PyInterpreterState *interp); // This is used exclusively for the sys and builtins modules: extern int _PyImport_FixupBuiltin( diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index 3f380c26cb6748..57cbce8f126aca 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -164,13 +164,6 @@ extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t t PyAPI_FUNC(void) _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m); extern int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m); -static inline void -_PyRecursiveMutex_at_fork_reinit(_PyRecursiveMutex *m) -{ - memset(m, 0, sizeof(*m)); -} - - // A readers-writer (RW) lock. The lock supports multiple concurrent readers or // a single writer. The lock is write-preferring: if a writer is waiting while // the lock is read-locked then, new readers will be blocked. This avoids diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index a7e1c4b20b5482..da7399de86f213 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -678,8 +678,8 @@ PyOS_AfterFork_Child(void) _PyEval_StartTheWorldAll(&_PyRuntime); _PyThreadState_DeleteList(list); - // gh-126688: Reinit lock because thread id may differ in child process. - _PyRecursiveMutex_at_fork_reinit(&tstate->interp->imports.lock); + _PyImport_ReInitLock(tstate->interp); + _PyImport_ReleaseLock(tstate->interp); _PySignal_AfterFork(); diff --git a/Python/import.c b/Python/import.c index 29bd8bf68ff5e1..09fe95fa1fb647 100644 --- a/Python/import.c +++ b/Python/import.c @@ -122,6 +122,13 @@ _PyImport_ReleaseLock(PyInterpreterState *interp) _PyRecursiveMutex_Unlock(&IMPORT_LOCK(interp)); } +void +_PyImport_ReInitLock(PyInterpreterState *interp) +{ + // gh-126688: Thread id may change after fork() on some operating systems. + IMPORT_LOCK(interp).thread = PyThread_get_thread_ident_ex(); +} + /***************/ /* sys.modules */