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

Don't preimport threading early #1897

Merged
merged 5 commits into from Jul 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog/1897.bugfix.rst
@@ -0,0 +1 @@
No longer preimport threading to fix support for `gpython <https://pypi.org/project/pygolang/#gpython>`_ and `gevent <https://www.gevent.org/>`_ - by :user:`navytux`.
21 changes: 18 additions & 3 deletions src/virtualenv/create/via_global_ref/_virtualenv.py
Expand Up @@ -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)
Expand Down
10 changes: 8 additions & 2 deletions tests/conftest.py
Expand Up @@ -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

Expand Down Expand Up @@ -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")]
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/create/test_creator.py
Expand Up @@ -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