Repository navigation
fix(conformance): clean up after an unexpected failure; stop kill hitting the process group - #102
Merged
Merged
Conversation
… hitting the process group
Three defects in the batteries' own housekeeping.
fs-conformance.ps1 runs with ErrorActionPreference = Stop and had no handler
around its body, so anything unexpected outside a Check - a missing binary, a
path that vanished - terminated the script with the drive letter still mounted
and the agent still running. The next run then could not take that letter. A
`trap` now reports the error, runs Cleanup and exits 97; it covers the whole
body without wrapping and re-indenting it.
fs-conformance.sh expanded its pids as "${AGENT_PID:-0}". A bare 0 is not a
harmless default: `kill 0` signals the entire process group, which is the shell
running the script and, in CI, the step around it. Both pids are now guarded and
killed only when actually set.
The `content matches` check copied through "$WORK/../cmp.bin" - deliberately
outside the served tree, since writing inside it would show up on the mount and
disturb later checks, but that also put it outside everything cleanup removes,
so every run left a file in /tmp. It now uses its own scratch directory, which
cleanup takes with it.
Verified: the Windows battery still passes 28/0, and with a failure injected
after the mount the trap fires, exits 97, and leaves no mounted drive and no
wsldrive processes behind. The Linux battery passes 31/0 and leaves no
/tmp/cmp.bin and no stray temp directories.
The performance notes on the same issue (per-op allocations in fuse_mount.cpp,
select-per-frame, Stats counters as atomics) are deliberately not in here: they
were measured at under 3% and are a separate change.
Closes #78
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Closes #78. Three defects in the batteries' own housekeeping.
kill 0signals the process group.fs-conformance.shexpanded its pids as"\0". A bare0is not a harmless default —kill 0signals the whole process group, which is the shell running the script and, in CI, the step around it. Both pids are now guarded and killed only when set.No cleanup on an unexpected error.
fs-conformance.ps1runs withErrorActionPreference = Stopand had nothing around its body, so anything unexpected outside aCheckleft the drive letter mounted and the agent running — and the next run then could not take that letter. Atrapreports the error, runsCleanupand exits 97. It covers the whole body without wrapping and re-indenting it.A file left in /tmp every run. The
content matchescheck copied through\/../cmp.bin, outside the served tree on purpose (writing inside it would show up on the mount and disturb later checks) — but that also put it outside everything cleanup removes. It now has its own scratch directory, which cleanup takes with it.Verification
/tmp/cmp.binor stray temp directories afterwards.The performance notes on the same issue (per-op allocations, select-per-frame,
Statscounters as atomics) are deliberately not here — measured at under 3%, and a separate change.🤖 Generated with Claude Code