Review comments from Sam, plus some cleanup

swtaarrs · May 3, 2024 · 0fd1967 · 0fd1967
1 parent fecf4bd
commit 0fd1967
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 19 deletions.
diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h
@@ -142,7 +142,11 @@ extern void _PyEval_ReleaseLock(PyInterpreterState *, PyThreadState *);
 //
 // The enabled state of the GIL will not change while one or more threads are
 // attached.
-extern int _PyEval_IsGILEnabled(PyThreadState *tstate);
+static inline int
+_PyEval_IsGILEnabled(PyThreadState *tstate)
+{
+    return tstate->interp->ceval.gil->enabled != 0;
+}
 
 // Enable or disable the GIL used by the interpreter that owns tstate, which
 // must be the current thread. This may affect other interpreters, if the GIL

diff --git a/Include/internal/pycore_import.h b/Include/internal/pycore_import.h
@@ -213,8 +213,11 @@ extern int _PyImport_CheckSubinterpIncompatibleExtensionAllowed(
 PyAPI_FUNC(int) _PyImport_ClearExtension(PyObject *name, PyObject *filename);
 
 #ifdef Py_GIL_DISABLED
+// Store an association between module and gil (which should be one of the
+// values for the Py_mod_gil module slot) in the current interpreter.
+//
+// Only for use on modules with md_def->m_size == -1.
 extern int _PyImport_SetModuleGIL(PyObject *module, void *gil);
-extern void *_PyImport_GetModuleDefGIL(PyModuleDef *def);
 
 // Assuming that the GIL is enabled from a call to
 // _PyEval_EnableGILTransient(), resolve the transient request depending on the

diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c
@@ -1033,17 +1033,11 @@ _PyEval_InitState(PyInterpreterState *interp)
 }
 
 #ifdef Py_GIL_DISABLED
-int
-_PyEval_IsGILEnabled(PyThreadState *tstate)
-{
-    return tstate->interp->ceval.gil->enabled != 0;
-}
-
 int
 _PyEval_EnableGILTransient(PyThreadState *tstate)
 {
-    if (_PyInterpreterState_GetConfig(tstate->interp)->enable_gil !=
-        _PyConfig_GIL_DEFAULT) {
+    const PyConfig *config = _PyInterpreterState_GetConfig(tstate->interp);
+    if (config->enable_gil != _PyConfig_GIL_DEFAULT) {
         return 0;
     }
     struct _gil_runtime_state *gil = tstate->interp->ceval.gil;
@@ -1094,8 +1088,8 @@ _PyEval_EnableGILTransient(PyThreadState *tstate)
 int
 _PyEval_EnableGILPermanent(PyThreadState *tstate)
 {
-    if (_PyInterpreterState_GetConfig(tstate->interp)->enable_gil !=
-        _PyConfig_GIL_DEFAULT) {
+    const PyConfig *config = _PyInterpreterState_GetConfig(tstate->interp);
+    if (config->enable_gil != _PyConfig_GIL_DEFAULT) {
         return 0;
     }
 
@@ -1114,8 +1108,8 @@ _PyEval_EnableGILPermanent(PyThreadState *tstate)
 int
 _PyEval_DisableGIL(PyThreadState *tstate)
 {
-    if (_PyInterpreterState_GetConfig(tstate->interp)->enable_gil !=
-        _PyConfig_GIL_DEFAULT) {
+    const PyConfig *config = _PyInterpreterState_GetConfig(tstate->interp);
+    if (config->enable_gil != _PyConfig_GIL_DEFAULT) {
         return 0;
     }
 

diff --git a/Python/import.c b/Python/import.c
@@ -508,9 +508,7 @@ _PyImport_SetModuleGIL(PyObject *module, void *gil)
     PyInterpreterState *interp = _PyInterpreterState_GET();
     assert(PyModule_Check(module));
     PyModuleDef *def = ((PyModuleObject *)module)->md_def;
-    if (def->m_size != -1) {
-        return 0;
-    }
+    assert(def->m_size == -1);
 
     Py_ssize_t index = def->m_base.m_index;
     assert(index > 0);

diff --git a/Python/importdl.c b/Python/importdl.c
@@ -478,7 +478,11 @@ _PyImport_RunModInitFunc(PyModInitFunction p0,
             // This module may be recreated (without running its init function)
             // in reload_singlephase_extension(), so remember its GIL slot
             // here.
-            _PyImport_SetModuleGIL(m, ((PyModuleObject *)m)->md_gil);
+            if (_PyImport_SetModuleGIL(m, ((PyModuleObject *)m)->md_gil) < 0) {
+                _Py_ext_module_loader_result_set_error(
+                        &res, _Py_ext_module_loader_result_EXCEPTION);
+                goto error;
+            }
         }
 #endif
     }

diff --git a/Python/pystate.c b/Python/pystate.c
@@ -2071,7 +2071,7 @@ _PyThreadState_Attach(PyThreadState *tstate)
         }
 
 #ifdef Py_GIL_DISABLED
-        if (!!tstate->interp->ceval.gil->enabled != acquired_gil) {
+        if (_PyEval_IsGILEnabled(tstate) != acquired_gil) {
             // The GIL was enabled between our call to _PyEval_AcquireLock()
             // and when we attached (the GIL can't go from enabled to disabled
             // here because only a thread holding the GIL can disable