Make the upgrade-wrapper E2E marker read tolerant of the detached writer#448
Merged
Conversation
Wrapper_DetachedTask_RunsAsSystemAfterWrapperExits intermittently failed with "The process cannot access the file ... because it is being used by another process". The detached Task Scheduler inner script writes the identity marker asynchronously; the test waits for the file to EXIST, then immediately File.ReadAllText'd it — which opens with FileShare.Read and collides with the writer's still-open handle (and could also read a partial/empty file mid-write). Replace the read with ReadMarkerWhenReady: open the marker with FileShare.ReadWrite (don't contend on the writer's handle) and poll until the content is non-empty, within a bounded 30s timeout with an actionable failure naming the marker path. The other marker-based tests only assert existence, so only this content-read needed the fix. Test-fixture race only — the marker is a test artifact; production reads upgrade traces from the agent over Halibut RPC, not via a concurrent local file read, so no production upgrade-wrapper code changes. Windows-only E2E (gated behind OperatingSystem.IsWindows()); the project compiles clean cross-platform.
The detached-task identity assertions read the marker the inner script writes asynchronously. Polling until merely non-empty leaves a narrow window where a torn/partial read could return content that then fails the 'system' assertions. Have ReadMarkerWhenReady accept an optional readiness predicate (default: non-empty) and pass one requiring the well-formed identity, so a partial read is retried within the bounded timeout instead of returned. Clarify the doc-comment that the retry loop — not FileShare.ReadWrite — is the load-bearing defence.
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.
Summary
Wrapper_DetachedTask_RunsAsSystemAfterWrapperExitsintermittently failed withIOException: ... because it is being used by another process(seen once on PR Wire disk self-heal into K8s-agent local-exec mode #440 CI, passed on re-run). The detached Task Scheduler inner script writes the identity marker asynchronously; the test waited for the file to exist, then immediatelyFile.ReadAllText'd it — which opens withFileShare.Readand collides with the writer's still-open handle (and could also read a partial/empty file mid-write).ReadMarkerWhenReady: open the marker withFileShare.ReadWrite(don't contend on the writer's handle) and poll until the content is non-empty, within a bounded 30s timeout with an actionable failure message naming the marker path.Scope
File.ReadAllText, so the race does not exist in production code (verified).OperatingSystem.IsWindows()).Test plan
Squid.WindowsTentacleE2ETestscompiles clean cross-platform (the test body skips on non-Windows).WindowsTentacle*E2E) exercises the real path; the fix removes the file-handle contention that caused the intermittentIOException.