Add a Per-Interpreter "HEAD" Lock

Feature or enhancement

We have a lock that we use for thread-safety when working with the set of interpreters: _PyRuntimeState.interpreters.mutex. We use two macros in several core files to manage that lock: HEAD_LOCK() and HEAD_UNLOCK() (from pycore_pystate.h).

Recently we've begun using that runtime-global lock for thread-safety when working with a single interpreter's set of thread states (PyInterpreterState.threads). The problem is that this means we are potentially blocking interpreters unnecessarily.

Thus it makes sense to introduce an interpreter-specific "head" lock: PyInterpreterState.threads.mutex (plus corresponding macros).

Linked PRs

Can this issue be closed? It seems to have been merged.

What change do you mean? AFAIK, we still only have the global HEAD_LOCK() and HEAD_UNLOCK() from pycore_pystate.h. Nothing has been done for this issue yet, has it?.

Oh, I'm sorry, I didn't seem to check the content of PR carefully. Please forgive my negligence.

Edit:
If allowed, I would like to take over this task. I‘ll add a mutex member to threads of PyInterpreterState and add corresponding macros for locking/unlocking. Finally, I'll try to replace the part that uses HEAD_LOCK(_PyRuntime).
(Or maybe you just need to add mutex member and macros).

Any help would be very welcome! 😄 I'd be glad to review your PR.

Maybe you can wait for my good news. I'll launch it today ：）.

I'm not sure if this‘s what you want, please review!
rruuaanng@414cfef
If the sections using the GIL need to be replaced with statements for this change, I'd be happy to go ahead and make the change.

Edit:
for example

--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -2371,7 +2371,6 @@ _PyEval_StartTheWorld(PyInterpreterState *interp)
 int
 PyThreadState_SetAsyncExc(unsigned long id, PyObject *exc)
 {
-    _PyRuntimeState *runtime = &_PyRuntime;
     PyInterpreterState *interp = _PyInterpreterState_GET();
 
     /* Although the GIL is held, a few C API functions can be called
@@ -2380,7 +2379,7 @@ PyThreadState_SetAsyncExc(unsigned long id, PyObject *exc)
      * list of thread states we're traversing, so to prevent that we lock
      * head_mutex for the duration.
      */
-    HEAD_LOCK(runtime);
+    INTERP_THREAD_LOCK(interp);
     for (PyThreadState *tstate = interp->threads.head; tstate != NULL; tstate = tstate->next) {
         if (tstate->thread_id != id) {
             continue;
@@ -2401,7 +2400,7 @@ PyThreadState_SetAsyncExc(unsigned long id, PyObject *exc)
         _Py_set_eval_breaker_bit(tstate, _PY_ASYNC_EXCEPTION_BIT);
         return 1;
     }
-    HEAD_UNLOCK(runtime);
+    INTERP_THREAD_UNLOCK(interp);
     return 0;
 }

@ericsnowcurrently

@ericsnowcurrently I wonder whether it's time to change this lock to recursive lock, we have had issues about deadlocks where re-entrant calls can deadlock, ex Py_DECREF.

If we consider recursive locks, we can try to add a count field to threads and maintain it through atomic operations.

Yeah, a recursive lock could simplify a few things and/or provide better thread-safety. We do have a number of places where we cut some corners in that regard. That said, there still may be a risk of deadlock, e.g. finalizers that create threads, which we'd have to resolve.

FWIW, having looked at the PR, I think we should take a little time to make sure we have a clear picture of what should change to the per-interpreter lock. There are also several cases that have possible races that we need to sort out.

gh-114940: Add _Py_FOR_EACH_TSTATE_UNLOCKED(), and Friends (gh-127077)

pythongh-114940: Add _Py_FOR_EACH_TSTATE_UNLOCKED(), and Friends (pyt…

Navigation Menu

Repository navigation

Add a Per-Interpreter "HEAD" Lock #114940

Feature or enhancement

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Add a Per-Interpreter "HEAD" Lock #114940

Description

Feature or enhancement

Linked PRs

Activity

rruuaanng commented on Oct 14, 2024

ericsnowcurrently commented on Oct 15, 2024

rruuaanng commented on Oct 15, 2024

ericsnowcurrently commented on Oct 15, 2024

rruuaanng commented on Oct 15, 2024

rruuaanng commented on Oct 16, 2024

kumaraditya303 commented on Oct 16, 2024

rruuaanng commented on Oct 17, 2024

ericsnowcurrently commented on Oct 18, 2024

ericsnowcurrently commented on Oct 18, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions