diff --git a/docs/changelog/1897.bugfix.rst b/docs/changelog/1897.bugfix.rst new file mode 100644 index 000000000..59ff4268f --- /dev/null +++ b/docs/changelog/1897.bugfix.rst @@ -0,0 +1 @@ +No longer preimport threading to fix support for `gpython `_ and `gevent `_ - by :user:`navytux`. diff --git a/src/virtualenv/create/via_global_ref/_virtualenv.py b/src/virtualenv/create/via_global_ref/_virtualenv.py index b399da4cc..31f9b81b2 100644 --- a/src/virtualenv/create/via_global_ref/_virtualenv.py +++ b/src/virtualenv/create/via_global_ref/_virtualenv.py @@ -39,18 +39,33 @@ def parse_config_files(self, *args, **kwargs): # https://docs.python.org/3/library/importlib.html#setting-up-an-importer from importlib.abc import MetaPathFinder from importlib.util import find_spec - from threading import Lock from functools import partial class _Finder(MetaPathFinder): """A meta path finder that allows patching the imported distutils modules""" fullname = None - lock = Lock() + + # lock[0] is threading.Lock(), but initialized lazily to avoid importing threading very early at startup, + # because there are gevent-based applications that need to be first to import threading by themselves. + # See https://github.com/pypa/virtualenv/issues/1895 for details. + lock = [] def find_spec(self, fullname, path, target=None): if fullname in _DISTUTILS_PATCH and self.fullname is None: - with self.lock: + # initialize lock[0] lazily + if len(self.lock) == 0: + import threading + + lock = threading.Lock() + # there is possibility that two threads T1 and T2 are simultaneously running into find_spec, + # observing .lock as empty, and further going into hereby initialization. However due to the GIL, + # list.append() operation is atomic and this way only one of the threads will "win" to put the lock + # - that every thread will use - into .lock[0]. + # https://docs.python.org/3/faq/library.html#what-kinds-of-global-value-mutation-are-thread-safe + self.lock.append(lock) + + with self.lock[0]: self.fullname = fullname try: spec = find_spec(fullname, path) diff --git a/tests/conftest.py b/tests/conftest.py index e69e4d72d..b2ddf03d9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -188,11 +188,11 @@ def check_os_environ_stable(): @pytest.fixture(autouse=True) -def coverage_env(monkeypatch, link): +def coverage_env(monkeypatch, link, request): """ Enable coverage report collection on the created virtual environments by injecting the coverage project """ - if COVERAGE_RUN: + if COVERAGE_RUN and "no_coverage" not in request.fixturenames: # we inject right after creation, we cannot collect coverage on site.py - used for helper scripts, such as debug from virtualenv import run @@ -230,6 +230,12 @@ def finish(): yield finish +# no_coverage tells coverage_env to disable coverage injection for no_coverage user. +@pytest.fixture +def no_coverage(): + pass + + class EnableCoverage(object): _COV_FILE = Path(coverage.__file__) _ROOT_COV_FILES_AND_FOLDERS = [i for i in _COV_FILE.parents[1].iterdir() if i.name.startswith("coverage")] diff --git a/tests/unit/create/test_creator.py b/tests/unit/create/test_creator.py index 5a24df6dd..10090c750 100644 --- a/tests/unit/create/test_creator.py +++ b/tests/unit/create/test_creator.py @@ -557,3 +557,19 @@ def test_zip_importer_can_import_setuptools(tmp_path): env = os.environ.copy() env[str("PYTHONPATH")] = str(zip_path) subprocess.check_call([str(result.creator.exe), "-c", "from setuptools.dist import Distribution"], env=env) + + +# verify that python in created virtualenv does not preimport threading. +# https://github.com/pypa/virtualenv/issues/1895 +# +# coverage is disabled, because when coverage is active, it imports threading in default mode. +@pytest.mark.xfail( + IS_PYPY and PY3 and sys.platform.startswith("darwin"), reason="https://foss.heptapod.net/pypy/pypy/-/issues/3269", +) +def test_no_preimport_threading(tmp_path, no_coverage): + session = cli_run([ensure_text(str(tmp_path))]) + out = subprocess.check_output( + [str(session.creator.exe), "-c", r"import sys; print('\n'.join(sorted(sys.modules)))"], universal_newlines=True, + ) + imported = set(out.splitlines()) + assert "threading" not in imported