| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent e7f5ffa commit 78fed07
4 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1280,6 +1280,14 @@ def _find_and_load(name, import_): | |||
| 1280 | 1280 | # NOTE: because of this, initializing must be set *before* | |
| 1281 | 1281 | # putting the new module in sys.modules. | |
| 1282 | 1282 | _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_) | ||
| 1283 | 1291 | ||
| 1284 | 1292 | if module is None: | |
| 1285 | 1293 | message = f'import of {name} halted; None in sys.modules' | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -259,6 +259,71 @@ def test_multiprocessing_pool_circular_import(self, size): | |||
| 259 | 259 | 'partial', 'pool_in_threads.py') | |
| 260 | 260 | script_helper.assert_python_ok(fn) | |
| 261 | 261 | ||
| 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 | + | ||
| 262 | 327 | ||
| 263 | 328 | def setUpModule(): | |
| 264 | 329 | thread_info = threading_helper.threading_setup() | |
| Original file line number | Diff line number | Diff 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. | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -301,6 +301,18 @@ PyImport_GetModule(PyObject *name) | |||
| 301 | 301 | remove_importlib_frames(tstate); | |
| 302 | 302 | return NULL; | |
| 303 | 303 | } | |
| 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); | ||
| 304 | 316 | } | |
| 305 | 317 | return mod; | |
| 306 | 318 | } | |
@@ -3882,6 +3894,24 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals, | |||
| 3882 | 3894 | if (import_ensure_initialized(tstate->interp, mod, abs_name) < 0) { | |
| 3883 | 3895 | goto error; | |
| 3884 | 3896 | } | |
| 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 | + } | ||
| 3885 | 3915 | } | |
| 3886 | 3916 | else { | |
| 3887 | 3917 | Py_XDECREF(mod); | |
| Back | FazBrowse Home | New Git URL |
0 commit comments