diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py index 07d938b18fe727..cfcebb7309803c 100644 --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -1280,6 +1280,14 @@ def _find_and_load(name, import_): # NOTE: because of this, initializing must be set *before* # putting the new module in sys.modules. _lock_unlock_module(name) + else: + # Verify the module is still in sys.modules. Another thread may have + # removed it (due to import failure) between our sys.modules.get() + # above and the _initializing check. If removed, we retry the import + # to preserve normal semantics: the caller gets the exception from + # the actual import failure rather than a synthetic error. + if sys.modules.get(name) is not module: + return _find_and_load(name, import_) if module is None: message = f'import of {name} halted; None in sys.modules' diff --git a/Lib/test/test_importlib/test_threaded_import.py b/Lib/test/test_importlib/test_threaded_import.py index f78dc399720c86..8b793ebf29bcae 100644 --- a/Lib/test/test_importlib/test_threaded_import.py +++ b/Lib/test/test_importlib/test_threaded_import.py @@ -259,6 +259,71 @@ def test_multiprocessing_pool_circular_import(self, size): 'partial', 'pool_in_threads.py') script_helper.assert_python_ok(fn) + def test_import_failure_race_condition(self): + # Regression test for race condition where a thread could receive + # a partially-initialized module when another thread's import fails. + # The race occurs when: + # 1. Thread 1 starts importing, adds module to sys.modules + # 2. Thread 2 sees the module in sys.modules + # 3. Thread 1's import fails, removes module from sys.modules + # 4. Thread 2 should NOT return the stale module reference + os.mkdir(TESTFN) + self.addCleanup(shutil.rmtree, TESTFN) + sys.path.insert(0, TESTFN) + self.addCleanup(sys.path.remove, TESTFN) + + # Create a module that partially initializes then fails + modname = 'failing_import_module' + with open(os.path.join(TESTFN, modname + '.py'), 'w') as f: + f.write(''' +import time +PARTIAL_ATTR = 'initialized' +time.sleep(0.05) # Widen race window +raise RuntimeError("Intentional import failure") +''') + self.addCleanup(forget, modname) + importlib.invalidate_caches() + + errors = [] + results = [] + + def do_import(delay=0): + time.sleep(delay) + try: + mod = __import__(modname) + # If we got a module, verify it's in sys.modules + if modname not in sys.modules: + errors.append( + f"Got module {mod!r} but {modname!r} not in sys.modules" + ) + elif sys.modules[modname] is not mod: + errors.append( + f"Got different module than sys.modules[{modname!r}]" + ) + else: + results.append(('success', mod)) + except RuntimeError: + results.append(('RuntimeError',)) + except Exception as e: + errors.append(f"Unexpected exception: {e}") + + # Run multiple iterations to increase chance of hitting the race + for _ in range(10): + errors.clear() + results.clear() + if modname in sys.modules: + del sys.modules[modname] + + t1 = threading.Thread(target=do_import, args=(0,)) + t2 = threading.Thread(target=do_import, args=(0.01,)) + t1.start() + t2.start() + t1.join() + t2.join() + + # Neither thread should have errors about stale modules + self.assertEqual(errors, [], f"Race condition detected: {errors}") + def setUpModule(): thread_info = threading_helper.threading_setup() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-01-10-10-58-36.gh-issue-143650.k8mR4x.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-01-10-10-58-36.gh-issue-143650.k8mR4x.rst new file mode 100644 index 00000000000000..7bee70a828acfe --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-01-10-10-58-36.gh-issue-143650.k8mR4x.rst @@ -0,0 +1,2 @@ +Fix race condition in :mod:`importlib` where a thread could receive a stale +module reference when another thread's import fails. diff --git a/Python/import.c b/Python/import.c index 466c5868ab7ee8..fa6770d0d32da9 100644 --- a/Python/import.c +++ b/Python/import.c @@ -297,12 +297,32 @@ PyImport_GetModule(PyObject *name) mod = import_get_module(tstate, name); if (mod != NULL && mod != Py_None) { if (import_ensure_initialized(tstate->interp, mod, name) < 0) { + goto error; + } + /* Verify the module is still in sys.modules. Another thread may have + removed it (due to import failure) between our import_get_module() + call and the _initializing check in import_ensure_initialized(). */ + PyObject *mod_check = import_get_module(tstate, name); + if (mod_check != mod) { + Py_XDECREF(mod_check); + if (_PyErr_Occurred(tstate)) { + goto error; + } + /* The module was removed or replaced. Return NULL to report + "not found" rather than trying to keep up with racing + modifications to sys.modules; returning the new value would + require looping to redo the ensure_initialized check. */ Py_DECREF(mod); - remove_importlib_frames(tstate); return NULL; } + Py_DECREF(mod_check); } return mod; + +error: + Py_DECREF(mod); + remove_importlib_frames(tstate); + return NULL; } /* Get the module object corresponding to a module name. @@ -3882,6 +3902,27 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals, if (import_ensure_initialized(tstate->interp, mod, abs_name) < 0) { goto error; } + /* Verify the module is still in sys.modules. Another thread may have + removed it (due to import failure) between our import_get_module() + call and the _initializing check in import_ensure_initialized(). + If removed, we retry the import to preserve normal semantics: the + caller gets the exception from the actual import failure rather + than a synthetic error. */ + PyObject *mod_check = import_get_module(tstate, abs_name); + if (mod_check != mod) { + Py_XDECREF(mod_check); + if (_PyErr_Occurred(tstate)) { + goto error; + } + Py_DECREF(mod); + mod = import_find_and_load(tstate, abs_name); + if (mod == NULL) { + goto error; + } + } + else { + Py_DECREF(mod_check); + } } else { Py_XDECREF(mod);