Skip to content

Commit e3a301f

Browse files
authored
Fix test-suite sys.executable leak that skipped free-threaded memory tests (#1066)
Root cause: test_coverup_30::test_scalene_cpu_count constructs Scalene(args) in-process to check __availableCPUs. Scalene.__init__ runs the full profiler setup, including scalene.redirect_python.redirect_python -- production behavior that rewrites sys.executable / sys.path / PATH to a /tmp/scalene*/python bash wrapper so child processes re-enter through Scalene. That is correct for a real one-shot 'scalene run' process, but in the shared, long-lived pytest interpreter it leaks: the test mocks ScaleneMapFile but not the redirect and never restores the globals. Every later test that spawns [sys.executable, '-m', 'scalene', ...] then launched the wrapper instead of real Python, so memory profiling failed to initialize ([Errno 2] .../tmp/scalene-malloc-signal<pid>) and the memory_* tests skipped with 'no usable profile' (or, for a few CPU tests, timed out). Ordering-dependent; it surfaced on free-threaded CI but is not free-threaded-specific. A full-suite scan found test_coverup_30 to be the sole polluter. Fix (minimal, at the source): test_coverup_30's cleanup fixture now snapshots and restores sys.executable / sys.path / PATH. Verified on Linux 3.13t: the test no longer leaves sys.executable pointing at the wrapper, and the memory tests pass in suite order even with bare sys.executable in the spawn helpers. Also: - tests/test_memory_stacks_bigmem.py: once the test stopped skipping it actually ran and exposed a too-strict assertion. The driver's own list.append() sites legitimately allocate (CPython list-backing-store growth), so a memory_stacks leaf occasionally lands on the append line, not make_vec. Require the allocator line to dominate (>=90% of leaf bytes) instead of being every leaf; the make_big/make_small split asserts are unchanged. - tests/conftest.py + tests.yml: a SCALENE_TEST_DIAG-gated hook that prints each skip's reason live/flushed (off by default; on for t-builds), so skip reasons survive a job-timeout cancellation for future debugging.
1 parent 52aec24 commit e3a301f

4 files changed

Lines changed: 72 additions & 7 deletions

File tree

.github/workflows/tests.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ jobs:
113113
- name: run tests
114114
# Free-threaded Python support is experimental; don't block CI on failures
115115
continue-on-error: ${{ endsWith(matrix.python, 't') }}
116+
# SCALENE_TEST_DIAG=1 makes tests/conftest.py print each skip's reason
117+
# live and flushed. On free-threaded builds the 45-min job timeout can
118+
# cancel pytest before its -rs summary, so this keeps skip reasons in
119+
# the log for future debugging.
120+
env:
121+
SCALENE_TEST_DIAG: ${{ endsWith(matrix.python, 't') && '1' || '' }}
116122
run: |
117123
python3 -m pytest
118124

tests/conftest.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
"""Pytest hooks for the Scalene test suite.
2+
3+
The free-threaded CI jobs (3.13t/3.14t) can run past the 45-minute job
4+
timeout and get cancelled mid-suite. When that happens pytest never reaches
5+
its end-of-session ``-rs`` summary, so the *reasons* any memory_* tests
6+
skipped are lost. ``pytest_runtest_logreport`` fires as each test finishes,
7+
so printing the skip reason here (flushed) guarantees it lands in the CI log
8+
even if the job is killed before the summary. Gated on SCALENE_TEST_DIAG=1
9+
so normal local runs stay quiet.
10+
"""
11+
12+
import os
13+
import sys
14+
15+
16+
def pytest_runtest_logreport(report):
17+
if os.environ.get("SCALENE_TEST_DIAG") != "1":
18+
return
19+
if not report.skipped:
20+
return
21+
if report.when not in ("setup", "call"):
22+
return
23+
# report.longrepr for a skip is a (path, lineno, reason) tuple.
24+
reason = report.longrepr
25+
if isinstance(reason, tuple) and len(reason) == 3:
26+
reason = reason[2]
27+
print(
28+
f"\n[SCALENE_TEST_DIAG] SKIP {report.nodeid}\n"
29+
f" reason: {reason}\n"
30+
f" LD_PRELOAD={os.environ.get('LD_PRELOAD')!r} "
31+
f"DYLD_INSERT_LIBRARIES={os.environ.get('DYLD_INSERT_LIBRARIES')!r}\n"
32+
f" PYTHONMALLOC={os.environ.get('PYTHONMALLOC')!r} "
33+
f"PYTHON_GIL={os.environ.get('PYTHON_GIL')!r}\n"
34+
f" sys.executable={sys.executable!r}",
35+
flush=True,
36+
)

tests/test_coverup_30.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,22 @@
1212

1313
@pytest.fixture
1414
def cleanup():
15-
# Fixture to clean up any state after the test
15+
# Constructing Scalene(args) runs the full profiler __init__, which calls
16+
# scalene.redirect_python.redirect_python -- production behavior that
17+
# rewrites sys.executable / sys.path / PATH to a /tmp/scalene*/python
18+
# wrapper so child processes re-enter through Scalene. That is correct for
19+
# a real one-shot `scalene run` process, but here it leaks into the shared
20+
# pytest interpreter: later tests that spawn [sys.executable, "-m",
21+
# "scalene", ...] would launch the wrapper instead of real Python and fail
22+
# to profile. Snapshot and restore the mutated global state.
23+
orig_executable = sys.executable
24+
orig_path = list(sys.path)
25+
orig_environ_path = os.environ.get("PATH")
1626
yield
17-
# No specific cleanup required for this test
27+
sys.executable = orig_executable
28+
sys.path[:] = orig_path
29+
if orig_environ_path is not None:
30+
os.environ["PATH"] = orig_environ_path
1831

1932

2033
@patch("scalene.scalene_profiler.ScaleneMapFile")

tests/test_memory_stacks_bigmem.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,22 @@ def test_memory_stacks_split_across_two_callers(tmp_path: Path) -> None:
111111
profile = _run_scalene(tmp_path)
112112
memory_stacks = profile["memory_stacks"]
113113

114-
# Every captured stack must have the allocator line 20 as its leaf.
114+
# The allocator line 20 must dominate the captured leaves. We don't
115+
# require *every* leaf to be line 20: the driver's own
116+
# ``small_list.append(...)`` / ``big_list.append(...)`` sites (lines 35/37)
117+
# grow CPython's list backing store, a genuine allocation whose
118+
# synchronously-captured leaf is that append line, not make_vec. That is
119+
# correct attribution, just not the allocator helper, and it shows up as
120+
# an occasional stray leaf under sampling noise (seen on macOS). Assert
121+
# that make_vec dominates rather than that nothing else ever allocates.
115122
leaves = [_leaf_line(frames) for frames, _mb in memory_stacks]
116-
assert all(leaf == ALLOCATOR_LINE for leaf in leaves), (
117-
f"Expected all memory_stacks leaves to land on allocator line "
118-
f"{ALLOCATOR_LINE}, got {leaves}. Intermediate frames being "
119-
f"skipped or the allocator being misattributed."
123+
alloc_leaf_mb = sum(mb for frames, mb in memory_stacks if _leaf_line(frames) == ALLOCATOR_LINE)
124+
total_leaf_mb = sum(mb for _frames, mb in memory_stacks)
125+
assert total_leaf_mb > 0 and alloc_leaf_mb >= 0.9 * total_leaf_mb, (
126+
f"Expected allocator line {ALLOCATOR_LINE} to dominate memory_stacks "
127+
f"leaves (>=90% of bytes), got {alloc_leaf_mb:.1f} of {total_leaf_mb:.1f} MB; "
128+
f"leaves={leaves}. Intermediate frames being skipped or the allocator "
129+
f"being misattributed."
120130
)
121131

122132
# The caller frame (directly above the allocator leaf) must pick up

0 commit comments

Comments
 (0)