smoketest_pool_spawn: validate profile output, not exit code - #1054
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Three changes layered on the previous version of this test:
1. Validate scalene's profile JSON as a fallback when scalene exits
with anything other than the known-good rc=0 / rc=1 codes. The
point of this smoketest is "did scalene complete profiling?" — if
the profile is on disk, it did, even if the interpreter then
crashed during multiprocessing resource-tracker teardown (the
``0xFFFFFFFF`` kill-style exit seen on Windows under the
spawn-pool shutdown race).
2. Preserve the previous accept-list (rc in {0, 1}) outright. On
master those are the overwhelmingly common cases on every OS;
asserting the profile path on every run was the source of the
Windows regression in the first pass of this change.
3. Don't pass ``-o`` to scalene. Let it write to its default
``scalene-profile.json`` in cwd. The first pass redirected output
to ``tempfile.gettempdir()``, which on Windows resolves to
``C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\...`` (the short-path
form of ``runneradmin``); scalene didn't write there in CI even
though local runs were fine. Sticking with the default avoids
that whole class of platform-specific path issue.
Also captures scalene's stderr to a temp file and dumps the last
4 KB only when validation fails. Diagnostic output is preserved
without flooding successful runs.
Verified locally on macOS Python 3.14: rc=0, profile produced.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
emeryberger
force-pushed
the
fix-windows-spawn-pool-flake
branch
from
May 11, 2026 20:39
0eab08b to
4d2291c
Compare
| # fresh output. | ||
| try: | ||
| os.remove(DEFAULT_PROFILE_PATH) | ||
| except FileNotFoundError: |
| if sys.platform != "win32": | ||
| try: | ||
| os.killpg(proc.pid, signal.SIGKILL) | ||
| except ProcessLookupError: |
| os.killpg(proc.pid, signal.SIGKILL) | ||
| except ProcessLookupError: | ||
| proc.wait(timeout=10) | ||
| except subprocess.TimeoutExpired: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a fallback success criterion to
test/smoketest_pool_spawn.py: if scalene exits with a code other than the known-good0/1set, accept the run as long as scalene produced a valid profile JSON. Real crashes (no profile, or corrupt profile) still fail.The problem this addresses
On Windows, scalene's main process has been observed to finish writing the profile and then crash with
0xFFFFFFFF((uint)-1) during the multiprocessing resource-tracker shutdown after the spawn pool finishes. This happened on the failedsmoketests (windows-latest, 3.10)job on PR #1049 and historically on different Python versions across different runs (3.11 on the #1050 merge), which is the classic signature of a shutdown race rather than a version-specific bug. The test docstring already calls out this flake mode ("Most flake-prone: multiprocessing resource tracker + spawn workers + signal handlers all interact during shutdown.").With #1053 now in (
fail-fast: true), a single Windows flake cancels the whole matrix in seconds instead of running it to completion. So a robust Windows test matters more than it did before.What changed
Accept-list preserved.
rc == 0andrc == 1(Windows memoryview-cleanup warning) pass outright, matching the prior behavior on every OS where this test reliably passes today on master.Profile-validation fallback. Any other rc — kill code, timeout-reaped — passes iff
scalene-profile.jsonparses as JSON and carries scalene's structural keys (program,files,elapsed_time_sec). Deliberately does not assert non-emptysamples: sampling cadence vs. workload length is the timing sensitivity that creates the flake in the first place, so over-asserting there would just shift the failure mode.Profile path: scalene's default. A first pass of this change added
-o tempfile.gettempdir()/...to direct the output somewhere predictable. On Windows the runner's tempdir resolves toC:\Users\RUNNER~1\AppData\Local\Temp\...(the short-path form ofrunneradmin), and scalene didn't write there in CI even though local runs were fine. Sticking with scalene's defaultscalene-profile.jsonin cwd avoids that whole class of platform-specific path issue.stderr captured to a temp file, dumped on failure. Successful runs don't flood logs; failing runs include the last 4 KB of scalene's stderr for diagnostics.
What is not fixed
The underlying Windows shutdown race itself. That lives somewhere in the interaction between scalene's atexit handler, multiprocessing's resource-tracker, and the OS process model on Windows — a much bigger investigation. Until it's fixed, the test asserts the property it claims to assert (did spawn-mode Pool.map complete under scalene?) rather than the proxy property (did the interpreter exit cleanly?).
Test plan
python3 test/smoketest_pool_spawn.pyexits 0, scalene's profile written to the default path.🤖 Generated with Claude Code