bpo-40226: PyInterpreterState_Delete() deletes pending calls

Include/internal/pycore_ceval.h

@@ -17,8 +17,8 @@ struct _frame;
 
 extern void _Py_FinishPendingCalls(PyThreadState *tstate);
 extern void _PyEval_InitRuntimeState(struct _ceval_runtime_state *);
-extern void _PyEval_InitState(struct _ceval_state *);
-extern void _PyEval_FiniThreads(PyThreadState *tstate);
+extern int _PyEval_InitState(struct _ceval_state *ceval);
+extern void _PyEval_FiniState(struct _ceval_state *ceval);
 PyAPI_FUNC(void) _PyEval_SignalReceived(PyThreadState *tstate);
 PyAPI_FUNC(int) _PyEval_AddPendingCall(
     PyThreadState *tstate,
@@ -51,7 +51,8 @@ extern PyObject *_PyEval_EvalCode(
     PyObject *name, PyObject *qualname);
 
 extern int _PyEval_ThreadsInitialized(_PyRuntimeState *runtime);
-extern PyStatus _PyEval_InitThreads(PyThreadState *tstate);
+extern PyStatus _PyEval_InitGIL(PyThreadState *tstate);
+extern void _PyEval_FiniGIL(PyThreadState *tstate);
 
 extern void _PyEval_ReleaseLock(PyThreadState *tstate);
 

Python/ceval.c

@@ -275,54 +275,52 @@ PyEval_ThreadsInitialized(void)
 }
 
 PyStatus
-_PyEval_InitThreads(PyThreadState *tstate)
+_PyEval_InitGIL(PyThreadState *tstate)
 {
-    assert(is_tstate_valid(tstate));
+    if (!_Py_IsMainInterpreter(tstate)) {
+        /* Currently, the GIL is shared by all interpreters,
+           and only the main interpreter is responsible to create
+           and destroy it. */
+        return _PyStatus_OK();
+    }
 
-    if (_Py_IsMainInterpreter(tstate)) {
-        struct _gil_runtime_state *gil = &tstate->interp->runtime->ceval.gil;
-        if (gil_created(gil)) {
-            return _PyStatus_OK();
-        }
+    struct _gil_runtime_state *gil = &tstate->interp->runtime->ceval.gil;
+    assert(!gil_created(gil));
 
-        PyThread_init_thread();
-        create_gil(gil);
+    PyThread_init_thread();
+    create_gil(gil);
 
-        take_gil(tstate);
-    }
-
-    struct _pending_calls *pending = &tstate->interp->ceval.pending;
-    assert(pending->lock == NULL);
-    pending->lock = PyThread_allocate_lock();
-    if (pending->lock == NULL) {
-        return _PyStatus_NO_MEMORY();
-    }
+    take_gil(tstate);
 
+    assert(gil_created(gil));
     return _PyStatus_OK();
 }
 
 void
-PyEval_InitThreads(void)
+_PyEval_FiniGIL(PyThreadState *tstate)
 {
-    /* Do nothing: kept for backward compatibility */
-}
+    if (!_Py_IsMainInterpreter(tstate)) {
+        /* Currently, the GIL is shared by all interpreters,
+           and only the main interpreter is responsible to create
+           and destroy it. */
+        return;
+    }
 
-void
-_PyEval_FiniThreads(PyThreadState *tstate)
-{
     struct _gil_runtime_state *gil = &tstate->interp->runtime->ceval.gil;
     if (!gil_created(gil)) {
+        /* First Py_InitializeFromConfig() call: the GIL doesn't exist
+           yet: do nothing. */
         return;
     }
 
     destroy_gil(gil);
     assert(!gil_created(gil));
+}
 
-    struct _pending_calls *pending = &tstate->interp->ceval.pending;
-    if (pending->lock != NULL) {
-        PyThread_free_lock(pending->lock);
-        pending->lock = NULL;
-    }
+void
+PyEval_InitThreads(void)
+{
+    /* Do nothing: kept for backward compatibility */
 }
 
 void
