René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"A race condition in `mypy_test.py`","articleBody":"#9408 (my PR) appears to have introduced a race condition in `mypy_test.py`.\r\n\r\n## Evidence of the race\r\n\r\nWe've had several \"flukey\" errors in CI over the last few days, all with the same traceback:\r\n- On Python 3.7: https://github.com/python/typeshed/actions/runs/3909559123/jobs/6682289730\r\n- On Python 3.8:\r\n  - https://github.com/python/typeshed/actions/runs/3904904286/jobs/6671274990\r\n  - https://github.com/python/typeshed/actions/runs/3895029167/jobs/6649818621\r\n- On Python 3.9: https://github.com/python/typeshed/actions/runs/3916447325/jobs/6695558555\r\n\r\n\u003cdetails\u003e\r\n\u003csummary\u003eThe traceback is the same every time:\u003c/summary\u003e\r\n\r\n```pytb\r\nTesting third-party packages...\r\nTraceback (most recent call last):\r\n  File \"./tests/mypy_test.py\", line 557, in \u003cmodule\u003e\r\n    main()\r\n  File \"./tests/mypy_test.py\", line 544, in main\r\n    code, files_checked_this_version = test_typeshed(code, args=config, tempdir=td_path)\r\n  File \"./tests/mypy_test.py\", line 525, in test_typeshed\r\n    code, third_party_files_checked = test_third_party_stubs(code, args, tempdir)\r\n  File \"./tests/mypy_test.py\", line 500, in test_third_party_stubs\r\n    setup_virtual_environments(distributions_to_check, args, tempdir)\r\n  File \"./tests/mypy_test.py\", line 444, in setup_virtual_environments\r\n    requirements_set, venv_info = venv_info_future.result()\r\n  File \"/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/concurrent/futures/_base.py\", line 428, in result\r\n    return self.__get_result()\r\n  File \"/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/concurrent/futures/_base.py\", line 384, in __get_result\r\n    raise self._exception\r\n  File \"/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/concurrent/futures/thread.py\", line 57, in run\r\n    result = self.fn(*self.args, **self.kwargs)\r\n  File \"./tests/mypy_test.py\", line 384, in setup_venv_for_external_requirements_set\r\n    return requirements_set, make_venv(venv_dir)\r\n  File \"/home/runner/work/typeshed/typeshed/tests/utils.py\", line 150, in make_venv\r\n    venv.create(venv_dir, with_pip=True, clear=True)\r\n  File \"/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/venv/__init__.py\", line 390, in create\r\n    builder.create(env_dir)\r\n  File \"/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/venv/__init__.py\", line 68, in create\r\n    self._setup_pip(context)\r\n  File \"/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/venv/__init__.py\", line 288, in _setup_pip\r\n    subprocess.check_output(cmd, stderr=subprocess.STDOUT)\r\n  File \"/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/subprocess.py\", line 411, in check_output\r\n    **kwargs).stdout\r\n  File \"/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/subprocess.py\", line 488, in run\r\n    with Popen(*popenargs, **kwargs) as process:\r\n  File \"/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/subprocess.py\", line 800, in __init__\r\n    restore_signals, start_new_session)\r\n  File \"/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/subprocess.py\", line 1551, in _execute_child\r\n    raise child_exception_type(errno_num, err_msg, err_filename)\r\nOSError: [Errno 26] Text file busy: '/tmp/tmp45sugieh/.venv-5354540476898/bin/python'\r\n```\r\n\r\n\u003c/details\u003e\r\n\r\n## Cause of the race\r\n\r\nThe race happens in this part of the test, where multiple virtual environments are set up concurrently using a threadpool.\r\n\r\nhttps://github.com/python/typeshed/blob/0a291da2f08d90604e7eba80abe1e3f64a8a10a4/tests/mypy_test.py#L438-L445\r\n\r\nHowever, it's unclear exactly _why_ the race occurs. The traceback indicates that two threads appear to be attempting to access the same Python executable at the same time. But it's hard to see why that would be the case.\r\n\r\n## Reproducing the race condition\r\n\r\n@JelleZijlstra has managed to reproduce the race locally on a Linux machine using Python 3.7. This rules out any GitHub Actions-specific hypotheses for why the race might be happening. \r\n\r\nI have not been able to reproduce the failure in my local environment (Windows, Python 3.10). (I've tried creating a venv for 145 stubs packages in a threadpool with 20 workers -- still no race for me.)\r\n\r\nNote that `mypy_test.py` is only ever run on Ubuntu in CI.\r\n\r\n## Fixing the issue\r\n\r\n### Idea (1): Just create the venvs one-at-a-time\r\n\r\nYou can't have a race condition if you don't try to do things concurrently. Locally, this makes the test much slower for me -- the time taken to create the venvs goes up from around 20 seconds to around 60 seconds. The specific part of creating a venv that is slow is the `venv.EnvBuilder._setup_pip` function in the stdlib -- the exact part that appears to be implicated in the race condition.\r\n\r\n_However_, if we did go this route, we could potentially mitigate the performance regression by caching venvs between runs of the test (both locally and in CI).\r\n\r\n### Idea (2): Do concurrency differently\r\n\r\nInstead of using a threadpool, we could manually create our threads. Or we could move to an `asyncio`-based concurrency model.\r\n\r\nEither of these _might_ fix the race condition, but it's hard to know if they _would_ without knowing exactly what's causing the race condition.\r\n\r\n### Idea (3): Paper over the problem\r\n\r\nThis diff would _probably_ \"fix\" the problem, but it wouldn't be a particularly _principled_ fix:\r\n\r\n\u003cdetails\u003e\r\n\r\n```diff\r\ndiff --git a/tests/mypy_test.py b/tests/mypy_test.py\r\nindex 087f41366..a0a66ed31 100644\r\n--- a/tests/mypy_test.py\r\n+++ b/tests/mypy_test.py\r\n@@ -381,7 +381,17 @@ _DISTRIBUTION_TO_VENV_MAPPING: dict[str, VenvInfo] = {}\r\n\r\n def setup_venv_for_external_requirements_set(requirements_set: frozenset[str], tempdir: Path) -\u003e tuple[frozenset[str], VenvInfo]:\r\n     venv_dir = tempdir / f\".venv-{hash(requirements_set)}\"\r\n-    return requirements_set, make_venv(venv_dir)\r\n+    while True:\r\n+        try:\r\n+            venv_info = make_venv(venv_dir)\r\n+        except OSError as e:\r\n+            if e.errno != 26:\r\n+                raise\r\n+            else:\r\n+                time.sleep(0.5)\r\n+        else:\r\n+            break\r\n+    return requirements_set, venv_info\r\n```\r\n\r\n\u003c/details\u003e","author":{"url":"https://github.com/AlexWaygood","@type":"Person","name":"AlexWaygood"},"datePublished":"2023-01-14T20:04:04.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":6},"url":"https://github.com/9537/typeshed/issues/9537"}