python · LewisGaul · Nov 21, 2019 · Dec 11, 2019 · Dec 13, 2019 · Dec 13, 2019
diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst
@@ -277,7 +277,8 @@ Initializing and finalizing the interpreter
    Undo all initializations made by :c:func:`Py_Initialize` and subsequent use of
    Python/C API functions, and destroy all sub-interpreters (see
    :c:func:`Py_NewInterpreter` below) that were created and not yet destroyed since
-   the last call to :c:func:`Py_Initialize`.  Ideally, this frees all memory
+   the last call to :c:func:`Py_Initialize`.  A resource warning is emitted if
+   there were remaining sub-interpreters.  Ideally, this frees all memory
    allocated by the Python interpreter.  This is a no-op when called for a second
    time (without calling :c:func:`Py_Initialize` again first).  Normally the
    return value is ``0``.  If there were errors during finalization
@@ -300,7 +301,8 @@ Initializing and finalizing the interpreter
    freed.  Some memory allocated by extension modules may not be freed.  Some
    extensions may not work properly if their initialization routine is called more
    than once; this can happen if an application calls :c:func:`Py_Initialize` and
-   :c:func:`Py_FinalizeEx` more than once.
+   :c:func:`Py_FinalizeEx` more than once.  Must be called from the main
+   interpreter.
 
    .. audit-event:: cpython._PySys_ClearAuditHooks "" c.Py_FinalizeEx
 

@@ -84,6 +84,7 @@ typedef struct pyruntimestate {
            If that becomes a problem later then we can adjust, e.g. by
            using a Python int. */
         int64_t next_id;
+        int allow_new;
     } interpreters;
     // XXX Remove this field once we have a tp_* slot.
     struct _xidregistry {

diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py
@@ -194,6 +194,27 @@ def test_subinterps_distinct_state(self):
                 self.assertNotEqual(sub.tstate, main.tstate)
                 self.assertNotEqual(sub.modules, main.modules)
 
+    def test_finalize_subinterps(self):
+        """
+        bpo-36225: Subinterpreters should implicitly be torn down by
+        Py_Finalize().
+        """
+        _, err = self.run_embedded_interpreter("test_finalize_subinterps")
+        if support.verbose > 1:
+            print()
+            print(err)
+        self.assertIn("ResourceWarning: extra 2 interpreters", err)
+
+    def test_finalize_from_subinterp(self):
+        """
+        bpo-38865: Py_Finalize() should not be called from a subinterpreter.
+        """
+        _, err = self.run_embedded_interpreter("test_finalize_from_subinterp",
+                                               returncode=-6)
+        self.assertIn(
+            "Fatal Python error: Py_FinalizeEx: must be called from the main interpreter",
+            err)
+
     def test_forced_io_encoding(self):
         # Checks forced configuration of embedded interpreter IO streams
         env = dict(os.environ, PYTHONIOENCODING="utf-8:surrogateescape")

diff --git a/Misc/NEWS.d/next/C API/2019-12-14-15-20-51.bpo-36225.z7Nnrq.rst b/Misc/NEWS.d/next/C API/2019-12-14-15-20-51.bpo-36225.z7Nnrq.rst
@@ -0,0 +1 @@
+:func:`Py_FinalizeEx()` now implicitly cleans up subinterpreters, as the C API documentation suggests.
diff --git a/Programs/_testembed.c b/Programs/_testembed.c
@@ -16,7 +16,7 @@
 /*********************************************************
  * Embedded interpreter tests that need a custom exe
  *
- * Executed via 'EmbeddingTests' in Lib/test/test_capi.py
+ * Executed via 'EmbeddingTests' in Lib/test/test_embed.py
  *********************************************************/
 
 /* Use path starting with "./" avoids a search along the PATH */
@@ -83,6 +83,60 @@ static int test_repeated_init_and_subinterpreters(void)
     return 0;
 }
 
+/* bpo-36225: Implicitly tear down subinterpreters with Py_Finalize() */
+static int test_finalize_subinterps(void)
+{
+    PyThreadState *mainstate;
+    PyThreadState *interp_tstate;
+    PyGILState_STATE gilstate;
+    int i;
+
+    _testembed_Py_Initialize();
+    mainstate = PyThreadState_Get();
+
+    PyEval_ReleaseThread(mainstate);
+
+    gilstate = PyGILState_Ensure();
+    print_subinterp();
+    PyThreadState_Swap(NULL);
+
+    // Create 3 subinterpreters and destroy the last one.
+    for (i=0; i<3; i++) {
+        interp_tstate = Py_NewInterpreter();
+        print_subinterp();
+    }
+    PyThreadState_Swap(interp_tstate);
+    Py_EndInterpreter(interp_tstate);
+
+    // Switch back to the main interpreter and finalize the runtime.
+    PyThreadState_Swap(mainstate);
+    print_subinterp();
+    PyGILState_Release(gilstate);
+
+    PyEval_RestoreThread(mainstate);
+    Py_Finalize();
+
+    return 0;
+}
+
+/* bpo-38865: Py_Finalize() should not be called from a subinterpreter */
+static int test_finalize_from_subinterp(void)
+{
+    PyThreadState *subinterp_tstate;
+    int rc;
+
+    _testembed_Py_Initialize();
+    PyGILState_Ensure();
+    PyThreadState_Swap(NULL);
+
+    subinterp_tstate = Py_NewInterpreter();
+    PyThreadState_Swap(subinterp_tstate);
+
+    rc = Py_FinalizeEx();
+
+    return rc;
+}
+
 /*****************************************************
  * Test forcing a particular IO encoding
  *****************************************************/
