Repository navigation
Add a Per-Interpreter "HEAD" Lock #114940
Description
Activity
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()andHEAD_UNLOCK()from pycore_pystate.h. Nothing has been done for this issue yet, has it?.Reacted by RUANG (James Roy)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 amutexmember to threads ofPyInterpreterStateand add corresponding macros for locking/unlocking. Finally, I'll try to replace the part that usesHEAD_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 theGILneed 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 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.
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()andHEAD_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
PyInterpreterState.threads#125561