[Python-checkins] gh-100227: Isolate the Import State to Each Interpreter (gh-101941)

ericsnowcurrently webhook-mailer at python.org
Thu Mar 9 11:46:44 EST 2023


https://github.com/python/cpython/commit/cf6e7c5e551b3513817d6a77ba88253dc8473298
commit: cf6e7c5e551b3513817d6a77ba88253dc8473298
branch: main
author: Eric Snow <ericsnowcurrently at gmail.com>
committer: ericsnowcurrently <ericsnowcurrently at gmail.com>
date: 2023-03-09T09:46:21-07:00
summary:

gh-100227: Isolate the Import State to Each Interpreter (gh-101941)

Specific changes:

* move the import lock to PyInterpreterState
* move the "find_and_load" diagnostic state to PyInterpreterState

Note that the import lock exists to keep multiple imports of the same module in the same interpreter (but in different threads) from stomping on each other.  Independently, we use a distinct global lock to protect globally shared import state, especially related to loaded extension modules.  For now we can rely on the GIL as that lock but with a per-interpreter GIL we'll need a new global lock.

The remaining state in _PyRuntimeState.imports will (probably) continue being global.

https://github.com/python/cpython/issues/100227

files:
M Include/cpython/import.h
M Include/internal/pycore_import.h
M Include/internal/pycore_runtime_init.h
M Modules/posixmodule.c
M Python/import.c

diff --git a/Include/cpython/import.h b/Include/cpython/import.h
index a58801b47f1b..2bca4ade4c4f 100644
--- a/Include/cpython/import.h
+++ b/Include/cpython/import.h
@@ -10,8 +10,8 @@ PyAPI_FUNC(PyObject *) _PyImport_GetModuleId(_Py_Identifier *name);
 PyAPI_FUNC(int) _PyImport_SetModule(PyObject *name, PyObject *module);
 PyAPI_FUNC(int) _PyImport_SetModuleString(const char *name, PyObject* module);
 