@@ -1195,10 +1249,14 @@ static int test_audit_subinterpreter(void)
     PySys_AddAuditHook(_audit_subinterpreter_hook, NULL);
     _testembed_Py_Initialize();
 
+    PyThreadState *mainstate = PyThreadState_Get();
+
     Py_NewInterpreter();
     Py_NewInterpreter();
     Py_NewInterpreter();
 
+    // Currently unable to call Py_Finalize from subinterpreter thread, see bpo-37776.
+    PyThreadState_Swap(mainstate);
     Py_Finalize();
 
     switch (_audit_subinterpreter_interpreter_count) {
@@ -1707,6 +1765,8 @@ struct TestCase
 static struct TestCase TestCases[] = {
     {"test_forced_io_encoding", test_forced_io_encoding},
     {"test_repeated_init_and_subinterpreters", test_repeated_init_and_subinterpreters},
+    {"test_finalize_subinterps", test_finalize_subinterps},
+    {"test_finalize_from_subinterp", test_finalize_from_subinterp},
     {"test_pre_initialization_api", test_pre_initialization_api},
     {"test_pre_initialization_sys_options", test_pre_initialization_sys_options},
     {"test_bpo20891", test_bpo20891},

@@ -76,6 +76,7 @@ _PyRuntime_Initialize(void)
         return _PyStatus_OK();
     }
     runtime_initialized = 1;
+    _PyRuntime.interpreters.allow_new = 0;
 
     return _PyRuntimeState_Init(&_PyRuntime);
 }
@@ -1015,6 +1016,7 @@ init_interp_main(PyThreadState *tstate)
          */
         if (is_main_interp) {
             interp->runtime->initialized = 1;
+            interp->runtime->interpreters.allow_new = 1;
         }
         return _PyStatus_OK();
     }
@@ -1086,6 +1088,7 @@ init_interp_main(PyThreadState *tstate)
         }
 
         interp->runtime->initialized = 1;
+        interp->runtime->interpreters.allow_new = 1;
     }
 
     if (config->site_import) {
@@ -1646,6 +1649,42 @@ Py_FinalizeEx(void)
 
     /* Get current thread state and interpreter pointer */
     PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime);
+    PyInterpreterState *interp = tstate->interp;
+
+    /* Check we're running in the main interpreter (not yet supported to call
+     * from any interpreter).
+     */
+    if (interp != PyInterpreterState_Main()) {
+        Py_FatalError("must be called from the main interpreter\n");
+    }
+
+    // Finalize sub-interpreters.
+    PyThread_acquire_lock(runtime->interpreters.mutex, WAIT_LOCK);
+    runtime->interpreters.allow_new = 0;
+    PyThread_release_lock(runtime->interpreters.mutex);
+    PyInterpreterState *curr_interp = PyInterpreterState_Head();
+    PyInterpreterState *next_interp;
+    int64_t num_destroyed = 0;
+    while (curr_interp != NULL) {
+        next_interp = PyInterpreterState_Next(curr_interp);
+        if (curr_interp != interp) {
+            PyThreadState_Swap(curr_interp->tstate_head);
+            Py_EndInterpreter(curr_interp->tstate_head);
+            num_destroyed++;
+        }
+        curr_interp = next_interp;
+    }
+    PyThreadState_Swap(tstate);
+
+    if (num_destroyed > 0) {
+        /* Sub-interpreters were still running, but should have be finalized
+         * before finalizing the runtime.
+         */
+        if (PyErr_ResourceWarning(NULL, 1,
+                                  "extra %zd interpreters", num_destroyed)) {
+            _PyErr_WriteUnraisableMsg("in PyFinalizeEx", NULL);
+        }
+    }
 
     // Wrap up existing "threading"-module-created, non-daemon threads.
     wait_for_thread_shutdown(tstate);
@@ -1668,13 +1707,13 @@ Py_FinalizeEx(void)
     /* Copy the core config, PyInterpreterState_Delete() free
        the core config memory */
 #ifdef Py_REF_DEBUG
-    int show_ref_count = tstate->interp->config.show_ref_count;
+    int show_ref_count = interp->config.show_ref_count;
 #endif
 #ifdef Py_TRACE_REFS
-    int dump_refs = tstate->interp->config.dump_refs;
+    int dump_refs = interp->config.dump_refs;
 #endif
 #ifdef WITH_PYMALLOC
-    int malloc_stats = tstate->interp->config.malloc_stats;
+    int malloc_stats = interp->config.malloc_stats;
 #endif
 
     /* Remaining daemon threads will automatically exit
@@ -1833,8 +1872,10 @@ new_interpreter(PyThreadState **tstate_p, int isolated_subinterpreter)
     }
     _PyRuntimeState *runtime = &_PyRuntime;
 
-    if (!runtime->initialized) {
-        return _PyStatus_ERR("Py_Initialize must be called first");
+    if (!runtime->interpreters.allow_new) {
+        return _PyStatus_ERR(
+            "New interpreters cannot currently be created - Py_Initialize must "
+            "be called first, and Py_Finalize must not have been called");
     }
 
     /* Issue #10915, #15751: The GIL API doesn't work with multiple
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		:func:`Py_FinalizeEx()` now implicitly cleans up subinterpreters, as the C API documentation suggests.