gh-133171: Make the executor_list thread-safe

Include/internal/pycore_interp_structs.h

@@ -9,6 +9,7 @@ extern "C" {
 
 #include "pycore_ast_state.h" // struct ast_state
 #include "pycore_llist.h" // struct llist_node
+#include "pycore_lock.h" // _PyRecursiveMutex
 #include "pycore_opcode_utils.h" // NUM_COMMON_CONSTANTS
 #include "pycore_pymath.h" // _PY_SHORT_FLOAT_REPR
 #include "pycore_structs.h" // PyHamtObject
@@ -29,6 +30,17 @@ extern "C" {
 # define NUM_WEAKREF_LIST_LOCKS 127
 #endif
 
+// Executor list lock macros for thread-safe access to executor linked lists
+#ifdef Py_GIL_DISABLED
+#define EXECUTOR_LIST_LOCK(interp) \
+ _PyRecursiveMutex_Lock(&(interp)->executor_list_lock)
+#define EXECUTOR_LIST_UNLOCK(interp) \
+ _PyRecursiveMutex_Unlock(&(interp)->executor_list_lock)
+#else
+#define EXECUTOR_LIST_LOCK(interp)
+#define EXECUTOR_LIST_UNLOCK(interp)
+#endif
+
 typedef int (*_Py_pending_call_func)(void *);
 
 struct _pending_call {
@@ -939,6 +951,9 @@ struct _is {
  struct _PyExecutorObject *executor_deletion_list_head;
  struct _PyExecutorObject *cold_executor;
  int executor_deletion_list_remaining_capacity;
+#ifdef Py_GIL_DISABLED
+ _PyRecursiveMutex executor_list_lock;
+#endif
  size_t executor_creation_counter;
  _rare_events rare_events;
  PyDict_WatchCallback builtins_dict_watcher;

Python/optimizer.c

@@ -245,6 +245,16 @@ _Py_ClearExecutorDeletionList(PyInterpreterState *interp)
  ts = PyThreadState_Next(ts);
  HEAD_UNLOCK(runtime);
  }
+
+ /* Create a list to collect executors that need to be freed.
+ * This avoids calling _PyExecutor_Free while holding the lock,
+ * which could trigger destructors and cause deadlock. */
+ PyObject *to_free = PyList_New(0);
+ if (to_free == NULL) {
+ goto error;
+ }
+
+ EXECUTOR_LIST_LOCK(interp);
  _PyExecutorObject **prev_to_next_ptr = &interp->executor_deletion_list_head;
  _PyExecutorObject *exec = *prev_to_next_ptr;
  while (exec != NULL) {
@@ -255,24 +265,48 @@ _Py_ClearExecutorDeletionList(PyInterpreterState *interp)
  }
  else {
  *prev_to_next_ptr = exec->vm_data.links.next;
- _PyExecutor_Free(exec);
+ if (PyList_Append(to_free, (PyObject *)exec)) {
+ EXECUTOR_LIST_UNLOCK(interp);
+ goto error;
+ }
  }
  exec = *prev_to_next_ptr;
  }
  interp->executor_deletion_list_remaining_capacity = EXECUTOR_DELETE_LIST_MAX;
+ EXECUTOR_LIST_UNLOCK(interp);
+
+ for (Py_ssize_t i = 0; i < PyList_GET_SIZE(to_free); i++) {
+ _PyExecutorObject *exec = (_PyExecutorObject *)PyList_GET_ITEM(to_free, i);
+ PyList_SET_ITEM(to_free, i, NULL);
+ _PyExecutor_Free(exec);
+ }
+ Py_DECREF(to_free);
+ return;
+
+error:
+ PyErr_Clear();
+ Py_XDECREF(to_free);
 }
 
 static void
 add_to_pending_deletion_list(_PyExecutorObject *self)
 {
  PyInterpreterState *interp = PyInterpreterState_Get();
+ EXECUTOR_LIST_LOCK(interp);
  self->vm_data.links.next = interp->executor_deletion_list_head;
  interp->executor_deletion_list_head = self;
  if (interp->executor_deletion_list_remaining_capacity > 0) {
  interp->executor_deletion_list_remaining_capacity--;
+ EXECUTOR_LIST_UNLOCK(interp);
  }
  else {
+ /* Release the lock before calling _Py_ClearExecutorDeletionList
+ * to avoid deadlock, since it also tries to acquire the same lock */
+ EXECUTOR_LIST_UNLOCK(interp);
  _Py_ClearExecutorDeletionList(interp);
+ EXECUTOR_LIST_LOCK(interp);
+ interp->executor_deletion_list_head = self;
+ EXECUTOR_LIST_UNLOCK(interp);
  }
 }
 
@@ -1450,6 +1484,7 @@ static void
 link_executor(_PyExecutorObject *executor)
 {
  PyInterpreterState *interp = _PyInterpreterState_GET();
+ EXECUTOR_LIST_LOCK(interp);
  _PyExecutorLinkListNode *links = &executor->vm_data.links;
  _PyExecutorObject *head = interp->executor_list_head;
  if (head == NULL) {
@@ -1467,6 +1502,7 @@ link_executor(_PyExecutorObject *executor)
  executor->vm_data.linked = true;
  /* executor_list_head must be first in list */
  assert(interp->executor_list_head->vm_data.links.previous == NULL);
+ EXECUTOR_LIST_UNLOCK(interp);
 }
 
 static void
@@ -1475,6 +1511,8 @@ unlink_executor(_PyExecutorObject *executor)
  if (!executor->vm_data.linked) {
  return;
  }
+ PyInterpreterState *interp = PyInterpreterState_Get();
+ EXECUTOR_LIST_LOCK(interp);
  _PyExecutorLinkListNode *links = &executor->vm_data.links;
  assert(executor->vm_data.valid);
  _PyExecutorObject *next = links->next;
@@ -1487,11 +1525,11 @@ unlink_executor(_PyExecutorObject *executor)
  }
  else {
  // prev == NULL implies that executor is the list head
- PyInterpreterState *interp = PyInterpreterState_Get();
  assert(interp->executor_list_head == executor);
  interp->executor_list_head = next;
  }
  executor->vm_data.linked = false;
+ EXECUTOR_LIST_UNLOCK(interp);
 }
 
 /* This must be called by optimizers before using the executor */
