gh-59956: Fix Function Groupings in pystate.c (gh-101172)

This is a follow-up to gh-101161.  The objective is to make it easier to read Python/pystate.c by grouping the functions there in a consistent way.  This exclusively involves moving code around and adding various kinds of comments.

https://github.com/python/cpython/issues/59956

1 files changed, 352 insertions, 237 deletions

diff --git a/Python/pystate.c b/Python/pystate.c
index 756c1c7..464e5d6 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -41,19 +41,36 @@ extern "C" {
 static void _PyThreadState_Delete(PyThreadState *tstate, int check_current);
 
 
-/* the current thread state */
+/****************************************/
+/* helpers for the current thread state */
+/****************************************/
+
+// API for the current thread state is further down.
+
+/* "current" means one of:
+   - bound to the current OS thread
+   - holds the GIL
+ */
+
+//-------------------------------------------------
+// a highly efficient lookup for the current thread
+//-------------------------------------------------
+
+/*
+   The stored thread state is set by PyThraedState_Swap().
+
+   For each of these functions, the GIL mus be held by the current thread.
+ */
 
 static inline PyThreadState *
 current_fast_get(_PyRuntimeState *runtime)
 {
-    // The GIL must be held by the current thread.
     return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->tstate_current);
 }
 
 static inline void
 current_fast_set(_PyRuntimeState *runtime, PyThreadState *tstate)
 {
-    // The GIL must be held by the current thread.
     assert(tstate != NULL);
     _Py_atomic_store_relaxed(&runtime->tstate_current, (uintptr_t)tstate);
 }
@@ -61,11 +78,20 @@ current_fast_set(_PyRuntimeState *runtime, PyThreadState *tstate)
 static inline void
 current_fast_clear(_PyRuntimeState *runtime)
 {
-    // The GIL must be held by the current thread.
     _Py_atomic_store_relaxed(&runtime->tstate_current, (uintptr_t)NULL);
 }
 
 
+//------------------------------------------------
+// the thread state bound to the current OS thread
+//------------------------------------------------
+
+/*
+   The stored thread state is set by bind_tstate() (AKA PyThreadState_Bind().
+
+   The GIL does no need to be held for these.
+  */
+
 static int
 current_tss_initialized(_PyRuntimeState *runtime)
 {
@@ -148,6 +174,7 @@ current_tss_reinit(_PyRuntimeState *runtime)
 }
 #endif
 
+
 static void
 bind_tstate(PyThreadState *tstate)
 {
@@ -213,6 +240,11 @@ unbind_tstate(PyThreadState *tstate)
     tstate->_status = PyThreadState_UNBOUND;
 }
 
+
+//----------------------------------------------
+// the thread state that currently holds the GIL
+//----------------------------------------------
+
 /* This is not exported, as it is not reliable!  It can only
    ever be compared to the state for the *current* thread.
    * If not equal, then it doesn't matter that the actual
@@ -234,7 +266,13 @@ holds_gil(PyThreadState *tstate)
 }
 
 
-/* the global runtime */
+/****************************/
+/* the global runtime state */
+/****************************/
+
+//----------
+// lifecycle
+//----------
 
 /* Suppress deprecation warning for PyBytesObject.ob_shash */
 _Py_COMP_DIAG_PUSH
@@ -438,6 +476,16 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime)
 #endif
 
 
+/*************************************/
+/* the per-interpreter runtime state */
+/*************************************/
+
+//----------
+// lifecycle
+//----------
+
+/* Calling this indicates that the runtime is ready to create interpreters. */
+
 PyStatus
 _PyInterpreterState_Enable(_PyRuntimeState *runtime)
 {
@@ -464,6 +512,7 @@ _PyInterpreterState_Enable(_PyRuntimeState *runtime)
     return _PyStatus_OK();
 }
 
