From 6393e9774a2e41ef802b0af75b3aec4bb21c16a5 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 10 Jan 2026 10:52:19 +0000 Subject: [PATCH 1/4] Fix race condition in import system with failing imports Fix a 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 via the fast path 3. Thread 1's import fails, removes module from sys.modules 4. Thread 2 returns a stale module reference not in sys.modules The fix adds verification after the "skip lock" optimization in both Python and C code paths to check if the module is still in sys.modules. If the module was removed (due to import failure), we retry the import so the caller receives the actual exception from the import failure rather than a stale module reference. Changes: - Lib/importlib/_bootstrap.py: Add check after fast path in _find_and_load() - Python/import.c: Add checks in PyImport_GetModule() and PyImport_ImportModuleLevelObject() - Add regression test test_import_failure_race_condition() Co-Authored-By: Claude Opus 4.5 --- Lib/importlib/_bootstrap.py | 8 +++ .../test_importlib/test_threaded_import.py | 65 +++++++++++++++++++ ...-01-10-10-58-36.gh-issue-143650.k8mR4x.rst | 2 + Python/import.c | 30 +++++++++ 4 files changed, 105 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-01-10-10-58-36.gh-issue-143650.k8mR4x.rst 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..f7d7907ff6a087 100644 --- a/Python/import.c +++ b/Python/import.c @@ -301,6 +301,18 @@ PyImport_GetModule(PyObject *name) remove_importlib_frames(tstate); return NULL; } + /* 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(). + Unlike the import path, we return NULL here since this function + only retrieves existing modules and doesn't trigger new imports. */ + PyObject *mod_check = import_get_module(tstate, name); + if (mod_check != mod) { + Py_XDECREF(mod_check); + Py_DECREF(mod); + return NULL; + } + Py_DECREF(mod_check); } return mod; } @@ -3882,6 +3894,24 @@ 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); + 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); From 0a682cd7cc553ad78bb2809d24af790791dd252b Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 10 Jan 2026 11:15:25 +0000 Subject: [PATCH 2/4] shorten comment --- Python/import.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Python/import.c b/Python/import.c index f7d7907ff6a087..185d1cf8c9dcfe 100644 --- a/Python/import.c +++ b/Python/import.c @@ -303,9 +303,7 @@ PyImport_GetModule(PyObject *name) } /* 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(). - Unlike the import path, we return NULL here since this function - only retrieves existing modules and doesn't trigger new imports. */ + 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); From 842c1607ee7fd65af6ca6ca531ef38e925dbd757 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 10 Jan 2026 21:04:46 +0000 Subject: [PATCH 3/4] Address review: handle import_get_module errors in race condition fix Add error checking inside the `if (mod_check != mod)` block to properly handle the case where import_get_module itself fails with an exception. Also refactor PyImport_GetModule to use an error label for cleanup. Co-Authored-By: Claude Opus 4.5 --- Python/import.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/Python/import.c b/Python/import.c index 185d1cf8c9dcfe..7ce9d70fa21f38 100644 --- a/Python/import.c +++ b/Python/import.c @@ -297,9 +297,7 @@ 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) { - Py_DECREF(mod); - remove_importlib_frames(tstate); - return NULL; + 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() @@ -307,12 +305,24 @@ PyImport_GetModule(PyObject *name) 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); 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. @@ -3901,6 +3911,10 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals, PyObject *mod_check = import_get_module(tstate, abs_name); if (mod_check != mod) { Py_XDECREF(mod_check); + if (_PyErr_Occurred(tstate)) { + Py_DECREF(mod); + goto error; + } Py_DECREF(mod); mod = import_find_and_load(tstate, abs_name); if (mod == NULL) { From 102845b8b5f93935c3e6b4ec997797a02e2ed325 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" <68491+gpshead@users.noreply.github.com> Date: Sun, 11 Jan 2026 10:54:41 -0800 Subject: [PATCH 4/4] remove extra decref, oops! (should've rerun buildbots afterall) --- Python/import.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index 7ce9d70fa21f38..fa6770d0d32da9 100644 --- a/Python/import.c +++ b/Python/import.c @@ -3912,7 +3912,6 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals, if (mod_check != mod) { Py_XDECREF(mod_check); if (_PyErr_Occurred(tstate)) { - Py_DECREF(mod); goto error; } Py_DECREF(mod);