From cf6e7c5e551b3513817d6a77ba88253dc8473298 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 9 Mar 2023 09:46:21 -0700 Subject: [PATCH] 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 --- Include/cpython/import.h | 4 +- Include/internal/pycore_import.h | 34 +++++---- Include/internal/pycore_runtime_init.h | 10 --- Modules/posixmodule.c | 13 ++-- Python/import.c | 98 ++++++++++++++------------ 5 files changed, 85 insertions(+), 74 deletions(-) diff --git a/Include/cpython/import.h b/Include/cpython/import.h index a58801b47f1..2bca4ade4c4 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 b7ffe01c0c0..69ed6273b7e 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 efc82b43a61..bdecac944df 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 7beb2cee64a..a3d86cbe7a5 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 57d4eea1488..1bf4199e125 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;