@@ -544,6 +542,10 @@ _PyEval_AddPendingCall(PyThreadState *tstate,
 {
     struct _pending_calls *pending = &tstate->interp->ceval.pending;
 
+    /* Ensure that _PyEval_InitPendingCalls() was called
+       and that _PyEval_FiniPendingCalls() is not called yet. */
+    assert(pending->lock != NULL);
+
     PyThread_acquire_lock(pending->lock, WAIT_LOCK);
     if (pending->finishing) {
         PyThread_release_lock(pending->lock);
@@ -721,10 +723,27 @@ _PyEval_InitRuntimeState(struct _ceval_runtime_state *ceval)
     _gil_initialize(&ceval->gil);
 }
 
-void
+int
 _PyEval_InitState(struct _ceval_state *ceval)
 {
-    /* PyInterpreterState_New() initializes ceval to zero */
+    struct _pending_calls *pending = &ceval->pending;
+    assert(pending->lock == NULL);
+
+    pending->lock = PyThread_allocate_lock();
+    if (pending->lock == NULL) {
+        return -1;
+    }
+    return 0;
+}
+
+void
+_PyEval_FiniState(struct _ceval_state *ceval)
+{
+    struct _pending_calls *pending = &ceval->pending;
+    if (pending->lock != NULL) {
+        PyThread_free_lock(pending->lock);
+        pending->lock = NULL;
+    }
 }
 
 int

Python/pylifecycle.c

@@ -523,6 +523,31 @@ pycore_init_runtime(_PyRuntimeState *runtime,
 }
 
 
+static PyStatus
+init_interp_create_gil(PyThreadState *tstate)
+{
+    PyStatus status;
+
+    /* finalize_interp_delete() comment explains why _PyEval_FiniGIL() is
+       only called here. */
+    _PyEval_FiniGIL(tstate);
+
+    /* Auto-thread-state API */
+    status = _PyGILState_Init(tstate);
+    if (_PyStatus_EXCEPTION(status)) {
+        return status;
+    }
+
+    /* Create the GIL and take it */
+    status = _PyEval_InitGIL(tstate);
+    if (_PyStatus_EXCEPTION(status)) {
+        return status;
+    }
+
+    return _PyStatus_OK();
+}
+
+
 static PyStatus
 pycore_create_interpreter(_PyRuntimeState *runtime,
                           const PyConfig *config,
@@ -544,21 +569,7 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
     }
     (void) PyThreadState_Swap(tstate);
 
-    /* We can't call _PyEval_FiniThreads() in Py_FinalizeEx because
-       destroying the GIL might fail when it is being referenced from
-       another running thread (see issue #9901).
-       Instead we destroy the previously created GIL here, which ensures
-       that we can call Py_Initialize / Py_FinalizeEx multiple times. */
-    _PyEval_FiniThreads(tstate);
-
-    /* Auto-thread-state API */
-    status = _PyGILState_Init(tstate);
-    if (_PyStatus_EXCEPTION(status)) {
-        return status;
-    }
-
-    /* Create the GIL and the pending calls lock */
-    status = _PyEval_InitThreads(tstate);
+    status = init_interp_create_gil(tstate);
     if (_PyStatus_EXCEPTION(status)) {
         return status;
     }
@@ -1323,6 +1334,12 @@ finalize_interp_delete(PyThreadState *tstate)
         _PyGILState_Fini(tstate);
     }
 
+    /* We can't call _PyEval_FiniGIL() here because destroying the GIL lock can
+       fail when it is being awaited by another running daemon thread (see
+       bpo-9901). Instead pycore_create_interpreter() destroys the previously
+       created GIL, which ensures that Py_Initialize / Py_FinalizeEx can be
+       called multiple times. */
+
     PyInterpreterState_Delete(tstate->interp);
 }
 
@@ -1578,8 +1595,7 @@ new_interpreter(PyThreadState **tstate_p)
         goto error;
     }
 
-    /* Create the pending calls lock */
-    status = _PyEval_InitThreads(tstate);
+    status = init_interp_create_gil(tstate);
     if (_PyStatus_EXCEPTION(status)) {
         return status;
     }

Python/pystate.c

@@ -222,7 +222,10 @@ PyInterpreterState_New(void)
     _PyRuntimeState *runtime = &_PyRuntime;
     interp->runtime = runtime;
 
-    _PyEval_InitState(&interp->ceval);
+    if (_PyEval_InitState(&interp->ceval) < 0) {
+        goto out_of_memory;
+    }
+
     _PyGC_InitState(&interp->gc);
     PyConfig_InitPythonConfig(&interp->config);
 
@@ -267,6 +270,14 @@ PyInterpreterState_New(void)
     interp->audit_hooks = NULL;
 
     return interp;
+
+out_of_memory:
+    if (tstate != NULL) {
+        _PyErr_NoMemory(tstate);
+    }
+
+    PyMem_RawFree(interp);
+    return NULL;
 }
 
 
@@ -335,6 +346,8 @@ PyInterpreterState_Delete(PyInterpreterState *interp)
     struct pyinterpreters *interpreters = &runtime->interpreters;
     zapthreads(interp, 0);
 
+    _PyEval_FiniState(&interp->ceval);
+
     /* Delete current thread. After this, many C API calls become crashy. */
     _PyThreadState_Swap(&runtime->gilstate, NULL);
 
@@ -352,13 +365,15 @@ PyInterpreterState_Delete(PyInterpreterState *interp)
         Py_FatalError("remaining threads");
     }
     *p = interp->next;
+
     if (interpreters->main == interp) {
         interpreters->main = NULL;
         if (interpreters->head != NULL) {
             Py_FatalError("remaining subinterpreters");
         }
     }
     HEAD_UNLOCK(runtime);
+
     if (interp->id_mutex != NULL) {
         PyThread_free_lock(interp->id_mutex);
     }
@@ -1198,6 +1213,12 @@ PyThreadState_IsCurrent(PyThreadState *tstate)
 PyStatus
 _PyGILState_Init(PyThreadState *tstate)
 {
+    if (!_Py_IsMainInterpreter(tstate)) {
+        /* Currently, PyGILState is shared by all interpreters. The main
+         * interpreter is responsible to initialize it. */
+        return _PyStatus_OK();
+    }
+
     /* must init with valid states */
     assert(tstate != NULL);
     assert(tstate->interp != NULL);