mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-21 14:53:58 +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>
147 lines
5.9 KiB
Python
147 lines
5.9 KiB
Python
# SPDX-License-Identifier: AGPL-3.0-only
|
|
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved.
|
|
|
|
"""Windows on ARM: install.ps1 must not settle for a native ARM64 interpreter.
|
|
|
|
pyarrow (via datasets) and hf-transfer publish no win_arm64 wheels, so an ARM64
|
|
Python source-builds both and dies minutes into the run. The resolver prefers an
|
|
x64 build of the requested minor and bootstraps one otherwise; the case pinned
|
|
here is the recovery path, where nothing can be downloaded but an x64 build of a
|
|
lower-priority supported minor is already installed.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import re
|
|
import shutil
|
|
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"
|
|
|
|
|
|
def _extract(pattern: str, source: str) -> str:
|
|
match = re.search(pattern, source, flags = re.DOTALL)
|
|
assert match is not None, f"install.ps1 block not found: {pattern}"
|
|
return match.group(0)
|
|
|
|
|
|
def _resolver_script(installed: list[tuple[str, str]], can_download: bool) -> str:
|
|
"""Both production functions verbatim, over a fake set of interpreters.
|
|
|
|
Extracted rather than reimplemented so the test cannot drift away from the
|
|
text install.ps1 actually runs. `installed` is (minor, arch) in py-launcher
|
|
order, so the first entry for a minor is what a bare `py -3.13` resolves to.
|
|
The fake interpreters are named `*.exe` and invoked through the call operator,
|
|
which resolves a string to a function, so no real binary is needed.
|
|
"""
|
|
source = INSTALL_PS1.read_text(encoding = "utf-8")
|
|
finder = _extract(r" function Find-CompatiblePython \{.*?\n \}\n", source)
|
|
installer = _extract(r" function Install-X64Python \{.*?\n \}\n", source)
|
|
|
|
names = [f"Py{minor.replace('.', '')}{arch}.exe" for minor, arch in installed]
|
|
table = ", ".join(
|
|
f'@{{ Minor = "{minor}"; Arch = "{arch}"; Name = "{name}" }}'
|
|
for (minor, arch), name in zip(installed, names)
|
|
)
|
|
downloaded = (
|
|
'@{ Version = "3.13"; Path = "Downloaded.exe"; Arch = "x86_64" }'
|
|
if can_download
|
|
else "$null"
|
|
)
|
|
version_stubs = "\n".join(
|
|
f"function {name} {{ param([Parameter(ValueFromRemainingArguments = $true)]$Rest)\n"
|
|
f' if ($Rest -contains "--version") {{ return "Python {minor}.0" }}\n'
|
|
f' return "{name}" }}'
|
|
for (minor, _arch), name in zip(installed, names)
|
|
)
|
|
return f"""
|
|
$ErrorActionPreference = "Stop"
|
|
$PythonVersion = "3.13"
|
|
$script:WingetAvailable = $false
|
|
$script:CondaSkipPattern = 'conda'
|
|
$Interpreters = @({table})
|
|
{version_stubs}
|
|
# `py -0p` lists every registration; `py -3.x` runs the launcher's preferred build
|
|
# for that minor, which on an ARM64 host is normally the native one.
|
|
function FakePy {{
|
|
param([Parameter(ValueFromRemainingArguments = $true)]$Rest)
|
|
if ($Rest -contains "-0p") {{
|
|
return @($Interpreters | ForEach-Object {{ " -V:$($_.Minor) * $($_.Name)" }})
|
|
}}
|
|
$minor = ([string]$Rest[0]).TrimStart('-')
|
|
$hit = @($Interpreters | Where-Object {{ $_.Minor -eq $minor }})
|
|
if ($hit.Count -eq 0) {{ return "" }}
|
|
if ($Rest -contains "--version") {{ return "Python $minor.0" }}
|
|
return $hit[0].Name
|
|
}}
|
|
function substep {{ param($a, $b) }}
|
|
function Get-HostMachineArch {{ return "arm64" }}
|
|
function Get-Command {{
|
|
param([Parameter(Position = 0)][string]$Name,
|
|
[Parameter(ValueFromRemainingArguments = $true)]$Rest)
|
|
if ($Name -eq "py") {{ return @([pscustomobject]@{{ Source = "FakePy" }}) }}
|
|
return @()
|
|
}}
|
|
function Test-Path {{ param([Parameter(ValueFromRemainingArguments = $true)]$Rest) return $true }}
|
|
function Test-IsCondaPython {{ param([string]$Exe) return $false }}
|
|
function Get-PythonPlatformTag {{
|
|
param([string]$Exe)
|
|
foreach ($i in $Interpreters) {{
|
|
if ($i.Name -eq $Exe) {{
|
|
if ($i.Arch -eq "x86_64") {{ return "win-amd64" }} else {{ return "win-arm64" }}
|
|
}}
|
|
}}
|
|
return "win-amd64"
|
|
}}
|
|
function Refresh-SessionPath {{ }}
|
|
function Install-PythonFromPythonOrg {{ param([string]$Arch = "") return {downloaded} }}
|
|
{finder}
|
|
{installer}
|
|
# The caller's ARM64 swap, condensed to what decides the interpreter.
|
|
$found = Find-CompatiblePython
|
|
if ($found -and $found.Arch -ne "x86_64") {{
|
|
$x64 = Install-X64Python
|
|
if ($x64) {{ $found = $x64 }}
|
|
}}
|
|
if ($found) {{ Write-Output "$($found.Version)|$($found.Arch)" }} else {{ Write-Output "none" }}
|
|
"""
|
|
|
|
|
|
def _pwsh(script: str) -> str:
|
|
# Every ARM64 case below is decided by the one "version|arch" line this run prints,
|
|
# and check = True means a pwsh that aborts at startup would surface as the resolver
|
|
# block itself throwing.
|
|
result = run_pwsh(
|
|
["pwsh", "-NoProfile", "-NonInteractive", "-Command", script],
|
|
check = True,
|
|
capture_output = True,
|
|
text = True,
|
|
env = os.environ.copy(),
|
|
)
|
|
return result.stdout.strip()
|
|
|
|
|
|
@pytest.mark.skipif(shutil.which("pwsh") is None, reason = "PowerShell is unavailable")
|
|
@pytest.mark.parametrize(
|
|
("installed", "can_download", "expected"),
|
|
[
|
|
# An x64 build of the requested minor wins outright, downloads irrelevant.
|
|
([("3.13", "arm64"), ("3.13", "x86_64")], False, "3.13|x86_64"),
|
|
# Requested minor is ARM64-only: bootstrap x64 rather than take the native one.
|
|
([("3.13", "arm64")], True, "3.13|x86_64"),
|
|
# Offline, but an x64 build of a lower-priority minor is here. Use it: the native
|
|
# 3.13 cannot resolve pyarrow or hf-transfer, and this one can.
|
|
([("3.13", "arm64"), ("3.11", "x86_64")], False, "3.11|x86_64"),
|
|
# ARM64 everywhere: still returned, and the caller warns.
|
|
([("3.13", "arm64"), ("3.11", "arm64")], False, "3.13|arm64"),
|
|
],
|
|
)
|
|
def test_arm64_host_prefers_an_x64_interpreter(installed, can_download, expected):
|
|
assert _pwsh(_resolver_script(installed, can_download)) == expected
|