Skip to content

Commit 78fed07

Browse files
gpsheadclaude
andcommitted
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 <noreply@anthropic.com>
1 parent e7f5ffa commit 78fed07

File tree

4 files changed

+105
-0
lines changed

4 files changed

+105
-0
lines changed

Lib/importlib/_bootstrap.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1280,6 +1280,14 @@ def _find_and_load(name, import_):
12801280
# NOTE: because of this, initializing must be set *before*
12811281
# putting the new module in sys.modules.
12821282
_lock_unlock_module(name)
1283+
else:
1284+
# Verify the module is still in sys.modules. Another thread may have
1285+
# removed it (due to import failure) between our sys.modules.get()
1286+
# above and the _initializing check. If removed, we retry the import
1287+
# to preserve normal semantics: the caller gets the exception from
1288+
# the actual import failure rather than a synthetic error.
1289+
if sys.modules.get(name) is not module:
1290+
return _find_and_load(name, import_)
12831291

12841292
if module is None:
12851293
message = f'import of {name} halted; None in sys.modules'

Lib/test/test_importlib/test_threaded_import.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,71 @@ def test_multiprocessing_pool_circular_import(self, size):
259259
'partial', 'pool_in_threads.py')
260260
script_helper.assert_python_ok(fn)
261261

262+
def test_import_failure_race_condition(self):
263+
# Regression test for race condition where a thread could receive
264+
# a partially-initialized module when another thread's import fails.
265+
# The race occurs when:
266+
# 1. Thread 1 starts importing, adds module to sys.modules
267+
# 2. Thread 2 sees the module in sys.modules
268+
# 3. Thread 1's import fails, removes module from sys.modules
269+
# 4. Thread 2 should NOT return the stale module reference
270+
os.mkdir(TESTFN)
271+
self.addCleanup(shutil.rmtree, TESTFN)
272+
sys.path.insert(0, TESTFN)
273+
self.addCleanup(sys.path.remove, TESTFN)
274+
275+
# Create a module that partially initializes then fails
276+
modname = 'failing_import_module'
277+
with open(os.path.join(TESTFN, modname + '.py'), 'w') as f:
278+
f.write('''
279+
import time
280+
PARTIAL_ATTR = 'initialized'
281+
time.sleep(0.05) # Widen race window
282+
raise RuntimeError("Intentional import failure")
283+
''')
284+
self.addCleanup(forget, modname)
285+
importlib.invalidate_caches()
286+
287+
errors = []
288+
results = []
289+
290+
def do_import(delay=0):
291+
time.sleep(delay)
292+
try:
293+
mod = __import__(modname)
294+
# If we got a module, verify it's in sys.modules
295+
if modname not in sys.modules:
296+
errors.append(
297+
f"Got module {mod!r} but {modname!r} not in sys.modules"
298+
)
299+
elif sys.modules[modname] is not mod:
300+
errors.append(
301+
f"Got different module than sys.modules[{modname!r}]"
302+
)
303+
else:
304+
results.append(('success', mod))
305+
except RuntimeError:
306+
results.append(('RuntimeError',))
307+
except Exception as e:
308+
errors.append(f"Unexpected exception: {e}")
309+
310+
# Run multiple iterations to increase chance of hitting the race
311+
for _ in range(10):
312+
errors.clear()
313+
results.clear()
314+
if modname in sys.modules:
315+
del sys.modules[modname]
316+
317+
t1 = threading.Thread(target=do_import, args=(0,))
318+
t2 = threading.Thread(target=do_import, args=(0.01,))
319+
t1.start()
320+
t2.start()
321+
t1.join()
322+
t2.join()
323+
324+
# Neither thread should have errors about stale modules
325+
self.assertEqual(errors, [], f"Race condition detected: {errors}")
326+
262327

263328
def setUpModule():
264329
thread_info = threading_helper.threading_setup()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix race condition in import system where a thread could receive a stale
2+
module reference when another thread's import fails.

Python/import.c

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,18 @@ PyImport_GetModule(PyObject *name)
301301
remove_importlib_frames(tstate);
302302
return NULL;
303303
}
304+
/* Verify the module is still in sys.modules. Another thread may have
305+
removed it (due to import failure) between our import_get_module()
306+
call and the _initializing check in import_ensure_initialized().
307+
Unlike the import path, we return NULL here since this function
308+
only retrieves existing modules and doesn't trigger new imports. */
309+
PyObject *mod_check = import_get_module(tstate, name);
310+
if (mod_check != mod) {
311+
Py_XDECREF(mod_check);
312+
Py_DECREF(mod);
313+
return NULL;
314+
}
315+
Py_DECREF(mod_check);
304316
}
305317
return mod;
306318
}
@@ -3882,6 +3894,24 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals,
38823894
if (import_ensure_initialized(tstate->interp, mod, abs_name) < 0) {
38833895
goto error;
38843896
}
3897+
/* Verify the module is still in sys.modules. Another thread may have
3898+
removed it (due to import failure) between our import_get_module()
3899+
call and the _initializing check in import_ensure_initialized().
3900+
If removed, we retry the import to preserve normal semantics: the
3901+
caller gets the exception from the actual import failure rather
3902+
than a synthetic error. */
3903+
PyObject *mod_check = import_get_module(tstate, abs_name);
3904+
if (mod_check != mod) {
3905+
Py_XDECREF(mod_check);
3906+
Py_DECREF(mod);
3907+
mod = import_find_and_load(tstate, abs_name);
3908+
if (mod == NULL) {
3909+
goto error;
3910+
}
3911+
}
3912+
else {
3913+
Py_DECREF(mod_check);
3914+
}
38853915
}
38863916
else {
38873917
Py_XDECREF(mod);

0 commit comments

Comments
 (0)