pythongh-99113: Add PyInterpreterConfig.own_gil (pythongh-104204)

We also add PyInterpreterState.ceval.own_gil to record if the interpreter actually has its own GIL. Note that for now we don't actually respect own_gil; all interpreters still share the one GIL. However, PyInterpreterState.ceval.own_gil does reflect PyInterpreterConfig.own_gil. That lie is a temporary one that we will fix when the GIL really becomes per-interpreter.
jbower-fb · May 8, 2023 · b057233 · b057233
1 parent c205faa
commit b057233
Show file tree

Hide file tree

Showing 11 changed files with 80 additions and 19 deletions.
diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h
@@ -252,6 +252,7 @@ typedef struct {
     int allow_threads;
     int allow_daemon_threads;
     int check_multi_interp_extensions;
+    int own_gil;
 } PyInterpreterConfig;
 
 #define _PyInterpreterConfig_INIT \
@@ -262,6 +263,7 @@ typedef struct {
         .allow_threads = 1, \
         .allow_daemon_threads = 0, \
         .check_multi_interp_extensions = 1, \
+        .own_gil = 1, \
     }
 
 #define _PyInterpreterConfig_LEGACY_INIT \
@@ -272,6 +274,7 @@ typedef struct {
         .allow_threads = 1, \
         .allow_daemon_threads = 1, \
         .check_multi_interp_extensions = 0, \
+        .own_gil = 0, \
     }
 
 /* --- Helper functions --------------------------------------- */

diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h
@@ -97,7 +97,7 @@ _PyEval_Vector(PyThreadState *tstate,
             PyObject *kwnames);
 
 extern int _PyEval_ThreadsInitialized(void);
-extern PyStatus _PyEval_InitGIL(PyThreadState *tstate);
+extern PyStatus _PyEval_InitGIL(PyThreadState *tstate, int own_gil);
 extern void _PyEval_FiniGIL(PyInterpreterState *interp);
 
 extern void _PyEval_ReleaseLock(PyThreadState *tstate);

