Skip to content

Commit 3706faf

Browse files
committed
fix: Prevent NameError when accessing _DISTUTILS_PATCH during file overwrite
1 parent a07135f commit 3706faf

File tree

4 files changed

+208
-6
lines changed

4 files changed

+208
-6
lines changed

docs/changelog/2969.bugfix.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix race condition in ``_virtualenv.py`` when file is overwritten during import, preventing ``NameError`` when ``_DISTUTILS_PATCH`` is accessed - by :user:`gracetyy`.

src/virtualenv/create/via_global_ref/_virtualenv.py

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22

33
from __future__ import annotations
44

5+
import contextlib
56
import os
67
import sys
78

8-
VIRTUALENV_PATCH_FILE = os.path.join(__file__)
9+
VIRTUALENV_PATCH_FILE = os.path.abspath(__file__)
910

1011

1112
def patch_dist(dist):
@@ -50,7 +51,14 @@ class _Finder:
5051
lock = [] # noqa: RUF012
5152

5253
def find_spec(self, fullname, path, target=None): # noqa: ARG002
53-
if fullname in _DISTUTILS_PATCH and self.fullname is None: # noqa: PLR1702
54+
# Guard against race conditions during file rewrite by checking if _DISTUTILS_PATCH is defined.
55+
# This can happen when the file is being overwritten while it's being imported by another process.
56+
# See https://github.com/pypa/virtualenv/issues/2969 for details.
57+
try:
58+
distutils_patch = _DISTUTILS_PATCH
59+
except NameError:
60+
return None
61+
if fullname in distutils_patch and self.fullname is None: # noqa: PLR1702
5462
# initialize lock[0] lazily
5563
if len(self.lock) == 0:
5664
import threading # noqa: PLC0415
@@ -89,14 +97,26 @@ def find_spec(self, fullname, path, target=None): # noqa: ARG002
8997
@staticmethod
9098
def exec_module(old, module):
9199
old(module)
92-
if module.__name__ in _DISTUTILS_PATCH:
93-
patch_dist(module)
100+
try:
101+
distutils_patch = _DISTUTILS_PATCH
102+
except NameError:
103+
return
104+
if module.__name__ in distutils_patch:
105+
# patch_dist or its dependencies may not be defined during file rewrite
106+
with contextlib.suppress(NameError):
107+
patch_dist(module)
94108

95109
@staticmethod
96110
def load_module(old, name):
97111
module = old(name)
98-
if module.__name__ in _DISTUTILS_PATCH:
99-
patch_dist(module)
112+
try:
113+
distutils_patch = _DISTUTILS_PATCH
114+
except NameError:
115+
return module
116+
if module.__name__ in distutils_patch:
117+
# patch_dist or its dependencies may not be defined during file rewrite
118+
with contextlib.suppress(NameError):
119+
patch_dist(module)
100120
return module
101121

