bpo-33608: Minor cleanup related to pending calls. (gh-12247)

Author: ericsnowcurrently

Include/internal/pycore_ceval.h

@@ -12,7 +12,6 @@ extern "C" {
 #include "pythread.h"
 
 struct _pending_calls {
-    unsigned long main_thread;
     PyThread_type_lock lock;
     /* Request for running pending calls. */
     _Py_atomic_int calls_to_do;

Include/internal/pycore_pystate.h

@@ -31,6 +31,8 @@ struct _is {
     int64_t id_refcount;
     PyThread_type_lock id_mutex;
 
+    int finalizing;
+
     PyObject *modules;
     PyObject *modules_by_index;
     PyObject *sysdict;
@@ -207,6 +209,8 @@ typedef struct pyruntimestate {
         struct _xidregitem *head;
     } xidregistry;
 
+    unsigned long main_thread;
+
 #define NEXITFUNCS 32
     void (*exitfuncs[NEXITFUNCS])(void);
     int nexitfuncs;

Python/ceval.c

@@ -174,9 +174,11 @@ PyEval_InitThreads(void)
     PyThread_init_thread();
     create_gil();
     take_gil(_PyThreadState_GET());
-    _PyRuntime.ceval.pending.main_thread = PyThread_get_thread_ident();
-    if (!_PyRuntime.ceval.pending.lock)
+    // Set it to the ID of the main thread of the main interpreter.
+    _PyRuntime.main_thread = PyThread_get_thread_ident();
+    if (!_PyRuntime.ceval.pending.lock) {
         _PyRuntime.ceval.pending.lock = PyThread_allocate_lock();
+    }
 }
 
 void
@@ -243,9 +245,9 @@ PyEval_ReInitThreads(void)
     if (!gil_created())
         return;
     recreate_gil();
-    _PyRuntime.ceval.pending.lock = PyThread_allocate_lock();
     take_gil(current_tstate);
-    _PyRuntime.ceval.pending.main_thread = PyThread_get_thread_ident();
+    _PyRuntime.main_thread = PyThread_get_thread_ident();
+    _PyRuntime.ceval.pending.lock = PyThread_allocate_lock();
 
     /* Destroy all threads except the current one */
     _PyThreadState_DeleteExcept(current_tstate);
@@ -323,6 +325,35 @@ _PyEval_SignalReceived(void)
     SIGNAL_PENDING_SIGNALS();
 }
 
+/* Push one item onto the queue while holding the lock. */
+static int
+_push_pending_call(int (*func)(void *), void *arg)
+{
+    int i = _PyRuntime.ceval.pending.last;
+    int j = (i + 1) % NPENDINGCALLS;
+    if (j == _PyRuntime.ceval.pending.first) {
+        return -1; /* Queue full */
+    }
+    _PyRuntime.ceval.pending.calls[i].func = func;
+    _PyRuntime.ceval.pending.calls[i].arg = arg;
+    _PyRuntime.ceval.pending.last = j;
+    return 0;
+}
+
+/* Pop one item off the queue while holding the lock. */
+static void
+_pop_pending_call(int (**func)(void *), void **arg)
+{
+    int i = _PyRuntime.ceval.pending.first;
+    if (i == _PyRuntime.ceval.pending.last) {
+        return; /* Queue empty */
+    }
+
+    *func = _PyRuntime.ceval.pending.calls[i].func;
+    *arg = _PyRuntime.ceval.pending.calls[i].arg;
+    _PyRuntime.ceval.pending.first = (i + 1) % NPENDINGCALLS;
+}
+
 /* This implementation is thread-safe.  It allows
    scheduling to be made from any thread, and even from an executing
    callback.
@@ -331,7 +362,6 @@ _PyEval_SignalReceived(void)
 int
 Py_AddPendingCall(int (*func)(void *), void *arg)
 {
-    int i, j, result=0;
     PyThread_type_lock lock = _PyRuntime.ceval.pending.lock;
 
     /* try a few times for the lock.  Since this mechanism is used
@@ -346,6 +376,7 @@ Py_AddPendingCall(int (*func)(void *), void *arg)
      * this function is called before any bytecode evaluation takes place.
      */
     if (lock != NULL) {
+        int i;
         for (i = 0; i<100; i++) {
             if (PyThread_acquire_lock(lock, NOWAIT_LOCK))
                 break;
@@ -354,15 +385,8 @@ Py_AddPendingCall(int (*func)(void *), void *arg)
             return -1;
     }
 
-    i = _PyRuntime.ceval.pending.last;
-    j = (i + 1) % NPENDINGCALLS;
-    if (j == _PyRuntime.ceval.pending.first) {
-        result = -1; /* Queue full */
-    } else {
-        _PyRuntime.ceval.pending.calls[i].func = func;
-        _PyRuntime.ceval.pending.calls[i].arg = arg;
-        _PyRuntime.ceval.pending.last = j;
-    }
+    int result = _push_pending_call(func, arg);
+
     /* signal main loop */
     SIGNAL_PENDING_CALLS();
     if (lock != NULL)
@@ -373,10 +397,10 @@ Py_AddPendingCall(int (*func)(void *), void *arg)
 static int
 handle_signals(void)
 {
-    /* Only handle signals on main thread. */
-    if (_PyRuntime.ceval.pending.main_thread &&
-        PyThread_get_thread_ident() != _PyRuntime.ceval.pending.main_thread)
-    {
+    /* Only handle signals on main thread.  PyEval_InitThreads must
+     * have been called already.
+     */
+    if (PyThread_get_thread_ident() != _PyRuntime.main_thread) {
         return 0;
     }
     /*
@@ -401,9 +425,7 @@ make_pending_calls(void)
     static int busy = 0;
 
     /* only service pending calls on main thread */
-    if (_PyRuntime.ceval.pending.main_thread &&
-        PyThread_get_thread_ident() != _PyRuntime.ceval.pending.main_thread)
-    {
+    if (PyThread_get_thread_ident() != _PyRuntime.main_thread) {
         return 0;
     }
 
@@ -428,24 +450,18 @@ make_pending_calls(void)
 
     /* perform a bounded number of calls, in case of recursion */
     for (int i=0; i<NPENDINGCALLS; i++) {
-        int j;
-        int (*func)(void *);
+        int (*func)(void *) = NULL;
         void *arg = NULL;
 
         /* pop one item off the queue while holding the lock */
         PyThread_acquire_lock(_PyRuntime.ceval.pending.lock, WAIT_LOCK);
-        j = _PyRuntime.ceval.pending.first;
-        if (j == _PyRuntime.ceval.pending.last) {
-            func = NULL; /* Queue empty */
-        } else {
-            func = _PyRuntime.ceval.pending.calls[j].func;
-            arg = _PyRuntime.ceval.pending.calls[j].arg;
-            _PyRuntime.ceval.pending.first = (j + 1) % NPENDINGCALLS;
-        }
+        _pop_pending_call(&func, &arg);
         PyThread_release_lock(_PyRuntime.ceval.pending.lock);
+
         /* having released the lock, perform the callback */
-        if (func == NULL)
+        if (func == NULL) {
             break;
+        }
         res = func(arg);
         if (res) {
             goto error;

Python/pylifecycle.c

@@ -1460,6 +1460,7 @@ Py_EndInterpreter(PyThreadState *tstate)
         Py_FatalError("Py_EndInterpreter: thread is not current");
     if (tstate->frame != NULL)
         Py_FatalError("Py_EndInterpreter: thread still has a frame");
+    interp->finalizing = 1;
 
     wait_for_thread_shutdown();
 

Python/pystate.c

@@ -60,6 +60,8 @@ _PyRuntimeState_Init_impl(_PyRuntimeState *runtime)
         return _Py_INIT_ERR("Can't initialize threads for cross-interpreter data registry");
     }
 
+    // runtime->main_thread is set in PyEval_InitThreads().
+
     return _Py_INIT_OK();
 }
 
@@ -133,42 +135,19 @@ PyInterpreterState_New(void)
         return NULL;
     }
 
+    memset(interp, 0, sizeof(*interp));
     interp->id_refcount = -1;
-    interp->id_mutex = NULL;
-    interp->modules = NULL;
-    interp->modules_by_index = NULL;
-    interp->sysdict = NULL;
-    interp->builtins = NULL;
-    interp->builtins_copy = NULL;
-    interp->tstate_head = NULL;
     interp->check_interval = 100;
-    interp->num_threads = 0;
-    interp->pythread_stacksize = 0;
-    interp->codec_search_path = NULL;
-    interp->codec_search_cache = NULL;
-    interp->codec_error_registry = NULL;
-    interp->codecs_initialized = 0;
-    interp->fscodec_initialized = 0;
     interp->core_config = _PyCoreConfig_INIT;
     interp->config = _PyMainInterpreterConfig_INIT;
-    interp->importlib = NULL;
-    interp->import_func = NULL;
     interp->eval_frame = _PyEval_EvalFrameDefault;
-    interp->co_extra_user_count = 0;
 #ifdef HAVE_DLOPEN
 #if HAVE_DECL_RTLD_NOW
     interp->dlopenflags = RTLD_NOW;
 #else
     interp->dlopenflags = RTLD_LAZY;
 #endif
 #endif
-#ifdef HAVE_FORK
-    interp->before_forkers = NULL;
-    interp->after_forkers_parent = NULL;
-    interp->after_forkers_child = NULL;
-#endif
-    interp->pyexitfunc = NULL;
-    interp->pyexitmodule = NULL;
 
     HEAD_LOCK();
     if (_PyRuntime.interpreters.next_id < 0) {
@@ -223,6 +202,9 @@ PyInterpreterState_Clear(PyInterpreterState *interp)
     Py_CLEAR(interp->after_forkers_parent);
     Py_CLEAR(interp->after_forkers_child);
 #endif
+    // XXX Once we have one allocator per interpreter (i.e.
+    // per-interpreter GC) we must ensure that all of the interpreter's
+    // objects have been cleaned up at the point.
 }
 
 
@@ -334,28 +316,39 @@ PyInterpreterState_GetID(PyInterpreterState *interp)
 }
 
 
-PyInterpreterState *
-_PyInterpreterState_LookUpID(PY_INT64_T requested_id)
+static PyInterpreterState *
+interp_look_up_id(PY_INT64_T requested_id)
 {
-    if (requested_id < 0)
-        goto error;
-
     PyInterpreterState *interp = PyInterpreterState_Head();
     while (interp != NULL) {
         PY_INT64_T id = PyInterpreterState_GetID(interp);
-        if (id < 0)
+        if (id < 0) {
             return NULL;
-        if (requested_id == id)
+        }
+        if (requested_id == id) {
             return interp;
+        }
         interp = PyInterpreterState_Next(interp);
     }
-
-error:
-    PyErr_Format(PyExc_RuntimeError,
-                 "unrecognized interpreter ID %lld", requested_id);
     return NULL;
 }
 
+PyInterpreterState *
+_PyInterpreterState_LookUpID(PY_INT64_T requested_id)
+{
+    PyInterpreterState *interp = NULL;
+    if (requested_id >= 0) {
+        HEAD_LOCK();
+        interp = interp_look_up_id(requested_id);
+        HEAD_UNLOCK();
+    }
+    if (interp == NULL && !PyErr_Occurred()) {
+        PyErr_Format(PyExc_RuntimeError,
+                     "unrecognized interpreter ID %lld", requested_id);
+    }
+    return interp;
+}
+
 
 int
 _PyInterpreterState_IDInitref(PyInterpreterState *interp)