diff --git a/Include/internal/pycore_ceval_state.h b/Include/internal/pycore_ceval_state.h
@@ -86,6 +86,7 @@ struct _pending_calls {
 struct _ceval_state {
     int recursion_limit;
     struct _gil_runtime_state *gil;
+    int own_gil;
     /* This single variable consolidates all requests to break out of
        the fast path in the eval loop. */
     _Py_atomic_int eval_breaker;

diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py
@@ -1401,23 +1401,37 @@ def test_configured_settings(self):
         DAEMON_THREADS = 1<<11
         FORK = 1<<15
         EXEC = 1<<16
-
-        features = ['obmalloc', 'fork', 'exec', 'threads', 'daemon_threads',
-                    'extensions']
+        ALL_FLAGS = (OBMALLOC | FORK | EXEC | THREADS | DAEMON_THREADS
+                     | EXTENSIONS);
+
+        features = [
+            'obmalloc',
+            'fork',
+            'exec',
+            'threads',
+            'daemon_threads',
+            'extensions',
+            'own_gil',
+        ]
         kwlist = [f'allow_{n}' for n in features]
         kwlist[0] = 'use_main_obmalloc'
-        kwlist[-1] = 'check_multi_interp_extensions'
+        kwlist[-2] = 'check_multi_interp_extensions'
+        kwlist[-1] = 'own_gil'
 
         # expected to work
         for config, expected in {
-            (True, True, True, True, True, True):
-                OBMALLOC | FORK | EXEC | THREADS | DAEMON_THREADS | EXTENSIONS,
-            (True, False, False, False, False, False): OBMALLOC,
-            (False, False, False, True, False, True): THREADS | EXTENSIONS,
+            (True, True, True, True, True, True, True):
+                (ALL_FLAGS, True),
+            (True, False, False, False, False, False, False):
+                (OBMALLOC, False),
+            (False, False, False, True, False, True, False):
+                (THREADS | EXTENSIONS, False),
         }.items():
             kwargs = dict(zip(kwlist, config))
+            exp_flags, exp_gil = expected
             expected = {
-                'feature_flags': expected,
+                'feature_flags': exp_flags,
+                'own_gil': exp_gil,
             }
             with self.subTest(config):
                 r, w = os.pipe()
@@ -1437,7 +1451,7 @@ def test_configured_settings(self):
 
         # expected to fail
         for config in [
-            (False, False, False, False, False, False),
+            (False, False, False, False, False, False, False),
         ]:
             kwargs = dict(zip(kwlist, config))
             with self.subTest(config):
@@ -1473,6 +1487,7 @@ def test_overridden_setting_extensions_subinterp_check(self):
             'allow_exec': True,
             'allow_threads': True,
             'allow_daemon_threads': True,
+            'own_gil': False,
         }
 
         def check(enabled, override):
@@ -1483,6 +1498,7 @@ def check(enabled, override):
             flags = BASE_FLAGS | EXTENSIONS if enabled else BASE_FLAGS
             settings = {
                 'feature_flags': flags,
+                'own_gil': False,
             }
 
             expected = {

diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py
@@ -1666,6 +1666,7 @@ def test_init_main_interpreter_settings(self):
             # All optional features should be enabled.
             'feature_flags':
                 OBMALLOC | FORK | EXEC | THREADS | DAEMON_THREADS,
+            'own_gil': True,
         }
         out, err = self.run_embedded_interpreter(
             'test_init_main_interpreter_settings',

diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py
@@ -1640,6 +1640,7 @@ class SubinterpImportTests(unittest.TestCase):
     )
     ISOLATED = dict(
         use_main_obmalloc=False,
+        own_gil=True,
     )
     NOT_ISOLATED = {k: not v for k, v in ISOLATED.items()}
 

diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
@@ -1349,6 +1349,7 @@ def func():
                 allow_threads={allowed},
                 allow_daemon_threads={daemon_allowed},
                 check_multi_interp_extensions=False,
+                own_gil=False,
             )
             """)
         with test.support.SuppressCrashReport():

diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
@@ -1488,6 +1488,7 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs)
     int allow_threads = -1;
     int allow_daemon_threads = -1;
     int check_multi_interp_extensions = -1;
+    int own_gil = -1;
     int r;
     PyThreadState *substate, *mainstate;
     /* only initialise 'cflags.cf_flags' to test backwards compatibility */
@@ -1500,13 +1501,15 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs)
                              "allow_threads",
                              "allow_daemon_threads",
                              "check_multi_interp_extensions",
+                             "own_gil",
                              NULL};
     if (!PyArg_ParseTupleAndKeywords(args, kwargs,
-                    "s$pppppp:run_in_subinterp_with_config", kwlist,
+                    "s$ppppppp:run_in_subinterp_with_config", kwlist,
                     &code, &use_main_obmalloc,
                     &allow_fork, &allow_exec,
                     &allow_threads, &allow_daemon_threads,
-                    &check_multi_interp_extensions)) {
+                    &check_multi_interp_extensions,
+                    &own_gil)) {
         return NULL;
     }
     if (use_main_obmalloc < 0) {
@@ -1525,6 +1528,10 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs)
         PyErr_SetString(PyExc_ValueError, "missing allow_threads");
         return NULL;
     }
+    if (own_gil < 0) {
+        PyErr_SetString(PyExc_ValueError, "missing own_gil");
+        return NULL;
+    }
     if (allow_daemon_threads < 0) {
         PyErr_SetString(PyExc_ValueError, "missing allow_daemon_threads");
         return NULL;
@@ -1545,6 +1552,7 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs)
         .allow_threads = allow_threads,
         .allow_daemon_threads = allow_daemon_threads,
         .check_multi_interp_extensions = check_multi_interp_extensions,
+        .own_gil = own_gil,
     };
     PyStatus status = Py_NewInterpreterFromConfig(&substate, &config);
     if (PyStatus_Exception(status)) {

diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c
@@ -729,6 +729,13 @@ get_interp_settings(PyObject *self, PyObject *args)
         return NULL;
     }
 
+    /* "own GIL" */
+    PyObject *own_gil = interp->ceval.own_gil ? Py_True : Py_False;
+    if (PyDict_SetItemString(settings, "own_gil", own_gil) != 0) {
+        Py_DECREF(settings);
+        return NULL;
+    }
+
     return settings;
 }
 

diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c
@@ -500,9 +500,18 @@ PyEval_ThreadsInitialized(void)
 }
 
 PyStatus
-_PyEval_InitGIL(PyThreadState *tstate)
+_PyEval_InitGIL(PyThreadState *tstate, int own_gil)
 {
     assert(tstate->interp->ceval.gil == NULL);
+    if (!own_gil) {
+        PyInterpreterState *main_interp = _PyInterpreterState_Main();
+        assert(tstate->interp != main_interp);
+        struct _gil_runtime_state *gil = main_interp->ceval.gil;
+        assert(gil_created(gil));
+        tstate->interp->ceval.gil = gil;
+        tstate->interp->ceval.own_gil = 0;
+        return _PyStatus_OK();
+    }
 
     /* XXX per-interpreter GIL */
     struct _gil_runtime_state *gil = &tstate->interp->runtime->ceval.gil;
@@ -512,15 +521,19 @@ _PyEval_InitGIL(PyThreadState *tstate)
            and destroy it. */
         assert(gil_created(gil));
         tstate->interp->ceval.gil = gil;
+        // XXX For now we lie.
+        tstate->interp->ceval.own_gil = 1;
         return _PyStatus_OK();
     }
+    assert(own_gil);
 
     assert(!gil_created(gil));
 
     PyThread_init_thread();
     create_gil(gil);
     assert(gil_created(gil));
     tstate->interp->ceval.gil = gil;
+    tstate->interp->ceval.own_gil = 1;
     take_gil(tstate);
     return _PyStatus_OK();
 }
@@ -530,6 +543,14 @@ _PyEval_FiniGIL(PyInterpreterState *interp)
 {
     if (interp->ceval.gil == NULL) {
         /* It was already finalized (or hasn't been initialized yet). */
+        assert(!interp->ceval.own_gil);
+        return;
+    }
+    else if (!interp->ceval.own_gil) {
+        PyInterpreterState *main_interp = _PyInterpreterState_Main();
+        assert(interp != main_interp);
+        assert(interp->ceval.gil == main_interp->ceval.gil);
+        interp->ceval.gil = NULL;
         return;
     }
 

diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
@@ -585,7 +585,7 @@ init_interp_settings(PyInterpreterState *interp,
 
 
 static PyStatus
-init_interp_create_gil(PyThreadState *tstate)
+init_interp_create_gil(PyThreadState *tstate, int own_gil)
 {
     PyStatus status;
 
@@ -600,7 +600,7 @@ init_interp_create_gil(PyThreadState *tstate)
     }
 
     /* Create the GIL and take it */
-    status = _PyEval_InitGIL(tstate);
+    status = _PyEval_InitGIL(tstate, own_gil);
     if (_PyStatus_EXCEPTION(status)) {
         return status;
     }
@@ -632,7 +632,9 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
         return status;
     }
 
-    const PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT;
+    PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT;
+    // The main interpreter always has its own GIL.
+    config.own_gil = 1;
     status = init_interp_settings(interp, &config);
     if (_PyStatus_EXCEPTION(status)) {
         return status;
@@ -645,7 +647,7 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
     _PyThreadState_Bind(tstate);
     (void) PyThreadState_Swap(tstate);
 
-    status = init_interp_create_gil(tstate);
+    status = init_interp_create_gil(tstate, config.own_gil);
     if (_PyStatus_EXCEPTION(status)) {
         return status;
     }
@@ -2047,7 +2049,7 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
         goto error;
     }
 
-    status = init_interp_create_gil(tstate);
+    status = init_interp_create_gil(tstate, config->own_gil);
     if (_PyStatus_EXCEPTION(status)) {
         goto error;
     }