From 74ae6dec333924bbab8dbfa86dafbd77474bafa9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 20 Nov 2024 14:28:52 -0700 Subject: [PATCH 1/5] Add PyInterpreterState.threads.mutex. --- Include/internal/pycore_interp.h | 1 + Include/internal/pycore_pystate.h | 10 +++++++--- Python/pystate.c | 22 +++++++++++----------- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 5e4bcbf835a4d0..4c893baf3a0292 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -127,6 +127,7 @@ struct _is { uintptr_t last_restart_version; struct pythreads { + PyMutex mutex; uint64_t next_unique_id; /* The linked list of threads, newest first. */ PyThreadState *head; diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index 54d8803bc0bdb6..76b978b1d42dce 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -269,14 +269,18 @@ extern int _PyOS_InterruptOccurred(PyThreadState *tstate); #define HEAD_UNLOCK(runtime) \ PyMutex_Unlock(&(runtime)->interpreters.mutex) +#define THREADS_HEAD_LOCK(interp) \ + PyMutex_LockFlags(&(interp)->threads.mutex, _Py_LOCK_DONT_DETACH) +#define THREADS_HEAD_UNLOCK(interp) \ + PyMutex_Unlock(&(interp)->threads.mutex) + #define _Py_FOR_EACH_TSTATE_UNLOCKED(interp, t) \ for (PyThreadState *t = interp->threads.head; t; t = t->next) #define _Py_FOR_EACH_TSTATE_BEGIN(interp, t) \ - HEAD_LOCK(interp->runtime); \ + THREADS_HEAD_LOCK(interp); \ _Py_FOR_EACH_TSTATE_UNLOCKED(interp, t) #define _Py_FOR_EACH_TSTATE_END(interp) \ - HEAD_UNLOCK(interp->runtime) - + THREADS_HEAD_UNLOCK(interp) // Get the configuration of the current interpreter. // The caller must hold the GIL. diff --git a/Python/pystate.c b/Python/pystate.c index 975eb6d4fbd0f2..543a91ff2f459f 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -773,7 +773,6 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) { assert(interp != NULL); assert(tstate != NULL); - _PyRuntimeState *runtime = interp->runtime; /* XXX Conditions we need to enforce: @@ -794,9 +793,9 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) // See https://github.com/python/cpython/issues/102126 // Must be called without HEAD_LOCK held as it can deadlock // if any finalizer tries to acquire that lock. - HEAD_UNLOCK(runtime); + THREADS_HEAD_UNLOCK(interp); PyThreadState_Clear(p); - HEAD_LOCK(runtime); + THREADS_HEAD_LOCK(interp); } _Py_FOR_EACH_TSTATE_END(interp); if (tstate->interp == interp) { @@ -1536,7 +1535,7 @@ new_threadstate(PyInterpreterState *interp, int whence) #endif /* We serialize concurrent creation to protect global state. */ - HEAD_LOCK(interp->runtime); + THREADS_HEAD_LOCK(interp); // Initialize the new thread state. interp->threads.next_unique_id += 1; @@ -1547,7 +1546,7 @@ new_threadstate(PyInterpreterState *interp, int whence) PyThreadState *old_head = interp->threads.head; add_threadstate(interp, (PyThreadState *)tstate, old_head); - HEAD_UNLOCK(interp->runtime); + THREADS_HEAD_UNLOCK(interp); #ifdef Py_GIL_DISABLED // Must be called with lock unlocked to avoid lock ordering deadlocks. @@ -1743,7 +1742,7 @@ tstate_delete_common(PyThreadState *tstate, int release_gil) } _PyRuntimeState *runtime = interp->runtime; - HEAD_LOCK(runtime); + THREADS_HEAD_LOCK(interp); if (tstate->prev) { tstate->prev->next = tstate->next; } @@ -1759,9 +1758,11 @@ tstate_delete_common(PyThreadState *tstate, int release_gil) if (interp->stoptheworld.requested) { decrement_stoptheworld_countdown(&interp->stoptheworld); } + HEAD_LOCK(runtime); if (runtime->stoptheworld.requested) { decrement_stoptheworld_countdown(&runtime->stoptheworld); } + HEAD_UNLOCK(runtime); } #if defined(Py_REF_DEBUG) && defined(Py_GIL_DISABLED) @@ -1772,7 +1773,7 @@ tstate_delete_common(PyThreadState *tstate, int release_gil) assert(tstate_impl->refcounts.values == NULL); #endif - HEAD_UNLOCK(runtime); + THREADS_HEAD_UNLOCK(interp); // XXX Unbind in PyThreadState_Clear(), or earlier // (and assert not-equal here)? @@ -1852,13 +1853,12 @@ _PyThreadState_RemoveExcept(PyThreadState *tstate) { assert(tstate != NULL); PyInterpreterState *interp = tstate->interp; - _PyRuntimeState *runtime = interp->runtime; #ifdef Py_GIL_DISABLED - assert(runtime->stoptheworld.world_stopped); + assert(interp->runtime->stoptheworld.world_stopped); #endif - HEAD_LOCK(runtime); + THREADS_HEAD_LOCK(interp); /* Remove all thread states, except tstate, from the linked list of thread states. */ PyThreadState *list = interp->threads.head; @@ -1873,7 +1873,7 @@ _PyThreadState_RemoveExcept(PyThreadState *tstate) } tstate->prev = tstate->next = NULL; interp->threads.head = tstate; - HEAD_UNLOCK(runtime); + THREADS_HEAD_UNLOCK(interp); return list; } From af6c962809dbec2a4733b15eb4361e4e431f7246 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 20 Nov 2024 14:45:06 -0700 Subject: [PATCH 2/5] Switch to _Py_FOR_EACH_TSTATE_BEGIN() where possible. --- Objects/object.c | 9 +++------ Objects/obmalloc.c | 3 ++- Python/gc_free_threading.c | 13 +++++++++--- Python/pystate.c | 41 ++++++++++++++++++++++++++++---------- 4 files changed, 45 insertions(+), 21 deletions(-) diff --git a/Objects/object.c b/Objects/object.c index 8868fa29066404..76855bf306b9bf 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -119,11 +119,11 @@ get_reftotal(PyInterpreterState *interp) since we can't determine which interpreter updated it. */ Py_ssize_t total = REFTOTAL(interp); #ifdef Py_GIL_DISABLED - _Py_FOR_EACH_TSTATE_UNLOCKED(interp, p) { - /* This may race with other threads modifications to their reftotal */ + _Py_FOR_EACH_TSTATE_BEGIN(interp, p) { _PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)p; total += _Py_atomic_load_ssize_relaxed(&tstate_impl->reftotal); } + _Py_FOR_EACH_TSTATE_END(interp); #endif return total; } @@ -317,10 +317,7 @@ _Py_GetLegacyRefTotal(void) Py_ssize_t _PyInterpreterState_GetRefTotal(PyInterpreterState *interp) { - HEAD_LOCK(&_PyRuntime); - Py_ssize_t total = get_reftotal(interp); - HEAD_UNLOCK(&_PyRuntime); - return total; + return get_reftotal(interp); } #endif /* Py_REF_DEBUG */ diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 2cc0377f68f990..f13518cdb81608 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1439,13 +1439,14 @@ get_mimalloc_allocated_blocks(PyInterpreterState *interp) { size_t allocated_blocks = 0; #ifdef Py_GIL_DISABLED - _Py_FOR_EACH_TSTATE_UNLOCKED(interp, t) { + _Py_FOR_EACH_TSTATE_BEGIN(interp, t) { _PyThreadStateImpl *tstate = (_PyThreadStateImpl *)t; for (int i = 0; i < _Py_MIMALLOC_HEAP_COUNT; i++) { mi_heap_t *heap = &tstate->mimalloc.heaps[i]; mi_heap_visit_blocks(heap, false, &count_blocks, &allocated_blocks); } } + _Py_FOR_EACH_TSTATE_END(interp); mi_abandoned_pool_t *pool = &interp->mimalloc.abandoned_pool; for (uint8_t tag = 0; tag < _Py_MIMALLOC_HEAP_COUNT; tag++) { diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index f7f44407494e51..f168ecc20ada28 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -304,7 +304,8 @@ gc_visit_heaps_lock_held(PyInterpreterState *interp, mi_block_visit_fun *visitor Py_ssize_t offset_pre = offset_base + 2 * sizeof(PyObject*); // visit each thread's heaps for GC objects - _Py_FOR_EACH_TSTATE_UNLOCKED(interp, p) { + int failed = 0; + _Py_FOR_EACH_TSTATE_BEGIN(interp, p) { struct _mimalloc_thread_state *m = &((_PyThreadStateImpl *)p)->mimalloc; if (!_Py_atomic_load_int(&m->initialized)) { // The thread may not have called tstate_mimalloc_bind() yet. @@ -314,14 +315,20 @@ gc_visit_heaps_lock_held(PyInterpreterState *interp, mi_block_visit_fun *visitor arg->offset = offset_base; if (!mi_heap_visit_blocks(&m->heaps[_Py_MIMALLOC_HEAP_GC], true, visitor, arg)) { - return -1; + failed = 1; + break; } arg->offset = offset_pre; if (!mi_heap_visit_blocks(&m->heaps[_Py_MIMALLOC_HEAP_GC_PRE], true, visitor, arg)) { - return -1; + failed = 1; + break; } } + _Py_FOR_EACH_TSTATE_END(interp); + if (failed) { + return -1; + } // visit blocks in the per-interpreter abandoned pool (from dead threads) mi_abandoned_pool_t *pool = &interp->mimalloc.abandoned_pool; diff --git a/Python/pystate.c b/Python/pystate.c index 543a91ff2f459f..b7ff32fe46d078 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1801,11 +1801,12 @@ zapthreads(PyInterpreterState *interp) { /* No need to lock the mutex here because this should only happen when the threads are all really dead (XXX famous last words). */ - _Py_FOR_EACH_TSTATE_UNLOCKED(interp, tstate) { + _Py_FOR_EACH_TSTATE_BEGIN(interp, tstate) { tstate_verify_not_active(tstate); tstate_delete_common(tstate, 0); free_threadstate((_PyThreadStateImpl *)tstate); } + _Py_FOR_EACH_TSTATE_END(interp); } @@ -2184,7 +2185,7 @@ park_detached_threads(struct _stoptheworld_state *stw) { int num_parked = 0; _Py_FOR_EACH_STW_INTERP(stw, i) { - _Py_FOR_EACH_TSTATE_UNLOCKED(i, t) { + _Py_FOR_EACH_TSTATE_BEGIN(i, t) { int state = _Py_atomic_load_int_relaxed(&t->state); if (state == _Py_THREAD_DETACHED) { // Atomically transition to "suspended" if in "detached" state. @@ -2197,6 +2198,7 @@ park_detached_threads(struct _stoptheworld_state *stw) _Py_set_eval_breaker_bit(t, _PY_EVAL_PLEASE_STOP_BIT); } } + _Py_FOR_EACH_TSTATE_END(i); } stw->thread_countdown -= num_parked; assert(stw->thread_countdown >= 0); @@ -2223,12 +2225,13 @@ stop_the_world(struct _stoptheworld_state *stw) stw->requester = _PyThreadState_GET(); // may be NULL _Py_FOR_EACH_STW_INTERP(stw, i) { - _Py_FOR_EACH_TSTATE_UNLOCKED(i, t) { + _Py_FOR_EACH_TSTATE_BEGIN(i, t) { if (t != stw->requester) { // Count all the other threads (we don't wait on ourself). stw->thread_countdown++; } } + _Py_FOR_EACH_TSTATE_END(i); } if (stw->thread_countdown == 0) { @@ -2269,7 +2272,7 @@ start_the_world(struct _stoptheworld_state *stw) stw->world_stopped = 0; // Switch threads back to the detached state. _Py_FOR_EACH_STW_INTERP(stw, i) { - _Py_FOR_EACH_TSTATE_UNLOCKED(i, t) { + _Py_FOR_EACH_TSTATE_BEGIN(i, t) { if (t != stw->requester) { assert(_Py_atomic_load_int_relaxed(&t->state) == _Py_THREAD_SUSPENDED); @@ -2277,6 +2280,7 @@ start_the_world(struct _stoptheworld_state *stw) _PyParkingLot_UnparkAll(&t->state); } } + _Py_FOR_EACH_TSTATE_END(i); } stw->requester = NULL; HEAD_UNLOCK(runtime); @@ -2511,7 +2515,8 @@ _PyThread_CurrentFrames(void) HEAD_LOCK(runtime); PyInterpreterState *i; for (i = runtime->interpreters.head; i != NULL; i = i->next) { - _Py_FOR_EACH_TSTATE_UNLOCKED(i, t) { + int failed = 0; + _Py_FOR_EACH_TSTATE_BEGIN(i, t) { _PyInterpreterFrame *frame = t->current_frame; frame = _PyFrame_GetFirstComplete(frame); if (frame == NULL) { @@ -2519,19 +2524,26 @@ _PyThread_CurrentFrames(void) } PyObject *id = PyLong_FromUnsignedLong(t->thread_id); if (id == NULL) { - goto fail; + failed = 1; + break; } PyObject *frameobj = (PyObject *)_PyFrame_GetFrameObject(frame); if (frameobj == NULL) { Py_DECREF(id); - goto fail; + failed = 1; + break; } int stat = PyDict_SetItem(result, id, frameobj); Py_DECREF(id); if (stat < 0) { - goto fail; + failed = 1; + break; } } + _Py_FOR_EACH_TSTATE_END(i); + if (failed) { + goto fail; + } } goto done; @@ -2576,14 +2588,16 @@ _PyThread_CurrentExceptions(void) HEAD_LOCK(runtime); PyInterpreterState *i; for (i = runtime->interpreters.head; i != NULL; i = i->next) { - _Py_FOR_EACH_TSTATE_UNLOCKED(i, t) { + int failed = 0; + _Py_FOR_EACH_TSTATE_BEGIN(i, t) { _PyErr_StackItem *err_info = _PyErr_GetTopmostException(t); if (err_info == NULL) { continue; } PyObject *id = PyLong_FromUnsignedLong(t->thread_id); if (id == NULL) { - goto fail; + failed = 1; + break; } PyObject *exc = err_info->exc_value; assert(exc == NULL || @@ -2593,9 +2607,14 @@ _PyThread_CurrentExceptions(void) int stat = PyDict_SetItem(result, id, exc == NULL ? Py_None : exc); Py_DECREF(id); if (stat < 0) { - goto fail; + failed = 1; + break; } } + _Py_FOR_EACH_TSTATE_END(i); + if (failed) { + goto fail; + } } goto done; From db6269e6285e0d9927dc9c33edee235fafc2a0d2 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 20 Nov 2024 15:33:12 -0700 Subject: [PATCH 3/5] Keep using the "head" lock where appropriate. --- Python/pystate.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index b7ff32fe46d078..60994e5af5984c 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -789,15 +789,19 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) } // Clear the current/main thread state last. + HEAD_LOCK(interp->runtime); _Py_FOR_EACH_TSTATE_BEGIN(interp, p) { // See https://github.com/python/cpython/issues/102126 // Must be called without HEAD_LOCK held as it can deadlock // if any finalizer tries to acquire that lock. THREADS_HEAD_UNLOCK(interp); + HEAD_UNLOCK(interp->runtime); PyThreadState_Clear(p); + HEAD_LOCK(interp->runtime); THREADS_HEAD_LOCK(interp); } _Py_FOR_EACH_TSTATE_END(interp); + HEAD_UNLOCK(interp->runtime); if (tstate->interp == interp) { /* We fix tstate->_status below when we for sure aren't using it (e.g. no longer need the GIL). */ @@ -1535,6 +1539,7 @@ new_threadstate(PyInterpreterState *interp, int whence) #endif /* We serialize concurrent creation to protect global state. */ + HEAD_LOCK(interp->runtime); THREADS_HEAD_LOCK(interp); // Initialize the new thread state. @@ -1547,6 +1552,7 @@ new_threadstate(PyInterpreterState *interp, int whence) add_threadstate(interp, (PyThreadState *)tstate, old_head); THREADS_HEAD_UNLOCK(interp); + HEAD_UNLOCK(interp->runtime); #ifdef Py_GIL_DISABLED // Must be called with lock unlocked to avoid lock ordering deadlocks. @@ -1742,7 +1748,13 @@ tstate_delete_common(PyThreadState *tstate, int release_gil) } _PyRuntimeState *runtime = interp->runtime; - THREADS_HEAD_LOCK(interp); + HEAD_LOCK(runtime); + + int locked = 1; + if (!PyMutex_IsLocked(&interp->threads.mutex)) { + locked = 0; + THREADS_HEAD_LOCK(interp); + } if (tstate->prev) { tstate->prev->next = tstate->next; } @@ -1752,17 +1764,19 @@ tstate_delete_common(PyThreadState *tstate, int release_gil) if (tstate->next) { tstate->next->prev = tstate->prev; } + if (!locked) { + THREADS_HEAD_UNLOCK(interp); + } + if (tstate->state != _Py_THREAD_SUSPENDED) { // Any ongoing stop-the-world request should not wait for us because // our thread is getting deleted. if (interp->stoptheworld.requested) { decrement_stoptheworld_countdown(&interp->stoptheworld); } - HEAD_LOCK(runtime); if (runtime->stoptheworld.requested) { decrement_stoptheworld_countdown(&runtime->stoptheworld); } - HEAD_UNLOCK(runtime); } #if defined(Py_REF_DEBUG) && defined(Py_GIL_DISABLED) @@ -1773,7 +1787,7 @@ tstate_delete_common(PyThreadState *tstate, int release_gil) assert(tstate_impl->refcounts.values == NULL); #endif - THREADS_HEAD_UNLOCK(interp); + HEAD_UNLOCK(runtime); // XXX Unbind in PyThreadState_Clear(), or earlier // (and assert not-equal here)? From d1c7c69238651a79e9c3d180a6d69b90f0779895 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 21 Nov 2024 11:32:10 -0700 Subject: [PATCH 4/5] Reinit the new lock upon fork. --- Python/pystate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Python/pystate.c b/Python/pystate.c index 60994e5af5984c..a6e175b0a38558 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -515,6 +515,7 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime) for (PyInterpreterState *interp = runtime->interpreters.head; interp != NULL; interp = interp->next) { + _PyMutex_at_fork_reinit(&interp->threads.mutex); for (int i = 0; i < NUM_WEAKREF_LIST_LOCKS; i++) { _PyMutex_at_fork_reinit(&interp->weakref_locks[i]); } From a99bfef92ab6ebd29ba476d62674d1e4dd9657a9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 21 Nov 2024 12:40:07 -0700 Subject: [PATCH 5/5] Use a recursive lock. --- Include/internal/pycore_interp.h | 2 +- Include/internal/pycore_lock.h | 35 ++++++++++++++++++++++++++++++- Include/internal/pycore_pystate.h | 4 ++-- Include/internal/pycore_runtime.h | 5 +++++ Modules/posixmodule.c | 3 +++ Python/pystate.c | 15 ++++++------- 6 files changed, 51 insertions(+), 13 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 4c893baf3a0292..adb4764d1bbac6 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -127,7 +127,7 @@ struct _is { uintptr_t last_restart_version; struct pythreads { - PyMutex mutex; + _PyRecursiveMutex mutex; uint64_t next_unique_id; /* The linked list of threads, newest first. */ PyThreadState *head; diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index 57cbce8f126aca..a14493c4965ad5 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -151,10 +151,12 @@ _PyOnceFlag_CallOnce(_PyOnceFlag *flag, _Py_once_fn_t *fn, void *arg) return _PyOnceFlag_CallOnceSlow(flag, fn, arg); } +#define _PyThread_ident_t unsigned long long + // A recursive mutex. The mutex should zero-initialized. typedef struct { PyMutex mutex; - unsigned long long thread; // i.e., PyThread_get_thread_ident_ex() + _PyThread_ident_t thread; // i.e., PyThread_get_thread_ident_ex() size_t level; } _PyRecursiveMutex; @@ -164,6 +166,37 @@ extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t t PyAPI_FUNC(void) _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m); extern int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m); +PyAPI_FUNC(_PyThread_ident_t) PyThread_get_thread_ident_ex(void); + +static inline void +_PyRecursiveMutex_LockFlags(_PyRecursiveMutex *m, _PyLockFlags flags) +{ + uint8_t expected = _Py_UNLOCKED; + if (_Py_atomic_compare_exchange_uint8(&m->mutex._bits, &expected, _Py_LOCKED)) { + m->thread = PyThread_get_thread_ident_ex(); + assert(m->level == 0); + } + else { + _PyRecursiveMutex_LockTimed(m, -1, flags); + } +} + +#ifdef HAVE_FORK +PyAPI_FUNC(_PyThread_ident_t) PyThread_get_thread_ident_ex(void); + +static inline void +_PyRecursiveMutex_at_fork_reinit(_PyRecursiveMutex *m, + _PyThread_ident_t parent) +{ + if (m->thread == parent) { + m->thread = PyThread_get_thread_ident_ex(); + } + else { + memset(m, 0, sizeof(*m)); + } +} +#endif + // 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/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index 76b978b1d42dce..d9d9ff780a312d 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -270,9 +270,9 @@ extern int _PyOS_InterruptOccurred(PyThreadState *tstate); PyMutex_Unlock(&(runtime)->interpreters.mutex) #define THREADS_HEAD_LOCK(interp) \ - PyMutex_LockFlags(&(interp)->threads.mutex, _Py_LOCK_DONT_DETACH) + _PyRecursiveMutex_LockFlags(&(interp)->threads.mutex, _Py_LOCK_DONT_DETACH) #define THREADS_HEAD_UNLOCK(interp) \ - PyMutex_Unlock(&(interp)->threads.mutex) + _PyRecursiveMutex_Unlock(&(interp)->threads.mutex) #define _Py_FOR_EACH_TSTATE_UNLOCKED(interp, t) \ for (PyThreadState *t = interp->threads.head; t; t = t->next) diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index 2f2cec22cf1589..da5b5b4b086be7 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -164,6 +164,11 @@ typedef struct pyruntimestate { _Py_AuditHookEntry *head; } audit_hooks; +#ifdef HAVE_FORK + // Guarded by multiple global locks. + PyThread_ident_t fork_tid; +#endif + struct _py_object_runtime_state object_state; struct _Py_float_runtime_state float_state; struct _Py_unicode_runtime_state unicode_state; diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 6eb7054b566e3f..40f9d2a7edc923 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -624,11 +624,13 @@ PyOS_BeforeFork(void) _PyImport_AcquireLock(interp); _PyEval_StopTheWorldAll(&_PyRuntime); HEAD_LOCK(&_PyRuntime); + _PyRuntime.fork_tid = PyThread_get_thread_ident_ex(); } void PyOS_AfterFork_Parent(void) { + _PyRuntime.fork_tid = 0; HEAD_UNLOCK(&_PyRuntime); _PyEval_StartTheWorldAll(&_PyRuntime); @@ -648,6 +650,7 @@ PyOS_AfterFork_Child(void) if (_PyStatus_EXCEPTION(status)) { goto fatal_error; } + _PyRuntime.fork_tid = 0; PyThreadState *tstate = _PyThreadState_GET(); _Py_EnsureTstateNotNULL(tstate); diff --git a/Python/pystate.c b/Python/pystate.c index a6e175b0a38558..1df0c562de3ccc 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -499,6 +499,9 @@ _PyRuntimeState_Fini(_PyRuntimeState *runtime) PyStatus _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime) { + PyThread_ident_t parent = _PyRuntime.fork_tid; + assert(parent != 0); + // This was initially set in _PyRuntimeState_Init(). runtime->main_thread = PyThread_get_thread_ident(); @@ -515,7 +518,7 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime) for (PyInterpreterState *interp = runtime->interpreters.head; interp != NULL; interp = interp->next) { - _PyMutex_at_fork_reinit(&interp->threads.mutex); + _PyRecursiveMutex_at_fork_reinit(&interp->threads.mutex, parent); for (int i = 0; i < NUM_WEAKREF_LIST_LOCKS; i++) { _PyMutex_at_fork_reinit(&interp->weakref_locks[i]); } @@ -1751,11 +1754,7 @@ tstate_delete_common(PyThreadState *tstate, int release_gil) HEAD_LOCK(runtime); - int locked = 1; - if (!PyMutex_IsLocked(&interp->threads.mutex)) { - locked = 0; - THREADS_HEAD_LOCK(interp); - } + THREADS_HEAD_LOCK(interp); if (tstate->prev) { tstate->prev->next = tstate->next; } @@ -1765,9 +1764,7 @@ tstate_delete_common(PyThreadState *tstate, int release_gil) if (tstate->next) { tstate->next->prev = tstate->prev; } - if (!locked) { - THREADS_HEAD_UNLOCK(interp); - } + THREADS_HEAD_UNLOCK(interp); if (tstate->state != _Py_THREAD_SUSPENDED) { // Any ongoing stop-the-world request should not wait for us because