Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-130094: Fix race conditions in importlib #130101

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

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
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.

2 participants