mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-20 22:34:00 +00:00
* Stop 19 test files racing on one PowerShell startup cache
Backend CI run 32341628757 on `1c3dde199` finished `284 failed, 8498 passed`. Every
one of the 284 was a pwsh subprocess ending `died with <Signals.SIGABRT: 6>`, across
19 files that all read as Windows-installer regressions. None of them were. 222 of
the aborts land inside a two-second window, 88 at 07:09:30 and 133 at 07:09:31,
which is a mass kill of every live pwsh rather than independent per-test flakiness.
The cause
------------------------------------------------------------------------
Every `-NonInteractive` startup reads and rewrites an ~83 KB
`$XDG_CACHE_HOME/powershell/StartupProfileData-NonInteractive`, and XDG_CACHE_HOME
defaults to `$HOME/.cache`. Under `-n 4` all four xdist workers share one HOME, so
the whole job's pwsh processes race on one file and a startup that deserialises a
half-written one dies before it reaches our script. `Stack overflow.` is .NET's
failfast, which cannot unwind a blown stack, so it prints one line and calls
abort(); that is the SIGABRT (PowerShell/PowerShell#24461).
Measured twice, independently, 4000 startups per arm:
run 1 shared cache dir 7/4000 died {-11: 3, -6: 4}
private cache dirs 0/4000
run 2 shared cache dir 11/4000 died {-11: 10, -6: 1}
private cache dirs 0/4000
Three distinct crash shapes appeared, and each names the torn file rather than our
scripts: `Stack overflow.`, `System.IO.FileLoadException: The given assembly name`,
and `System.ArgumentException: String cannot have zero length.` 18 deaths in 8000
shared startups, 0 in 8000 private.
CI agrees from the other direction. Of the pwsh-heavy files in that run, exactly one
had zero failures, tests/test_windows_amd_gpu_scan_fallback.py, and it is the only
one that hands its child a private HOME, across roughly 80 startups where the run's
own rate predicts about 16 failures.
What is NOT established
------------------------------------------------------------------------
Neither experiment reproduces CI's rate. Roughly 20% of pwsh startups died there
against 0.2 to 0.3% here, and at CI's actual `-n 4` on this box I measured 0/1200 in
both arms: the race needed 48-way concurrency before it appeared at all. The likely
reason is that four workers on a 4-core runner are in real contention while four
threads on a 192-core box almost never overlap in the critical section, but that is
reasoning and not a measurement, so treat the mechanism as proven and the magnitude
as unexplained. That is also why this does not stop at removing the shared file.
Three layers, in order
------------------------------------------------------------------------
1. Remove the contended resource. One cache directory per xdist worker, fresh per
session. Workers run their tests one at a time, so within a worker the startups
stay sequential and the cache still does its job warm; across workers the
directories are disjoint and there is nothing left to race on. Fresh rather than a
stable path, because a cache torn by an earlier run would otherwise poison every
later session on the same box.
2. Retry a run that produced no verdict. Three attempts, unslept, because the trigger
is process startup rather than a resource that frees up.
3. Attribute what is left. A crash raises PwshInterpreterCrash naming the interpreter.
Layer 1 is the fix; 2 and 3 exist because of the unexplained magnitude above.
Deliberately NOT done: bounding pwsh concurrency with a lock, or giving up `-n 4`.
The workflow records 806.1s to 219.7s from that flag, and the contended resource can
be removed rather than rationed.
The rule that keeps this honest
------------------------------------------------------------------------
A signal is not a verdict, so retrying it papers over nothing: the script never ran
to its end. A normal exit is returned untouched on the first attempt whatever its
code, so a pwsh that runs and gives the WRONG answer still fails with its own
message. Getting that second half wrong would turn this into a way to retry real
regressions into green, which is worse than the bug it fixes, so both directions are
executed in tests/studio/test_pwsh_interpreter_crash_attribution.py against a real
SIGABRT rather than reviewed.
Mutation-tested: relaxing the crash test from `returncode < 0` to `returncode != 0`
fails test_a_clean_run_with_the_wrong_answer_still_fails_with_its_own_message and
test_a_clean_run_is_not_retried, which are exactly the two that guard that direction.
This also generalises `_run_pwsh` from tests/studio/test_install_phase_timing.py,
added earlier today for a second, signal-free shape: pwsh printing its "The
PowerShell process will exit" banner and exiting normally with empty stdout. That one
cannot be seen in the exit status, so it stays a text match.
Verified
------------------------------------------------------------------------
tests/python/test_windows_xformers_installer.py, tests/studio/test_install_phase_timing.py,
tests/studio/install/ and the new guard: 2635 passed, 3 skipped.
Guard alone: 5 passed.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Drop the subprocess import the pwsh conversion left behind
Source lint's import-hoist check is right: every subprocess.run in
test_windows_xformers_installer.py became run_pwsh, so `import subprocess` has no
references left except the one inside a comment explaining why run_pwsh is used
instead. Its wording names the shape exactly -- "was used before, now unused
(references re-pointed)" -- which is what a mechanical call-site rewrite leaves
behind.
Swept the other 18 converted files the same way with an AST pass rather than by
eye. This was the only real one: the remaining hits are `from __future__ import
annotations`, which every such scan reports, and a PropertyMock in
test_rocm_support.py that is present on main unchanged.
42 passed.
* Suppress the core dump on the forged SIGABRT
tests/test_deliberate_crashes_suppress_cores.py caught this: the abort child had no
PR_SET_DUMPABLE=0, so each of these aborts piped a multi-MB core to apport before the
child could be reaped. The guard is right and its message names the fix.
The child still exits -6 and PR_GET_DUMPABLE reads 0, so all five verdicts are
unchanged. Linux-only and non-fatal elsewhere: Windows has no CDLL(None) and pipes no
core, so arming it there would trade a no-op for a lost test.
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <unslothai@gmail.com>
342 lines
13 KiB
Python
342 lines
13 KiB
Python
# SPDX-License-Identifier: AGPL-3.0-only
|
|
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved.
|
|
|
|
"""Windows must not build the venv on a CPython that cannot import torch.
|
|
|
|
CPython 3.13.8 carries python/cpython#139783: inspect.getsourcelines() drops a
|
|
function body when a decorator is followed by a comment, which is the shape of
|
|
the @_overload_method blocks torch/nn/modules/rnn.py parses at import time, so
|
|
`import torch` raises IndentationError (#7803).
|
|
|
|
Windows reaches such an interpreter differently from install.sh: uv is handed a
|
|
resolved path rather than a version, so it never picks the patch itself, but
|
|
Find-CompatiblePython matches on the *minor* version and would happily return an
|
|
already-installed 3.13.8. Remove-SkippedPython is what turns that into "not
|
|
found", so the caller installs $PythonFallbackFullVersion instead.
|
|
|
|
The function is extracted from install.ps1 and executed under pwsh rather than
|
|
reimplemented, so the test cannot drift from the text the installer runs.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import re
|
|
import shutil
|
|
import stat
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from unsloth_pwsh_runner import run_pwsh
|
|
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parents[2]
|
|
INSTALL_PS1 = REPO_ROOT / "install.ps1"
|
|
|
|
pytestmark = pytest.mark.skipif(
|
|
shutil.which("pwsh") is None, reason = "pwsh is required to execute install.ps1 blocks"
|
|
)
|
|
|
|
|
|
def _extract(pattern: str) -> str:
|
|
source = INSTALL_PS1.read_text(encoding = "utf-8")
|
|
match = re.search(pattern, source, flags = re.DOTALL)
|
|
assert match is not None, f"install.ps1 no longer contains {pattern!r}"
|
|
return match.group(0)
|
|
|
|
|
|
def _blocks() -> tuple:
|
|
"""The skip list and the screen, straight out of install.ps1.
|
|
|
|
Hoisted out of the f-strings below: a backslash inside an f-string expression
|
|
is a syntax error before 3.12, and this repo is 3.9+ (ruff targets py311).
|
|
"""
|
|
return (
|
|
_extract(
|
|
r" # Patch releases the stack cannot run.*?"
|
|
r"if \(\$SkipTorch\) \{ \$PythonSkip = @\(\) \}"
|
|
),
|
|
_extract(r" function Remove-SkippedPython \{.*?\n \}"),
|
|
)
|
|
|
|
|
|
def _fake_python(tmp_path: Path, version: str) -> Path:
|
|
"""An executable that reports ``version`` for the resolver's probe."""
|
|
if os.name == "nt":
|
|
exe = tmp_path / "python.cmd"
|
|
exe.write_text(f"@echo off\r\necho {version}\r\n", encoding = "utf-8")
|
|
return exe
|
|
exe = tmp_path / "python"
|
|
exe.write_text(f'#!/bin/sh\necho "{version}"\n', encoding = "utf-8")
|
|
exe.chmod(exe.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
|
|
return exe
|
|
|
|
|
|
def _run(tmp_path: Path, version: str | None) -> str:
|
|
candidate = (
|
|
"$null"
|
|
if version is None
|
|
else f'@{{ Version = "3.13"; Path = "{_fake_python(tmp_path, version)}" }}'
|
|
)
|
|
skip_block, screen_block = _blocks()
|
|
script = f"""
|
|
$ErrorActionPreference = "Stop"
|
|
$SkipTorch = $false
|
|
# Write-Host, like the real substep: Write-Output would put the message on
|
|
# the pipeline, so the function would return @(message, $null) and every
|
|
# `if ($DetectedPython)` downstream would read it as truthy.
|
|
function substep {{ param($m, $c) Write-Host "SUBSTEP: $m" }}
|
|
{skip_block}
|
|
{screen_block}
|
|
$result = Remove-SkippedPython ({candidate})
|
|
if ($null -eq $result) {{ Write-Output "RESULT: rejected" }}
|
|
else {{ Write-Output "RESULT: kept" }}
|
|
"""
|
|
# The whole verdict is the single RESULT line this script prints, so a pwsh that
|
|
# aborts at startup would read as a screen that reached the opposite conclusion.
|
|
completed = run_pwsh(
|
|
["pwsh", "-NoProfile", "-NonInteractive", "-Command", script],
|
|
capture_output = True,
|
|
text = True,
|
|
)
|
|
return completed.stdout + completed.stderr
|
|
|
|
|
|
def test_a_skipped_patch_is_rejected(tmp_path):
|
|
out = _run(tmp_path, "3.13.8")
|
|
assert "RESULT: rejected" in out, out
|
|
assert "cannot import torch" in out, "the user should be told why: " + out
|
|
|
|
|
|
def test_a_good_patch_of_the_same_minor_is_kept(tmp_path):
|
|
# The screen is per patch: 3.13 itself is fine and must not be refused.
|
|
out = _run(tmp_path, "3.13.13")
|
|
assert "RESULT: kept" in out, out
|
|
|
|
|
|
def test_nothing_found_stays_nothing(tmp_path):
|
|
out = _run(tmp_path, None)
|
|
assert "RESULT: rejected" in out, out
|
|
|
|
|
|
def test_an_unreadable_interpreter_is_not_treated_as_bad(tmp_path):
|
|
# A probe that cannot run is not evidence of a bad version, and refusing it
|
|
# would send a working machine down the install path for no reason.
|
|
missing = tmp_path / "does-not-exist"
|
|
skip_block, screen_block = _blocks()
|
|
script = f"""
|
|
$ErrorActionPreference = "Stop"
|
|
$SkipTorch = $false
|
|
# Write-Host, like the real substep: Write-Output would put the message on
|
|
# the pipeline, so the function would return @(message, $null) and every
|
|
# `if ($DetectedPython)` downstream would read it as truthy.
|
|
function substep {{ param($m, $c) Write-Host "SUBSTEP: $m" }}
|
|
{skip_block}
|
|
{screen_block}
|
|
$result = Remove-SkippedPython (@{{ Version = "3.13"; Path = "{missing}" }})
|
|
if ($null -eq $result) {{ Write-Output "RESULT: rejected" }}
|
|
else {{ Write-Output "RESULT: kept" }}
|
|
"""
|
|
# This case asserts the screen KEEPS an interpreter it could not probe, so an
|
|
# interpreter that dies would masquerade as the screen wrongly rejecting it.
|
|
completed = run_pwsh(
|
|
["pwsh", "-NoProfile", "-NonInteractive", "-Command", script],
|
|
capture_output = True,
|
|
text = True,
|
|
)
|
|
assert "RESULT: kept" in completed.stdout + completed.stderr, (
|
|
completed.stdout + completed.stderr
|
|
)
|
|
|
|
|
|
def test_the_resolver_is_screened_at_every_entry_point():
|
|
"""A bare Find-CompatiblePython in the install flow would defeat the screen."""
|
|
source = INSTALL_PS1.read_text(encoding = "utf-8")
|
|
flow = source[source.index("# ── Install Python if no compatible version") :]
|
|
flow = flow[: flow.index("# ── Install uv ──")]
|
|
bare = [
|
|
line.strip()
|
|
for line in flow.splitlines()
|
|
if "= Find-CompatiblePython" in line and "Remove-SkippedPython" not in line
|
|
]
|
|
assert not bare, f"unscreened resolver calls in the install flow: {bare}"
|
|
|
|
|
|
# ── The screen inside the resolver ──
|
|
# The window above starts at the install step, so it never sees the recovery
|
|
# paths: Install-PythonFromPythonOrg and Install-X64Python both end in a bare
|
|
# `return (Find-CompatiblePython)`. Screening every candidate as it is
|
|
# enumerated is what makes those safe, and is also what lets the resolver carry
|
|
# on to its next minor instead of giving up on the machine.
|
|
|
|
|
|
def _every_version_match_screens_the_patch() -> list[str]:
|
|
body = _extract(r" function Find-CompatiblePython \{.*?\n \}")
|
|
lines = body.splitlines()
|
|
unscreened = []
|
|
for i, line in enumerate(lines):
|
|
if 'match "Python' not in line:
|
|
continue
|
|
window = "\n".join(lines[i : i + 9])
|
|
if "$PythonSkip -contains" not in window:
|
|
unscreened.append(line.strip())
|
|
return unscreened
|
|
|
|
|
|
def test_every_enumerated_candidate_is_screened():
|
|
assert (
|
|
len(
|
|
[
|
|
l
|
|
for l in _extract(r" function Find-CompatiblePython \{.*?\n \}").splitlines()
|
|
if 'match "Python' in l
|
|
]
|
|
)
|
|
== 3
|
|
), "the resolver's enumeration sites moved; re-check the screen"
|
|
assert not _every_version_match_screens_the_patch(), (
|
|
"Find-CompatiblePython enumerates a candidate without checking $PythonSkip: "
|
|
f"{_every_version_match_screens_the_patch()}"
|
|
)
|
|
|
|
|
|
# The launcher below is a /bin/sh script. Windows has no shebang and no PATHEXT
|
|
# entry for an extensionless file, so `Get-Command py` does not find it and the
|
|
# resolver reports "none" whatever the versions are -- which would make the
|
|
# negative case pass for the wrong reason. The PowerShell under test is the same
|
|
# text on every platform, and pwsh runs it here, so these three cases run on
|
|
# POSIX and the rest of the file still covers Windows.
|
|
_POSIX_LAUNCHER_ONLY = pytest.mark.skipif(
|
|
os.name == "nt", reason = "the fake py launcher is a /bin/sh script"
|
|
)
|
|
|
|
|
|
def _fake_launcher(root: Path, versions: dict[str, str]) -> Path:
|
|
"""A `py` launcher over fake interpreters, one per minor in ``versions``."""
|
|
root.mkdir(parents = True, exist_ok = True)
|
|
branches = []
|
|
for minor, full in versions.items():
|
|
exe = root / f"python{minor.replace('.', '')}"
|
|
# -S -c "import sys; print(sys.base_prefix)" for the conda screen.
|
|
exe.write_text('#!/bin/sh\necho "/usr"\n', encoding = "utf-8")
|
|
exe.chmod(0o755)
|
|
branches.append(f' {minor}) ver="{full}"; exe="{exe}" ;;')
|
|
launcher = root / "py"
|
|
launcher.write_text(
|
|
"#!/bin/sh\n"
|
|
'case "$1" in\n'
|
|
+ "\n".join(f" -{b.lstrip()}" for b in branches)
|
|
+ "\n *) exit 1 ;;\nesac\n"
|
|
"shift\n"
|
|
'case "$1" in\n'
|
|
' --version) echo "Python $ver" ;;\n'
|
|
' -S) echo "$exe" ;;\n'
|
|
" *) exit 1 ;;\n"
|
|
"esac\n",
|
|
encoding = "utf-8",
|
|
)
|
|
launcher.chmod(0o755)
|
|
return launcher
|
|
|
|
|
|
def _resolve(tmp_path: Path, versions: dict[str, str]) -> str:
|
|
"""Run the real Find-CompatiblePython over ``versions`` and report the hit."""
|
|
root = tmp_path / "bin"
|
|
_fake_launcher(root, versions)
|
|
skip_block, screen_block = _blocks()
|
|
# Hoisted for the same reason as _blocks: a backslash in an f-string
|
|
# expression does not parse before 3.12.
|
|
conda_block = _extract(r" function Test-IsCondaPython \{.*?\n \}")
|
|
tag_block = _extract(r" function Get-PythonPlatformTag \{.*?\n \}")
|
|
resolver_block = _extract(r" function Find-CompatiblePython \{.*?\n \}")
|
|
script = f"""
|
|
$ErrorActionPreference = "Stop"
|
|
$SkipTorch = $false
|
|
$env:PATH = "{root}"
|
|
$PythonVersion = "3.13"
|
|
function substep {{ param($m, $c) Write-Host "SUBSTEP: $m" }}
|
|
function Get-HostMachineArch {{ return "x86_64" }}
|
|
{skip_block}
|
|
$script:CondaSkipPattern = '(?i)(conda|miniconda|anaconda|miniforge|mambaforge)'
|
|
{conda_block}
|
|
{tag_block}
|
|
{resolver_block}
|
|
$found = Find-CompatiblePython
|
|
if ($null -eq $found) {{ Write-Output "RESULT: none" }}
|
|
else {{ Write-Output "RESULT: $($found.Version)" }}
|
|
"""
|
|
# The caller scrapes the resolver's chosen minor out of stdout; a crashed pwsh
|
|
# leaves nothing to scrape and would fail as if Find-CompatiblePython went silent.
|
|
completed = run_pwsh(
|
|
["pwsh", "-NoProfile", "-NonInteractive", "-Command", script],
|
|
capture_output = True,
|
|
text = True,
|
|
)
|
|
out = completed.stdout + completed.stderr
|
|
match = re.search(r"RESULT: (\S+)", out)
|
|
assert match is not None, out
|
|
return match.group(1)
|
|
|
|
|
|
@_POSIX_LAUNCHER_ONLY
|
|
def test_the_resolver_falls_through_to_the_next_minor(tmp_path):
|
|
# The offline/locked-down case: 3.13.8 and a healthy 3.12 both installed and
|
|
# nothing installable. Ending the search on the 3.13 would leave the caller
|
|
# with a Python that cannot import torch; refusing it outright would fail a
|
|
# machine that has a perfectly good interpreter one entry down the list.
|
|
assert _resolve(tmp_path, {"3.13": "3.13.8", "3.12": "3.12.11"}) == "3.12"
|
|
|
|
|
|
@_POSIX_LAUNCHER_ONLY
|
|
def test_a_good_preferred_minor_still_wins(tmp_path):
|
|
assert _resolve(tmp_path, {"3.13": "3.13.13", "3.12": "3.12.11"}) == "3.13"
|
|
|
|
|
|
@_POSIX_LAUNCHER_ONLY
|
|
def test_nothing_usable_is_still_nothing(tmp_path):
|
|
# Paired with a positive control over the same tree, because "none" is also
|
|
# what a harness that cannot run the launcher at all reports: without the
|
|
# control this case would pass on a machine where it proves nothing.
|
|
assert _resolve(tmp_path / "good", {"3.13": "3.13.13"}) == "3.13"
|
|
assert _resolve(tmp_path / "bad", {"3.13": "3.13.8"}) == "none"
|
|
|
|
|
|
@_POSIX_LAUNCHER_ONLY
|
|
def test_no_torch_mode_keeps_the_skipped_patch(tmp_path):
|
|
"""The list is about `import torch`; -NoTorch never imports it.
|
|
|
|
A locked-down GGUF-only machine whose only Python is 3.13.8 would otherwise
|
|
be pushed into winget/python.org recovery it may not be able to complete.
|
|
"""
|
|
root = tmp_path / "bin"
|
|
_fake_launcher(root, {"3.13": "3.13.8"})
|
|
skip_block, screen_block = _blocks()
|
|
conda_block = _extract(r" function Test-IsCondaPython \{.*?\n \}")
|
|
tag_block = _extract(r" function Get-PythonPlatformTag \{.*?\n \}")
|
|
resolver_block = _extract(r" function Find-CompatiblePython \{.*?\n \}")
|
|
script = f"""
|
|
$ErrorActionPreference = "Stop"
|
|
$SkipTorch = $true
|
|
$env:PATH = "{root}"
|
|
$PythonVersion = "3.13"
|
|
function substep {{ param($m, $c) Write-Host "SUBSTEP: $m" }}
|
|
function Get-HostMachineArch {{ return "x86_64" }}
|
|
{skip_block}
|
|
$script:CondaSkipPattern = '(?i)(conda|miniconda|anaconda|miniforge|mambaforge)'
|
|
{conda_block}
|
|
{tag_block}
|
|
{resolver_block}
|
|
$found = Find-CompatiblePython
|
|
if ($null -eq $found) {{ Write-Output "RESULT: none" }}
|
|
else {{ Write-Output "RESULT: $($found.Version)" }}
|
|
"""
|
|
# -NoTorch must leave 3.13.8 in place, and dying before the RESULT line is printed
|
|
# is indistinguishable here from the screen having removed it.
|
|
completed = run_pwsh(
|
|
["pwsh", "-NoProfile", "-NonInteractive", "-Command", script],
|
|
capture_output = True,
|
|
text = True,
|
|
)
|
|
out = completed.stdout + completed.stderr
|
|
assert "RESULT: 3.13" in out, out
|