+
 static PyInterpreterState *
 alloc_interpreter(void)
 {
@@ -798,19 +847,40 @@ _PyInterpreterState_DeleteExceptMain(_PyRuntimeState *runtime)
 #endif
 
 
-PyInterpreterState *
-PyInterpreterState_Get(void)
+// Used by finalize_modules()
+void
+_PyInterpreterState_ClearModules(PyInterpreterState *interp)
 {
-    PyThreadState *tstate = current_fast_get(&_PyRuntime);
-    _Py_EnsureTstateNotNULL(tstate);
-    PyInterpreterState *interp = tstate->interp;
-    if (interp == NULL) {
-        Py_FatalError("no current interpreter");
+    if (!interp->modules_by_index) {
+        return;
+    }
+
+    Py_ssize_t i;
+    for (i = 0; i < PyList_GET_SIZE(interp->modules_by_index); i++) {
+        PyObject *m = PyList_GET_ITEM(interp->modules_by_index, i);
+        if (PyModule_Check(m)) {
+            /* cleanup the saved copy of module dicts */
+            PyModuleDef *md = PyModule_GetDef(m);
+            if (md) {
+                Py_CLEAR(md->m_base.m_copy);
+            }
+        }
+    }
+
+    /* Setting modules_by_index to NULL could be dangerous, so we
+       clear the list instead. */
+    if (PyList_SetSlice(interp->modules_by_index,
+                        0, PyList_GET_SIZE(interp->modules_by_index),
+                        NULL)) {
+        PyErr_WriteUnraisable(interp->modules_by_index);
     }
-    return interp;
 }
 
 
+//----------
+// accessors
+//----------
+
 int64_t
 PyInterpreterState_GetID(PyInterpreterState *interp)
 {
@@ -822,41 +892,6 @@ PyInterpreterState_GetID(PyInterpreterState *interp)
 }
 
 
-static PyInterpreterState *
-interp_look_up_id(_PyRuntimeState *runtime, int64_t requested_id)
-{
-    PyInterpreterState *interp = runtime->interpreters.head;
-    while (interp != NULL) {
-        int64_t id = PyInterpreterState_GetID(interp);
-        if (id < 0) {
-            return NULL;
-        }
-        if (requested_id == id) {
-            return interp;
-        }
-        interp = PyInterpreterState_Next(interp);
-    }
-    return NULL;
-}
-
-PyInterpreterState *
-_PyInterpreterState_LookUpID(int64_t requested_id)
-{
-    PyInterpreterState *interp = NULL;
-    if (requested_id >= 0) {
-        _PyRuntimeState *runtime = &_PyRuntime;
-        HEAD_LOCK(runtime);
-        interp = interp_look_up_id(runtime, requested_id);
-        HEAD_UNLOCK(runtime);
-    }
-    if (interp == NULL && !PyErr_Occurred()) {
-        PyErr_Format(PyExc_RuntimeError,
-                     "unrecognized interpreter ID %lld", requested_id);
-    }
-    return interp;
-}
-
-
 int
 _PyInterpreterState_IDInitref(PyInterpreterState *interp)
 {
@@ -945,6 +980,76 @@ PyInterpreterState_GetDict(PyInterpreterState *interp)
     return interp->dict;
 }
 
+
+//-----------------------------
+// look up an interpreter state
+//-----------------------------
+
+/* Return the interpreter associated with the current OS thread.
+
+   The GIL must be held.
+  */
+
+PyInterpreterState *
+PyInterpreterState_Get(void)
+{
+    PyThreadState *tstate = current_fast_get(&_PyRuntime);
+    _Py_EnsureTstateNotNULL(tstate);
+    PyInterpreterState *interp = tstate->interp;
+    if (interp == NULL) {
+        Py_FatalError("no current interpreter");
+    }
+    return interp;
+}
+
+
+static PyInterpreterState *
+interp_look_up_id(_PyRuntimeState *runtime, int64_t requested_id)
+{
+    PyInterpreterState *interp = runtime->interpreters.head;
+    while (interp != NULL) {
+        int64_t id = PyInterpreterState_GetID(interp);
+        if (id < 0) {
+            return NULL;
+        }
+        if (requested_id == id) {
+            return interp;
+        }
+        interp = PyInterpreterState_Next(interp);
+    }
+    return NULL;
+}
+
+/* Return the interpreter state with the given ID.
+
+   Fail with RuntimeError if the interpreter is not found. */
+
+PyInterpreterState *
+_PyInterpreterState_LookUpID(int64_t requested_id)
+{
+    PyInterpreterState *interp = NULL;
+    if (requested_id >= 0) {
+        _PyRuntimeState *runtime = &_PyRuntime;
+        HEAD_LOCK(runtime);
+        interp = interp_look_up_id(runtime, requested_id);
+        HEAD_UNLOCK(runtime);
+    }
+    if (interp == NULL && !PyErr_Occurred()) {
+        PyErr_Format(PyExc_RuntimeError,
+                     "unrecognized interpreter ID %lld", requested_id);
+    }
+    return interp;
+}
+
+
+/********************************/
+/* the per-thread runtime state */
+/********************************/
+
+//----------
+// lifecycle
+//----------
+
 /* Minimum size of data stack chunk */
 #define DATA_STACK_CHUNK_SIZE (16*1024)
 
@@ -1107,140 +1212,6 @@ _PyThreadState_Init(PyThreadState *tstate)
 }
 
 void
-_PyThreadState_Bind(PyThreadState *tstate)
-{
-    bind_tstate(tstate);
-}
-
-PyObject*
-PyState_FindModule(PyModuleDef* module)
-{
-    Py_ssize_t index = module->m_base.m_index;
-    PyInterpreterState *state = _PyInterpreterState_GET();
-    PyObject *res;
-    if (module->m_slots) {
-        return NULL;
-    }
-    if (index == 0)
-        return NULL;
-    if (state->modules_by_index == NULL)
-        return NULL;
-    if (index >= PyList_GET_SIZE(state->modules_by_index))
-        return NULL;
-    res = PyList_GET_ITEM(state->modules_by_index, index);
-    return res==Py_None ? NULL : res;
-}
-
-int
-_PyState_AddModule(PyThreadState *tstate, PyObject* module, PyModuleDef* def)
-{
-    if (!def) {
-        assert(_PyErr_Occurred(tstate));
-        return -1;
-    }
-    if (def->m_slots) {
-        _PyErr_SetString(tstate,
-                         PyExc_SystemError,
-                         "PyState_AddModule called on module with slots");
-        return -1;
-    }
-
-    PyInterpreterState *interp = tstate->interp;
-    if (!interp->modules_by_index) {
-        interp->modules_by_index = PyList_New(0);
-        if (!interp->modules_by_index) {
-            return -1;
-        }
-    }
-
-    while (PyList_GET_SIZE(interp->modules_by_index) <= def->m_base.m_index) {
-        if (PyList_Append(interp->modules_by_index, Py_None) < 0) {
-            return -1;
-        }
-    }
-
-    return PyList_SetItem(interp->modules_by_index,
-                          def->m_base.m_index, Py_NewRef(module));
-}
-
-int
-PyState_AddModule(PyObject* module, PyModuleDef* def)
-{
-    if (!def) {
-        Py_FatalError("module definition is NULL");
-        return -1;
-    }
-
-    PyThreadState *tstate = current_fast_get(&_PyRuntime);
-    PyInterpreterState *interp = tstate->interp;
-    Py_ssize_t index = def->m_base.m_index;
-    if (interp->modules_by_index &&
-        index < PyList_GET_SIZE(interp->modules_by_index) &&
-        module == PyList_GET_ITEM(interp->modules_by_index, index))
-    {
-        _Py_FatalErrorFormat(__func__, "module %p already added", module);
-        return -1;
-    }
-    return _PyState_AddModule(tstate, module, def);
-}
-
-int
-PyState_RemoveModule(PyModuleDef* def)
-{
-    PyThreadState *tstate = current_fast_get(&_PyRuntime);
-    PyInterpreterState *interp = tstate->interp;
-
-    if (def->m_slots) {
-        _PyErr_SetString(tstate,
-                         PyExc_SystemError,
-                         "PyState_RemoveModule called on module with slots");
-        return -1;
-    }
-
-    Py_ssize_t index = def->m_base.m_index;
-    if (index == 0) {
-        Py_FatalError("invalid module index");
-    }
-    if (interp->modules_by_index == NULL) {
-        Py_FatalError("Interpreters module-list not accessible.");
-    }
-    if (index > PyList_GET_SIZE(interp->modules_by_index)) {
-        Py_FatalError("Module index out of bounds.");
-    }
-
-    return PyList_SetItem(interp->modules_by_index, index, Py_NewRef(Py_None));
-}
-
-// Used by finalize_modules()
-void
-_PyInterpreterState_ClearModules(PyInterpreterState *interp)
-{
-    if (!interp->modules_by_index) {
-        return;
-    }
-
-    Py_ssize_t i;
-    for (i = 0; i < PyList_GET_SIZE(interp->modules_by_index); i++) {
-        PyObject *m = PyList_GET_ITEM(interp->modules_by_index, i);
-        if (PyModule_Check(m)) {
-            /* cleanup the saved copy of module dicts */
-            PyModuleDef *md = PyModule_GetDef(m);
-            if (md) {
-                Py_CLEAR(md->m_base.m_copy);
-            }
-        }
-    }
-
-    /* Setting modules_by_index to NULL could be dangerous, so we
-       clear the list instead. */
-    if (PyList_SetSlice(interp->modules_by_index,
-                        0, PyList_GET_SIZE(interp->modules_by_index),
-                        NULL)) {
-        PyErr_WriteUnraisable(interp->modules_by_index);
-    }
-}
-
-void
 PyThreadState_Clear(PyThreadState *tstate)
 {
     int verbose = _PyInterpreterState_GetConfig(tstate->interp)->verbose;
@@ -1406,59 +1377,9 @@ _PyThreadState_DeleteExcept(_PyRuntimeState *runtime, PyThreadState *tstate)
 }
 
 
-PyThreadState *
-_PyThreadState_UncheckedGet(void)
-{
-    return current_fast_get(&_PyRuntime);
-}
-
-
-PyThreadState *
-PyThreadState_Get(void)
-{
-    PyThreadState *tstate = current_fast_get(&_PyRuntime);
-    _Py_EnsureTstateNotNULL(tstate);
-    return tstate;
-}
-
-
-PyThreadState *
-_PyThreadState_Swap(_PyRuntimeState *runtime, PyThreadState *newts)
-{
-    PyThreadState *oldts = current_fast_get(runtime);
-
-    if (newts == NULL) {
-        current_fast_clear(runtime);
-    }
-    else {
-        current_fast_set(runtime, newts);
-    }
-    /* It should not be possible for more than one thread state
-       to be used for a thread.  Check this the best we can in debug
-       builds.
-    */
-    // XXX The above isn't true when multiple interpreters are involved.
-#if defined(Py_DEBUG)
-    if (newts && current_tss_initialized(runtime)) {
-        /* This can be called from PyEval_RestoreThread(). Similar
-           to it, we need to ensure errno doesn't change.
-        */
-        int err = errno;
-        PyThreadState *check = current_tss_get(runtime);
-        if (check && check->interp == newts->interp && check != newts) {
-            Py_FatalError("Invalid thread state for this thread");
-        }
-        errno = err;
-    }
-#endif
-    return oldts;
-}
-
-PyThreadState *
-PyThreadState_Swap(PyThreadState *newts)
-{
-    return _PyThreadState_Swap(&_PyRuntime, newts);
-}
+//----------
+// accessors
+//----------
 
 /* An extension mechanism to store arbitrary additional per-thread state.
    PyThreadState_GetDict() returns a dictionary that can be used to hold such
@@ -1523,6 +1444,10 @@ PyThreadState_GetID(PyThreadState *tstate)
 }
 
 
+//----------
+// other API
+//----------
+
 /* Asynchronously raise an exception in a thread.
    Requested by Just van Rossum and Alex Martelli.
    To prevent naive misuse, you must write your own extension
@@ -1531,6 +1456,8 @@ PyThreadState_GetID(PyThreadState *tstate)
    match any known thread id).  Can be called with exc=NULL to clear an
    existing async exception.  This raises no exceptions. */
 
+// XXX Move this to Python/ceval_gil.c?
+// XXX Deprecate this.
 int
 PyThreadState_SetAsyncExc(unsigned long id, PyObject *exc)
 {
@@ -1568,8 +1495,79 @@ PyThreadState_SetAsyncExc(unsigned long id, PyObject *exc)
     return 0;
 }
 
-/* Routines for advanced debuggers, requested by David Beazley.
-   Don't use unless you know what you are doing! */
+
+//---------------------------------
+// API for the current thread state
+//---------------------------------
+
+PyThreadState *
+_PyThreadState_UncheckedGet(void)
+{
+    return current_fast_get(&_PyRuntime);
+}
+
+
+PyThreadState *
+PyThreadState_Get(void)
+{
+    PyThreadState *tstate = current_fast_get(&_PyRuntime);
+    _Py_EnsureTstateNotNULL(tstate);
+    return tstate;
+}
+
+
+PyThreadState *
+_PyThreadState_Swap(_PyRuntimeState *runtime, PyThreadState *newts)
+{
+    PyThreadState *oldts = current_fast_get(runtime);
+
+    if (newts == NULL) {
+        current_fast_clear(runtime);
+    }
+    else {
+        current_fast_set(runtime, newts);
+    }
+    /* It should not be possible for more than one thread state
+       to be used for a thread.  Check this the best we can in debug
+       builds.
+    */
+    // XXX The above isn't true when multiple interpreters are involved.
+#if defined(Py_DEBUG)
+    if (newts && current_tss_initialized(runtime)) {
+        /* This can be called from PyEval_RestoreThread(). Similar
+           to it, we need to ensure errno doesn't change.
+        */
+        int err = errno;
+        PyThreadState *check = current_tss_get(runtime);
+        if (check && check->interp == newts->interp && check != newts) {
+            Py_FatalError("Invalid thread state for this thread");
+        }
+        errno = err;
+    }
+#endif
+    return oldts;
+}
+
+PyThreadState *
+PyThreadState_Swap(PyThreadState *newts)
+{
+    return _PyThreadState_Swap(&_PyRuntime, newts);
+}
+
+
+void
+_PyThreadState_Bind(PyThreadState *tstate)
+{
+    bind_tstate(tstate);
+}
+
+
+/***********************************/
+/* routines for advanced debuggers */
+/***********************************/
+
+// (requested by David Beazley)
+// Don't use unless you know what you are doing!
 
 PyInterpreterState *
 PyInterpreterState_Head(void)
@@ -1598,6 +1596,11 @@ PyThreadState_Next(PyThreadState *tstate) {
     return tstate->next;
 }
 
+
+/********************************************/
+/* reporting execution state of all threads */
+/********************************************/
+
 /* The implementation of sys._current_frames().  This is intended to be
    called with the GIL held, as it will be when called via
    sys._current_frames().  It's possible it would work fine even without
@@ -1659,6 +1662,11 @@ done:
     return result;
 }
 
+/* The implementation of sys._current_exceptions().  This is intended to be
+   called with the GIL held, as it will be when called via
+   sys._current_exceptions().  It's possible it would work fine even without
+   the GIL held, but haven't thought enough about that.
+*/
 PyObject *
 _PyThread_CurrentExceptions(void)
 {
@@ -1718,7 +1726,114 @@ done:
     return result;
 }
 
+
+/****************/
+/* module state */
+/****************/
+
+PyObject*
+PyState_FindModule(PyModuleDef* module)
+{
+    Py_ssize_t index = module->m_base.m_index;
+    PyInterpreterState *state = _PyInterpreterState_GET();
+    PyObject *res;
+    if (module->m_slots) {
+        return NULL;
+    }
+    if (index == 0)
+        return NULL;
+    if (state->modules_by_index == NULL)
+        return NULL;
+    if (index >= PyList_GET_SIZE(state->modules_by_index))
+        return NULL;
+    res = PyList_GET_ITEM(state->modules_by_index, index);
+    return res==Py_None ? NULL : res;
+}
+
+int
+_PyState_AddModule(PyThreadState *tstate, PyObject* module, PyModuleDef* def)
+{
+    if (!def) {
+        assert(_PyErr_Occurred(tstate));
+        return -1;
+    }
+    if (def->m_slots) {
+        _PyErr_SetString(tstate,
+                         PyExc_SystemError,
+                         "PyState_AddModule called on module with slots");
+        return -1;
+    }
+
+    PyInterpreterState *interp = tstate->interp;
+    if (!interp->modules_by_index) {
+        interp->modules_by_index = PyList_New(0);
+        if (!interp->modules_by_index) {
+            return -1;
+        }
+    }
+
+    while (PyList_GET_SIZE(interp->modules_by_index) <= def->m_base.m_index) {
+        if (PyList_Append(interp->modules_by_index, Py_None) < 0) {
+            return -1;
+        }
+    }
+
+    return PyList_SetItem(interp->modules_by_index,
+                          def->m_base.m_index, Py_NewRef(module));
+}
+
+int
+PyState_AddModule(PyObject* module, PyModuleDef* def)
+{
+    if (!def) {
+        Py_FatalError("module definition is NULL");
+        return -1;
+    }
+
+    PyThreadState *tstate = current_fast_get(&_PyRuntime);
+    PyInterpreterState *interp = tstate->interp;
+    Py_ssize_t index = def->m_base.m_index;
+    if (interp->modules_by_index &&
+        index < PyList_GET_SIZE(interp->modules_by_index) &&
+        module == PyList_GET_ITEM(interp->modules_by_index, index))
+    {
+        _Py_FatalErrorFormat(__func__, "module %p already added", module);
+        return -1;
+    }
+    return _PyState_AddModule(tstate, module, def);
+}
+
+int
+PyState_RemoveModule(PyModuleDef* def)
+{
+    PyThreadState *tstate = current_fast_get(&_PyRuntime);
+    PyInterpreterState *interp = tstate->interp;
+
+    if (def->m_slots) {
+        _PyErr_SetString(tstate,
+                         PyExc_SystemError,
+                         "PyState_RemoveModule called on module with slots");
+        return -1;
+    }
+
+    Py_ssize_t index = def->m_base.m_index;
+    if (index == 0) {
+        Py_FatalError("invalid module index");
+    }
+    if (interp->modules_by_index == NULL) {
+        Py_FatalError("Interpreters module-list not accessible.");
+    }
+    if (index > PyList_GET_SIZE(interp->modules_by_index)) {
+        Py_FatalError("Module index out of bounds.");
+    }
+
+    return PyList_SetItem(interp->modules_by_index, index, Py_NewRef(Py_None));
+}
+
+
+/***********************************/
 /* Python "auto thread state" API. */
+/***********************************/
 
 /* Internal initialization/finalization functions called by
    Py_Initialize/Py_FinalizeEx