-PyAPI_FUNC(void) _PyImport_AcquireLock(void);
-PyAPI_FUNC(int) _PyImport_ReleaseLock(void);
+PyAPI_FUNC(void) _PyImport_AcquireLock(PyInterpreterState *interp);
+PyAPI_FUNC(int) _PyImport_ReleaseLock(PyInterpreterState *interp);
 
 PyAPI_FUNC(int) _PyImport_FixupBuiltin(
     PyObject *mod,
diff --git a/Include/internal/pycore_import.h b/Include/internal/pycore_import.h
index b7ffe01c0c0e..69ed6273b7e6 100644
--- a/Include/internal/pycore_import.h
+++ b/Include/internal/pycore_import.h
@@ -21,17 +21,6 @@ struct _import_runtime_state {
        This is initialized lazily in _PyImport_FixupExtensionObject().
        Modules are added there and looked up in _imp.find_extension(). */
     PyObject *extensions;
-    /* The global import lock. */
-    struct {
-        PyThread_type_lock mutex;
-        unsigned long thread;
-        int level;
-    } lock;
-    struct {
-        int import_level;
-        _PyTime_t accumulated;
-        int header;
-    } find_and_load;
     /* Package context -- the full module name for package imports */
     const char * pkgcontext;
 };
@@ -69,6 +58,18 @@ struct _import_state {
     int dlopenflags;
 #endif
     PyObject *import_func;
+    /* The global import lock. */
+    struct {
+        PyThread_type_lock mutex;
+        unsigned long thread;
+        int level;
+    } lock;
+    /* diagnostic info in PyImport_ImportModuleLevelObject() */
+    struct {
+        int import_level;
+        _PyTime_t accumulated;
+        int header;
+    } find_and_load;
 };
 
 #ifdef HAVE_DLOPEN
@@ -86,8 +87,15 @@ struct _import_state {
 
 #define IMPORTS_INIT \
     { \
-        .override_frozen_modules = 0, \
         DLOPENFLAGS_INIT \
+        .lock = { \
+            .mutex = NULL, \
+            .thread = PYTHREAD_INVALID_THREAD_ID, \
+            .level = 0, \
+        }, \
+        .find_and_load = { \
+            .header = 1, \
+        }, \
     }
 
 extern void _PyImport_ClearCore(PyInterpreterState *interp);
@@ -138,7 +146,7 @@ extern void _PyImport_FiniExternal(PyInterpreterState *interp);
 
 
 #ifdef HAVE_FORK
-extern PyStatus _PyImport_ReInitLock(void);
+extern PyStatus _PyImport_ReInitLock(PyInterpreterState *interp);
 #endif
 
 
diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h
index efc82b43a61d..bdecac944dfd 100644
--- a/Include/internal/pycore_runtime_init.h
+++ b/Include/internal/pycore_runtime_init.h
@@ -40,16 +40,6 @@ extern PyTypeObject _PyExc_MemoryError;
            in accordance with the specification. */ \
         .autoTSSkey = Py_tss_NEEDS_INIT, \
         .parser = _parser_runtime_state_INIT, \
-        .imports = { \
-            .lock = { \
-                .mutex = NULL, \
-                .thread = PYTHREAD_INVALID_THREAD_ID, \
-                .level = 0, \
-            }, \
-            .find_and_load = { \
-                .header = 1, \
-            }, \
-        }, \
         .ceval = { \
             .perf = _PyEval_RUNTIME_PERF_INIT, \
         }, \
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 7beb2cee64a0..a3d86cbe7a57 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -567,18 +567,21 @@ run_at_forkers(PyObject *lst, int reverse)
 void
 PyOS_BeforeFork(void)
 {
-    run_at_forkers(_PyInterpreterState_GET()->before_forkers, 1);
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+    run_at_forkers(interp->before_forkers, 1);
 
-    _PyImport_AcquireLock();
+    _PyImport_AcquireLock(interp);
 }
 
 void
 PyOS_AfterFork_Parent(void)
 {
-    if (_PyImport_ReleaseLock() <= 0)
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+    if (_PyImport_ReleaseLock(interp) <= 0) {
         Py_FatalError("failed releasing import lock after fork");
+    }
 
-    run_at_forkers(_PyInterpreterState_GET()->after_forkers_parent, 0);
+    run_at_forkers(interp->after_forkers_parent, 0);
 }
 
 void
@@ -604,7 +607,7 @@ PyOS_AfterFork_Child(void)
         goto fatal_error;
     }
 
-    status = _PyImport_ReInitLock();
+    status = _PyImport_ReInitLock(tstate->interp);
     if (_PyStatus_EXCEPTION(status)) {
         goto fatal_error;
     }
diff --git a/Python/import.c b/Python/import.c
index 57d4eea14881..1bf4199e125a 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -56,11 +56,6 @@ static struct _inittab *inittab_copy = NULL;
 #define LAST_MODULE_INDEX _PyRuntime.imports.last_module_index
 #define EXTENSIONS _PyRuntime.imports.extensions
 
-#define import_lock _PyRuntime.imports.lock.mutex
-#define import_lock_thread _PyRuntime.imports.lock.thread
-#define import_lock_level _PyRuntime.imports.lock.level
-
-#define FIND_AND_LOAD _PyRuntime.imports.find_and_load
 #define PKGCONTEXT (_PyRuntime.imports.pkgcontext)
 
 
@@ -85,6 +80,16 @@ static struct _inittab *inittab_copy = NULL;
 #define IMPORT_FUNC(interp) \
     (interp)->imports.import_func
 
+#define IMPORT_LOCK(interp) \
+    (interp)->imports.lock.mutex
+#define IMPORT_LOCK_THREAD(interp) \
+    (interp)->imports.lock.thread
+#define IMPORT_LOCK_LEVEL(interp) \
+    (interp)->imports.lock.level
+
+#define FIND_AND_LOAD(interp) \
+    (interp)->imports.find_and_load
+
 
 /*******************/
 /* the import lock */
@@ -95,45 +100,45 @@ static struct _inittab *inittab_copy = NULL;
    These calls are serialized by the global interpreter lock. */
 
 void
-_PyImport_AcquireLock(void)
+_PyImport_AcquireLock(PyInterpreterState *interp)
 {
     unsigned long me = PyThread_get_thread_ident();
     if (me == PYTHREAD_INVALID_THREAD_ID)
         return; /* Too bad */
-    if (import_lock == NULL) {
-        import_lock = PyThread_allocate_lock();
-        if (import_lock == NULL)
+    if (IMPORT_LOCK(interp) == NULL) {
+        IMPORT_LOCK(interp) = PyThread_allocate_lock();
+        if (IMPORT_LOCK(interp) == NULL)
             return;  /* Nothing much we can do. */
     }
-    if (import_lock_thread == me) {
-        import_lock_level++;
+    if (IMPORT_LOCK_THREAD(interp) == me) {
+        IMPORT_LOCK_LEVEL(interp)++;
         return;
     }
-    if (import_lock_thread != PYTHREAD_INVALID_THREAD_ID ||
-        !PyThread_acquire_lock(import_lock, 0))
+    if (IMPORT_LOCK_THREAD(interp) != PYTHREAD_INVALID_THREAD_ID ||
+        !PyThread_acquire_lock(IMPORT_LOCK(interp), 0))
     {
         PyThreadState *tstate = PyEval_SaveThread();
-        PyThread_acquire_lock(import_lock, WAIT_LOCK);
+        PyThread_acquire_lock(IMPORT_LOCK(interp), WAIT_LOCK);
         PyEval_RestoreThread(tstate);
     }
-    assert(import_lock_level == 0);
-    import_lock_thread = me;
-    import_lock_level = 1;
+    assert(IMPORT_LOCK_LEVEL(interp) == 0);
+    IMPORT_LOCK_THREAD(interp) = me;
+    IMPORT_LOCK_LEVEL(interp) = 1;
 }
 
 int
-_PyImport_ReleaseLock(void)
+_PyImport_ReleaseLock(PyInterpreterState *interp)
 {
     unsigned long me = PyThread_get_thread_ident();
-    if (me == PYTHREAD_INVALID_THREAD_ID || import_lock == NULL)
+    if (me == PYTHREAD_INVALID_THREAD_ID || IMPORT_LOCK(interp) == NULL)
         return 0; /* Too bad */
-    if (import_lock_thread != me)
+    if (IMPORT_LOCK_THREAD(interp) != me)
         return -1;
-    import_lock_level--;
-    assert(import_lock_level >= 0);
-    if (import_lock_level == 0) {
-        import_lock_thread = PYTHREAD_INVALID_THREAD_ID;
-        PyThread_release_lock(import_lock);
+    IMPORT_LOCK_LEVEL(interp)--;
+    assert(IMPORT_LOCK_LEVEL(interp) >= 0);
+    if (IMPORT_LOCK_LEVEL(interp) == 0) {
+        IMPORT_LOCK_THREAD(interp) = PYTHREAD_INVALID_THREAD_ID;
+        PyThread_release_lock(IMPORT_LOCK(interp));
     }
     return 1;
 }
@@ -144,23 +149,23 @@ _PyImport_ReleaseLock(void)
    We now acquire the import lock around fork() calls but on some platforms
    (Solaris 9 and earlier? see isue7242) that still left us with problems. */
 PyStatus
-_PyImport_ReInitLock(void)
+_PyImport_ReInitLock(PyInterpreterState *interp)
 {
-    if (import_lock != NULL) {
-        if (_PyThread_at_fork_reinit(&import_lock) < 0) {
+    if (IMPORT_LOCK(interp) != NULL) {
+        if (_PyThread_at_fork_reinit(&IMPORT_LOCK(interp)) < 0) {
             return _PyStatus_ERR("failed to create a new lock");
         }
     }
 
-    if (import_lock_level > 1) {
+    if (IMPORT_LOCK_LEVEL(interp) > 1) {
         /* Forked as a side effect of import */
         unsigned long me = PyThread_get_thread_ident();
-        PyThread_acquire_lock(import_lock, WAIT_LOCK);
-        import_lock_thread = me;
-        import_lock_level--;
+        PyThread_acquire_lock(IMPORT_LOCK(interp), WAIT_LOCK);
+        IMPORT_LOCK_THREAD(interp) = me;
+        IMPORT_LOCK_LEVEL(interp)--;
     } else {
-        import_lock_thread = PYTHREAD_INVALID_THREAD_ID;
-        import_lock_level = 0;
+        IMPORT_LOCK_THREAD(interp) = PYTHREAD_INVALID_THREAD_ID;
+        IMPORT_LOCK_LEVEL(interp) = 0;
     }
     return _PyStatus_OK();
 }
@@ -2506,8 +2511,8 @@ import_find_and_load(PyThreadState *tstate, PyObject *abs_name)
     PyObject *mod = NULL;
     PyInterpreterState *interp = tstate->interp;
     int import_time = _PyInterpreterState_GetConfig(interp)->import_time;
-#define import_level FIND_AND_LOAD.import_level
-#define accumulated FIND_AND_LOAD.accumulated
+#define import_level FIND_AND_LOAD(interp).import_level
+#define accumulated FIND_AND_LOAD(interp).accumulated
 
     _PyTime_t t1 = 0, accumulated_copy = accumulated;
 
@@ -2528,7 +2533,7 @@ import_find_and_load(PyThreadState *tstate, PyObject *abs_name)
      * _PyDict_GetItemIdWithError().
      */
     if (import_time) {
-#define header FIND_AND_LOAD.header
+#define header FIND_AND_LOAD(interp).header
         if (header) {
             fputs("import time: self [us] | cumulative | imported package\n",
                   stderr);
@@ -2867,10 +2872,6 @@ _PyImport_Fini(void)
 {
     /* Destroy the database used by _PyImport_{Fixup,Find}Extension */
     _extensions_cache_clear_all();
-    if (import_lock != NULL) {
-        PyThread_free_lock(import_lock);
-        import_lock = NULL;
-    }
 
     /* Use the same memory allocator as _PyImport_Init(). */
     PyMemAllocatorEx old_alloc;
@@ -2959,6 +2960,11 @@ _PyImport_FiniCore(PyInterpreterState *interp)
         PyErr_WriteUnraisable(NULL);
     }
 
+    if (IMPORT_LOCK(interp) != NULL) {
+        PyThread_free_lock(IMPORT_LOCK(interp));
+        IMPORT_LOCK(interp) = NULL;
+    }
+
     _PyImport_ClearCore(interp);
 }
 
@@ -3090,7 +3096,9 @@ static PyObject *
 _imp_lock_held_impl(PyObject *module)
 /*[clinic end generated code: output=8b89384b5e1963fc input=9b088f9b217d9bdf]*/
 {
-    return PyBool_FromLong(import_lock_thread != PYTHREAD_INVALID_THREAD_ID);
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+    return PyBool_FromLong(
+            IMPORT_LOCK_THREAD(interp) != PYTHREAD_INVALID_THREAD_ID);
 }
 
 /*[clinic input]
@@ -3106,7 +3114,8 @@ static PyObject *
 _imp_acquire_lock_impl(PyObject *module)
 /*[clinic end generated code: output=1aff58cb0ee1b026 input=4a2d4381866d5fdc]*/
 {
-    _PyImport_AcquireLock();
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+    _PyImport_AcquireLock(interp);
     Py_RETURN_NONE;
 }
 
@@ -3122,7 +3131,8 @@ static PyObject *
 _imp_release_lock_impl(PyObject *module)
 /*[clinic end generated code: output=7faab6d0be178b0a input=934fb11516dd778b]*/
 {
-    if (_PyImport_ReleaseLock() < 0) {
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+    if (_PyImport_ReleaseLock(interp) < 0) {
         PyErr_SetString(PyExc_RuntimeError,
                         "not holding the import lock");
         return NULL;



More information about the Python-checkins mailing list