102122

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
from __future__ import annotations
2+
3+
import importlib.util
4+
import sys
5+
import tempfile
6+
from pathlib import Path
7+
from textwrap import dedent
8+
9+
10+
def test_race_condition_simulation():
11+
"""Test that simulates the race condition described in the issue.
12+
13+
This test creates a temporary directory with _virtualenv.py and _virtualenv.pth,
14+
then simulates the scenario where:
15+
- One process imports and uses the _virtualenv module (simulating marimo)
16+
- Another process overwrites the _virtualenv.py file (simulating uv venv)
17+
18+
The test verifies that no NameError is raised for _DISTUTILS_PATCH.
19+
"""
20+
with tempfile.TemporaryDirectory() as tmpdir:
21+
venv_path = Path(tmpdir)
22+
23+
# Create the _virtualenv.py file
24+
virtualenv_file = venv_path / "_virtualenv.py"
25+
source_file = (
26+
Path(__file__).parent.parent / "src" / "virtualenv" / "create" / "via_global_ref" / "_virtualenv.py"
27+
)
28+
29+
if not source_file.exists():
30+
return # Skip test if source file doesn't exist
31+
32+
content = source_file.read_text(encoding="utf-8")
33+
virtualenv_file.write_text(content, encoding="utf-8")
34+
35+
# Create the _virtualenv.pth file
36+
pth_file = venv_path / "_virtualenv.pth"
37+
pth_file.write_text("import _virtualenv", encoding="utf-8")
38+
39+
# Simulate the race condition by alternating between importing and overwriting
40+
errors = []
41+
for _ in range(5):
42+
# Overwrite the file
43+
virtualenv_file.write_text(content, encoding="utf-8")
44+
45+
# Try to import it
46+
sys.path.insert(0, str(venv_path))
47+
try:
48+
if "_virtualenv" in sys.modules:
49+
del sys.modules["_virtualenv"]
50+
51+
import _virtualenv # noqa: F401
52+
53+
# Try to trigger find_spec
54+
try:
55+
importlib.util.find_spec("distutils.dist")
56+
except NameError as e:
57+
if "_DISTUTILS_PATCH" in str(e):
58+
errors.append(str(e))
59+
finally:
60+
if str(venv_path) in sys.path:
61+
sys.path.remove(str(venv_path))
62+
63+
# Clean up
64+
if "_virtualenv" in sys.modules:
65+
del sys.modules["_virtualenv"]
66+
67+
assert not errors, f"Race condition detected: {errors}"
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
from __future__ import annotations
2+
3+
import sys
4+
import tempfile
5+
from pathlib import Path
6+
from textwrap import dedent
7+
8+
9+
def test_virtualenv_py_race_condition_find_spec():
10+
"""Test that _Finder.find_spec handles NameError gracefully when _DISTUTILS_PATCH is not defined."""
11+
# Create a temporary file with partial _virtualenv.py content (simulating race condition)
12+
with tempfile.TemporaryDirectory() as tmpdir:
13+
venv_file = Path(tmpdir) / "_virtualenv_test.py"
14+
15+
# Write a partial version of _virtualenv.py that has _Finder but not _DISTUTILS_PATCH
16+
# This simulates the state during a race condition where the file is being rewritten
17+
partial_content = dedent("""
18+
import sys
19+
20+
class _Finder:
21+
fullname = None
22+
lock = []
23+
24+
def find_spec(self, fullname, path, target=None):
25+
# This should handle the NameError gracefully
26+
try:
27+
distutils_patch = _DISTUTILS_PATCH # noqa: F821
28+
except NameError:
29+
return None
30+
if fullname in distutils_patch and self.fullname is None:
31+
return None
32+
return None
33+
34+
@staticmethod
35+
def exec_module(old, module):
36+
old(module)
37+
try:
38+
distutils_patch = _DISTUTILS_PATCH # noqa: F821
39+
except NameError:
40+
return
41+
if module.__name__ in distutils_patch:
42+
pass # Would call patch_dist(module)
43+
44+
@staticmethod
45+
def load_module(old, name):
46+
module = old(name)
47+
try:
48+
distutils_patch = _DISTUTILS_PATCH # noqa: F821
49+
except NameError:
50+
return module
51+
if module.__name__ in distutils_patch:
52+
pass # Would call patch_dist(module)
53+
return module
54+
55+
finder = _Finder()
56+
""")
57+
58+
venv_file.write_text(partial_content, encoding="utf-8")
59+
60+
# Add the directory to sys.path temporarily
61+
sys.path.insert(0, tmpdir)
62+
try:
63+
# Import the module
64+
import _virtualenv_test # noqa: F401
65+
66+
# Get the finder instance
67+
finder = _virtualenv_test.finder
68+
69+
# Try to call find_spec - this should not raise NameError
70+
result = finder.find_spec("distutils.dist", None)
71+
assert result is None, "find_spec should return None when _DISTUTILS_PATCH is not defined"
72+
73+
# Create a mock module object
74+
class MockModule:
75+
__name__ = "distutils.dist"
76+
77+
# Try to call exec_module - this should not raise NameError
78+
mock_old_exec = lambda x: None # noqa: E731
79+
finder.exec_module(mock_old_exec, MockModule())
80+
81+
# Try to call load_module - this should not raise NameError
82+
mock_old_load = lambda name: MockModule() # noqa: E731
83+
result = finder.load_module(mock_old_load, "distutils.dist")
84+
assert result.__name__ == "distutils.dist"
85+
86+
finally:
87+
# Clean up
88+
sys.path.remove(tmpdir)
89+
if "_virtualenv_test" in sys.modules:
90+
del sys.modules["_virtualenv_test"]
91+
92+
93+
def test_virtualenv_py_normal_operation():
94+
"""Test that the fix doesn't break normal operation when _DISTUTILS_PATCH is defined."""
95+
# Read the actual _virtualenv.py file
96+
virtualenv_py_path = (
97+
Path(__file__).parent.parent.parent.parent.parent
98+
/ "src"
99+
/ "virtualenv"
100+
/ "create"
101+
/ "via_global_ref"
102+
/ "_virtualenv.py"
103+
)
104+
105+
if not virtualenv_py_path.exists():
106+
return # Skip if we can't find the file
107+
108+
content = virtualenv_py_path.read_text(encoding="utf-8")
109+
110+
# Verify the fix is present
111+
assert "try:" in content
112+
assert "distutils_patch = _DISTUTILS_PATCH" in content
113+
assert "except NameError:" in content
114+
assert "return None" in content or "return" in content

0 commit comments

Comments
 (0)