diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py index 499da1e04..9d911e1dc 100644 --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -1375,6 +1375,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/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index 95ce14b2c..6a828ae75 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -946,7 +946,7 @@ class FileLoader: def get_data(self, path): """Return the data from path as raw bytes.""" - if isinstance(self, (SourceLoader, ExtensionFileLoader)): + if isinstance(self, (SourceLoader, SourcelessFileLoader, ExtensionFileLoader)): with _io.open_code(str(path)) as file: return file.read() else: diff --git a/Lib/test/test_importlib/frozen/test_loader.py b/Lib/test/test_importlib/frozen/test_loader.py index 0824af53e..7b5b791be 100644 --- a/Lib/test/test_importlib/frozen/test_loader.py +++ b/Lib/test/test_importlib/frozen/test_loader.py @@ -5,7 +5,6 @@ machinery = util.import_importlib('importlib.machinery') from test.support import captured_stdout, import_helper, STDLIB_DIR import contextlib import os.path -import sys import types import unittest import warnings @@ -76,7 +75,7 @@ class ExecModuleTests(abc.LoaderTests): self.assertHasAttr(module, '__spec__') self.assertEqual(module.__spec__.loader_state.origname, name) - @unittest.skipIf(sys.platform == "win32", "TODO: RUSTPYTHON") + @unittest.skipIf(__import__("sys").platform == "win32", "TODO: RUSTPYTHON") def test_package(self): name = '__phello__' module, output = self.exec_module(name) @@ -147,7 +146,7 @@ class InspectLoaderTests: result = self.machinery.FrozenImporter.get_source('__hello__') self.assertIsNone(result) - @unittest.skipIf(sys.platform == "win32", "TODO: RUSTPYTHON") + @unittest.skipIf(__import__("sys").platform == "win32", "TODO: RUSTPYTHON") def test_is_package(self): # Should be able to tell what is a package. test_for = (('__hello__', False), ('__phello__', True), diff --git a/Lib/test/test_importlib/resources/test_resource.py b/Lib/test/test_importlib/resources/test_resource.py index 83fc2ef4e..fcede14b8 100644 --- a/Lib/test/test_importlib/resources/test_resource.py +++ b/Lib/test/test_importlib/resources/test_resource.py @@ -1,4 +1,3 @@ -import os import unittest from . import util diff --git a/Lib/test/test_importlib/test_locks.py b/Lib/test/test_importlib/test_locks.py index b1f5f9d6c..d42f623b3 100644 --- a/Lib/test/test_importlib/test_locks.py +++ b/Lib/test/test_importlib/test_locks.py @@ -155,8 +155,8 @@ class LifetimeTests: # TODO: RUSTPYTHON; dead weakref module locks not cleaned up in frozen bootstrap Frozen_LifetimeTests.test_all_locks = unittest.skip("TODO: RUSTPYTHON")( - Frozen_LifetimeTests.test_all_locks) - + Frozen_LifetimeTests.test_all_locks +) def setUpModule(): thread_info = threading_helper.threading_setup() diff --git a/Lib/test/test_importlib/test_threaded_import.py b/Lib/test/test_importlib/test_threaded_import.py index c8d156f7f..befee06f4 100644 --- a/Lib/test/test_importlib/test_threaded_import.py +++ b/Lib/test/test_importlib/test_threaded_import.py @@ -263,6 +263,72 @@ class ThreadedImportTests(unittest.TestCase): 'partial', 'pool_in_threads.py') script_helper.assert_python_ok(fn) + @unittest.expectedFailure # TODO: RUSTPYTHON + 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/Lib/test/test_importlib/util.py b/Lib/test/test_importlib/util.py index 85e7ffcb6..2eaefd687 100644 --- a/Lib/test/test_importlib/util.py +++ b/Lib/test/test_importlib/util.py @@ -15,10 +15,10 @@ import sys import tempfile import types -try: - _testsinglephase = import_helper.import_module("_testsinglephase") +try: # TODO: RUSTPYTHON + import_helper.import_module("_testmultiphase") except unittest.SkipTest: - _testsinglephase = None # TODO: RUSTPYTHON + _testmultiphase = None BUILTINS = types.SimpleNamespace()