@@ -1616,16 +1654,19 @@ _Py_Executors_InvalidateDependency(PyInterpreterState *interp, void *obj, int is
  }
  /* Clearing an executor can deallocate others, so we need to make a list of
  * executors to invalidate first */
+ EXECUTOR_LIST_LOCK(interp);
  for (_PyExecutorObject *exec = interp->executor_list_head; exec != NULL;) {
  assert(exec->vm_data.valid);
  _PyExecutorObject *next = exec->vm_data.links.next;
  if (bloom_filter_may_contain(&exec->vm_data.bloom, &obj_filter) &&
  PyList_Append(invalidate, (PyObject *)exec))
  {
+ EXECUTOR_LIST_UNLOCK(interp);
  goto error;
  }
  exec = next;
  }
+ EXECUTOR_LIST_UNLOCK(interp);
  for (Py_ssize_t i = 0; i < PyList_GET_SIZE(invalidate); i++) {
  PyObject *exec = PyList_GET_ITEM(invalidate, i);
  executor_clear(exec);
@@ -1646,6 +1687,7 @@ _Py_Executors_InvalidateDependency(PyInterpreterState *interp, void *obj, int is
 void
 _Py_Executors_InvalidateAll(PyInterpreterState *interp, int is_invalidation)
 {
+ EXECUTOR_LIST_LOCK(interp);
  while (interp->executor_list_head) {
  _PyExecutorObject *executor = interp->executor_list_head;
  assert(executor->vm_data.valid == 1 && executor->vm_data.linked == 1);
@@ -1660,6 +1702,7 @@ _Py_Executors_InvalidateAll(PyInterpreterState *interp, int is_invalidation)
  OPT_STAT_INC(executors_invalidated);
  }
  }
+ EXECUTOR_LIST_UNLOCK(interp);
 }
 
 void
@@ -1674,11 +1717,13 @@ _Py_Executors_InvalidateCold(PyInterpreterState *interp)
 
  /* Clearing an executor can deallocate others, so we need to make a list of
  * executors to invalidate first */
+ EXECUTOR_LIST_LOCK(interp);
  for (_PyExecutorObject *exec = interp->executor_list_head; exec != NULL;) {
  assert(exec->vm_data.valid);
  _PyExecutorObject *next = exec->vm_data.links.next;
 
  if (!exec->vm_data.warm && PyList_Append(invalidate, (PyObject *)exec) < 0) {
+ EXECUTOR_LIST_UNLOCK(interp);
  goto error;
  }
  else {
@@ -1687,6 +1732,7 @@ _Py_Executors_InvalidateCold(PyInterpreterState *interp)
 
  exec = next;
  }
+ EXECUTOR_LIST_UNLOCK(interp);
  for (Py_ssize_t i = 0; i < PyList_GET_SIZE(invalidate); i++) {
  PyObject *exec = PyList_GET_ITEM(invalidate, i);
  executor_clear(exec);
@@ -1810,10 +1856,12 @@ _PyDumpExecutors(FILE *out)
  fprintf(out, "digraph ideal {\n\n");
  fprintf(out, " rankdir = \"LR\"\n\n");
  PyInterpreterState *interp = PyInterpreterState_Get();
+ EXECUTOR_LIST_LOCK(interp);
  for (_PyExecutorObject *exec = interp->executor_list_head; exec != NULL;) {
  executor_to_gv(exec, out);
  exec = exec->vm_data.links.next;
  }
+ EXECUTOR_LIST_UNLOCK(interp);
  fprintf(out, "}\n\n");
  return 0;
 }

Python/pystate.c

@@ -575,6 +575,9 @@ init_interpreter(PyInterpreterState *interp,
  interp->executor_list_head = NULL;
  interp->executor_deletion_list_head = NULL;
  interp->executor_deletion_list_remaining_capacity = 0;
+#ifdef Py_GIL_DISABLED
+ interp->executor_list_lock = (_PyRecursiveMutex){0};
+#endif
  interp->executor_creation_counter = JIT_CLEANUP_THRESHOLD;
  if (interp != &runtime->_main_interpreter) {
  /* Fix the self-referential, statically initialized fields. */