Fix parallel app-data base image creation (#1870)

Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>

4 files changed, 62 insertions, 7 deletions

diff --git a/docs/changelog/1869.bugfix.rst b/docs/changelog/1869.bugfix.rst
new file mode 100644
index 0000000..d99a017
--- /dev/null
+++ b/docs/changelog/1869.bugfix.rst
@@ -0,0 +1 @@
+Fix that when the ``app-data`` seeders image creation fails the exception is silently ignored. Avoid two virtual environment creations to step on each others toes by using a lock while creating the base images. By :user:`gaborbernat`.
diff --git a/src/virtualenv/seed/embed/via_app_data/via_app_data.py b/src/virtualenv/seed/embed/via_app_data/via_app_data.py
index 779ee18..e595128 100644
--- a/src/virtualenv/seed/embed/via_app_data/via_app_data.py
+++ b/src/virtualenv/seed/embed/via_app_data/via_app_data.py
@@ -2,6 +2,8 @@
 from __future__ import absolute_import, unicode_literals
 
 import logging
+import sys
+import traceback
 from contextlib import contextmanager
 from subprocess import CalledProcessError
 from threading import Lock, Thread
@@ -11,7 +13,9 @@ import six
 from virtualenv.info import fs_supports_symlink
 from virtualenv.seed.embed.base_embed import BaseEmbed
 from virtualenv.seed.wheels import get_wheel
+from virtualenv.util.lock import _CountedFileLock
 from virtualenv.util.path import Path
+from virtualenv.util.six import ensure_text
 
 from .pip_install.copy import CopyPipInstall
 from .pip_install.symlink import SymlinkPipInstall
@@ -42,21 +46,32 @@ class FromAppData(BaseEmbed):
         with self._get_seed_wheels(creator) as name_to_whl:
             pip_version = name_to_whl["pip"].version_tuple if "pip" in name_to_whl else None
             installer_class = self.installer_class(pip_version)
+            exceptions = {}
 
             def _install(name, wheel):
-                logging.debug("install %s from wheel %s via %s", name, wheel, installer_class.__name__)
-                key = Path(installer_class.__name__) / wheel.path.stem
-                wheel_img = self.app_data.wheel_image(creator.interpreter.version_release_str, key)
-                installer = installer_class(wheel.path, creator, wheel_img)
-                if not installer.has_image():
-                    installer.build_image()
-                installer.install(creator.interpreter.version_info)
+                try:
+                    logging.debug("install %s from wheel %s via %s", name, wheel, installer_class.__name__)
+                    key = Path(installer_class.__name__) / wheel.path.stem
+                    wheel_img = self.app_data.wheel_image(creator.interpreter.version_release_str, key)
+                    installer = installer_class(wheel.path, creator, wheel_img)
+                    with _CountedFileLock(ensure_text(str(wheel_img.parent / "{}.lock".format(wheel_img.name)))):
+                        if not installer.has_image():
+                            installer.build_image()
+                    installer.install(creator.interpreter.version_info)
+                except Exception:  # noqa
+                    exceptions[name] = sys.exc_info()
 
             threads = list(Thread(target=_install, args=(n, w)) for n, w in name_to_whl.items())
             for thread in threads:
                 thread.start()
             for thread in threads:
                 thread.join()
+            if exceptions:
+                messages = ["failed to build image {} because:".format(", ".join(exceptions.keys()))]
+                for value in exceptions.values():
+                    exc_type, exc_value, exc_traceback = value
+                    messages.append("".join(traceback.format_exception(exc_type, exc_value, exc_traceback)))
+                raise RuntimeError("\n".join(messages))
 
     @contextmanager
     def _get_seed_wheels(self, creator):
diff --git a/src/virtualenv/util/lock.py b/src/virtualenv/util/lock.py
index eb7a78f..1fb8e4e 100644
--- a/src/virtualenv/util/lock.py
+++ b/src/virtualenv/util/lock.py
@@ -13,6 +13,12 @@ from virtualenv.util.path import Path
 
 class _CountedFileLock(FileLock):
     def __init__(self, lock_file):
+        parent = os.path.dirname(lock_file)
+        if not os.path.exists(parent):
+            try:
+                os.makedirs(parent)
+            except OSError:
+                pass
         super(_CountedFileLock, self).__init__(lock_file)
         self.count = 0
         self.thread_safe = RLock()
diff --git a/tests/unit/seed/embed/test_boostrap_link_via_app_data.py b/tests/unit/seed/embed/test_boostrap_link_via_app_data.py
index a48edd4..934e007 100644
--- a/tests/unit/seed/embed/test_boostrap_link_via_app_data.py
+++ b/tests/unit/seed/embed/test_boostrap_link_via_app_data.py
@@ -3,6 +3,7 @@ from __future__ import absolute_import, unicode_literals
 import os
 import sys
 from stat import S_IREAD, S_IRGRP, S_IROTH, S_IWUSR
+from threading import Thread
 
 import pytest
 
@@ -130,3 +131,35 @@ def test_base_bootstrap_link_via_app_data_no(tmp_path, coverage_env, current_fas
     assert not (result.creator.purelib / pkg).exists()
     for key in {"pip", "setuptools", "wheel"} - {pkg}:
         assert (result.creator.purelib / key).exists()
+
+
+def test_app_data_parallel_ok(tmp_path, temp_app_data):
+    exceptions = _run_parallel_threads(tmp_path)
+    assert not exceptions, "\n".join(exceptions)
+
+
+def test_app_data_parallel_fail(tmp_path, temp_app_data, mocker):
+    mocker.patch("virtualenv.seed.embed.via_app_data.pip_install.base.PipInstall.build_image", side_effect=RuntimeError)
+    exceptions = _run_parallel_threads(tmp_path)
+    assert len(exceptions) == 2
+    for exception in exceptions:
+        assert exception.startswith("failed to build image wheel because:\nTraceback")
+        assert "RuntimeError" in exception, exception
+
+
+def _run_parallel_threads(tmp_path):
+    exceptions = []
+
+    def _run(name):
+        try:
+            cli_run(["--seeder", "app-data", str(tmp_path / name), "--no-pip", "--no-setuptools"])
+        except Exception as exception:  # noqa
+            as_str = str(exception)
+            exceptions.append(as_str)
+
+    threads = [Thread(target=_run, args=("env{}".format(i),)) for i in range(1, 3)]
+    for thread in threads:
+        thread.start()
+    for thread in threads:
+        thread.join()
+    return exceptions