Skip to content

Conversation

@colesbury
Copy link
Contributor

@colesbury colesbury commented Feb 14, 2025

Entries may be added or removed from sys.meta_path concurrently. For example, setuptools temporarily adds and removes the distutils finder from the beginning of the list. The local copy ensures that we don't skip over any entries.

Some packages modify sys.modules during import. For example, collections inserts the entry for collections.abc into sys.modules during import. We need to ensure that we re-check sys.modules after the parent module is fully initialized.

Entries may be added or removed from `sys.meta_path` concurrently. For
example, setuptools temporarily adds and removes the `distutils`
finder from the beginning of the list. The local copy ensures that we
don't skip over any entries.
@eendebakpt
Copy link
Contributor

This does not resolve the issue for me, although it is more difficult to trigger now. It is a bit hard to determine what is going wrong as adding simple print statements to importlib._bootstrap.py causes the compilation to fail (something related to the deep freezing or the printing not being available during bootstrap).

The error I get is now always in collections.abc. I suspect that there is a race with two threads are concurrently loading collections and collections.abc. Traceback is here:

Exception in thread Thread-7 (work):
Traceback (most recent call last):
  File "c:\projects\misc\cpython\Lib\threading.py", line 1054, in _bootstrap_inner
    self.run()
    ~~~~~~~~^^
  File "c:\projects\misc\cpython\Lib\threading.py", line 996, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\projects\tmp\ft_import.py", line 21, in work
    mod = import_module(m)
  File "c:\projects\misc\cpython\Lib\importlib\__init__.py", line 88, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1389, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1362, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1326, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'collections.abc'

A minimal reproducer (no exeternal packages involved now):

import sys
from threading import Thread, Barrier
from importlib import import_module
import time


number_of_threads = 2
barrier = Barrier(number_of_threads)

pmods = []

def work(ii):
    # Method doing the actual import
    barrier.wait()
    while True:
        try:
            m = pmods.pop()
            mod = import_module(m)
        except IndexError:
            return


worker_threads = []
for ii in range(number_of_threads):
    worker_threads.append(Thread(target=work, args=[ii]))


def parallel_import(modules: list[str]):
    global pmods
    pmods += modules
    for t in worker_threads:
        t.start()
    for t in worker_threads:
        t.join()

mods = [
    "collections.abc",
    "collections",
]

parallel_import(mods)
print("parallel_import complete")

When running the minimal example with a batch file 80 times, the issue is triggered with very high probability.

@colesbury colesbury changed the title gh-130094: Fix race condition in _find_spec gh-130094: Fix race conditions in _find_spec Feb 14, 2025
@colesbury
Copy link
Contributor Author

@eendebakpt - I fixed another race condition. Would you please try again?

@colesbury colesbury changed the title gh-130094: Fix race conditions in _find_spec gh-130094: Fix race conditions in importlib Feb 14, 2025
@eendebakpt
Copy link
Contributor

@colesbury With the latest commits I see no problems any more.

I created a unit test for this issue based on importlib.reload in

3dc2bee

The test works, but is a bit slow. If I reduce the number of iterations (so that total running time is less than 0.1), odds of triggering the problem are rather low.

@colesbury colesbury merged commit 857bdba into python:main Feb 18, 2025
39 checks passed
@colesbury colesbury deleted the gh-130094-sys-meta-path branch February 18, 2025 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants