Add a Per-Interpreter "HEAD" Lock

[ericsnowcurrently]

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

• gh-114940: Use fine-grained mutex protection for PyInterpreterState.threads #125561
• gh-114940: Add a Per-Interpreter Lock For the List of Thread States #127037
• gh-114940: Add _Py_FOR_EACH_TSTATE_UNLOCKED(), and Friends #127077
• gh-114940: Add a Per-Interpreter Lock For the List of Thread States #127115

[rruuaanng]

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

[ericsnowcurrently]

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?.

[rruuaanng]

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).

[ericsnowcurrently]

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

[rruuaanng]

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

[rruuaanng]

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;
 }

[kumaraditya303]

@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.

[rruuaanng]

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

[ericsnowcurrently]

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.

[ericsnowcurrently]

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.