Skip to content

Fix Text file busy race for fake executables in Infrastructure.Tests - #21749

Merged
Jose Perez Rodriguez (joperezr) merged 1 commit into
mainfrom
aspireagent/infra-tests-etxtbsy
Nov 5, 2026
Merged

Jose Perez Rodriguez (joperezr) merged 1 commit into
mainfrom
aspireagent/infra-tests-etxtbsy

Conversation

@davidfowl

Copy link
Copy Markdown
Collaborator

Supersedes #21558. That PR's head is on a fork (davidfowl/aspire) with maintainer_can_modify=false, so it couldn't be brought up to date with main. This PR contains the same change, squashed onto current main.

Fixes #21557

Failure

On main, Infrastructure.Tests.AnalyzeCiFailureWorkflowTests.LastSuccessfulMainRunFallsBackToOlderWindow failed with Expected: 0 / Actual: 126 (run 40984409791, Infrastructure (ubuntu-latest)).

Denominator: 1 failure in 115 attempt-1 Infrastructure (ubuntu-latest) jobs in the last ~48h of ci.yml runs (104 success, 3 failure, 8 cancelled). The other two failures were unrelated, PR-specific breaks.

Root cause

This is the ETXTBSY ("Text file busy") race that #21448 fixes for Aspire.Acquisition.Tests (#18732). This PR fixes it in tests/Infrastructure.Tests, which #21448 doesn't touch.

  1. Several tests write a stub executable (gh, actionlint, curl, dotnet.sh) from the test host with File.WriteAllText[Async], then chmod it and run a script that execs it.
  2. xUnit runs test classes in parallel, so other tests in the same host keep starting processes.
  3. A child forked while the host has the stub open for writing inherits that descriptor until it calls exec.
  4. Linux refuses to exec a file that any process has open for writing, returning ETXTBSY. bash reports that as exit 126.

Writing to a temp file and renaming it doesn't help, because the inherited descriptor still refers to the same inode.

Fix

  • New ExecutableFile.WriteAsync/Write test helper. On Unix it writes the contents (from stdin) and sets the mode inside a short-lived /bin/sh -c 'cat > "$1" && chmod "$2" "$1"' child, so the test host never holds a writable descriptor that another fork could inherit. On Windows it is a plain File.WriteAllTextAsync.
  • Every Infrastructure.Tests call site that execs a freshly written stub now uses the helper:
    • AnalyzeCiFailureWorkflowTests
    • LintHandwrittenWorkflowsScriptTests
    • NixCliPackageTests
    • TemplateManifestGenerationTests
    • ExtensionReleaseWorkflowTests (the separate chmod +x step is removed)
  • BuildWarningPolicyTests is unchanged because it runs bash build.sh rather than exec'ing the file.
  • No assertions are changed, and no sleeps or retries are added.

Regression test

ExecutableFileTests.WrittenExecutableCanBeExecutedImmediatelyWhileOtherThreadsStartProcesses (Unix only):

  • Four threads continuously fork /bin/sh -c "exit 0".
  • Meanwhile, 1000 stubs are written with the helper and exec'd immediately.
  • The test asserts that no launch fails and that each stub returns its expected exit code.

WritesContentsAndExecutableMode checks that the contents and mode round-trip.

Helper's Unix path Test runs failed Launches failed
Before: in-process File.WriteAllTextAsync + File.SetUnixFileMode 20 / 20 561 / 20,000 (18–35 per run, all Text file busy)
After: this PR 0 / 20 0 / 20,000

Change since #21558

main moved AnalyzeCiFailureWorkflowTests.cs into the new tests/AnalyzeCiFailure.Tests project. The helper call moved with it, so ExecutableFile.cs is now linked into that project (<Compile Include="../Infrastructure.Tests/ExecutableFile.cs" Link="shared/ExecutableFile.cs" />). This uses the same pattern the project already uses for RepoRoot.cs and CommandResult.cs. Without the link the project fails to build with CS0103.

Confidence

  • Behavior and regression coverage: High. ExecutableFileTests reproduces the race: the old write path failed in 20 of 20 runs, and the new helper failed in 0 of 20. No assertions changed in the converted tests.
  • Tests and pipelines run: Ran locally on Linux against this commit:
    • AnalyzeCiFailure.Tests: 1162 of 1163 passed. The one failure, AnalyzerBuildsFromRealSparseCheckoutWithoutHostileParentImports, comes from NuGet's vulnerability check (audit) being unable to reach nuget.org from the local machine (NU1900/NU1905), not from this change.
    • Infrastructure.Tests classes this PR touches, all passing: ExecutableFile 2/2, NixCliPackage 3/3, TemplateManifestGeneration 7/7, ExtensionReleaseWorkflow 9/9, LintHandwrittenWorkflowsScript 3/3.
    • CI is pending.
  • Build artifacts: Not affected. Only test code and a test .csproj change; no product or packaging changes.
  • E2E: Not applicable. This is test-infrastructure only.

How we stop this recurring: New tests that exec a freshly written stub should write it with ExecutableFile.WriteAsync instead of File.WriteAllText followed by chmod. The regression test guards the helper itself.

🤖 Posted by AspireAgent (AI Agent)

Write fake executables (stub gh, actionlint, curl, dotnet.sh) from a short-lived
/bin/sh child so the test host never holds a writable descriptor that a
concurrently forked child could inherit, which makes exec fail with ETXTBSY
(exit 126). Link the helper into AnalyzeCiFailure.Tests, which now hosts
AnalyzeCiFailureWorkflowTests.

Supersedes #21558. Fixes #21557.

Co-authored-by: David Fowler <davidfowl@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 21749

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 21749"

@aspire-repo-bot
aspire-repo-bot Bot requested a balanced review from Copilot November 4, 2026 04:03
@github-actions github-actions Bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Tests selector

2 / 101 PR test projects · 0 PR jobs, from 8 changed files.

Selected PR test projects (2 / 101)

AnalyzeCiFailure.Tests, Infrastructure.Tests

Selected PR jobs (0)

none


How these were chosen — grouped by what changed

🧪 tests/AnalyzeCiFailure.Tests/AnalyzeCiFailure.Tests.csproj (changed test)
→ 1 directly: AnalyzeCiFailure.Tests

🧪 tests/AnalyzeCiFailure.Tests/AnalyzeCiFailureWorkflowTests.cs (changed test)
→ 1 directly: AnalyzeCiFailure.Tests

🧪 tests/Infrastructure.Tests/ExecutableFile.cs (changed test)
→ 1 directly: Infrastructure.Tests

🧪 tests/Infrastructure.Tests/ExecutableFileTests.cs (changed test)
→ 1 directly: Infrastructure.Tests

🧪 tests/Infrastructure.Tests/Pipelines/NixCliPackageTests.cs (changed test)
→ 1 directly: Infrastructure.Tests

🧪 tests/Infrastructure.Tests/TemplateManifest/TemplateManifestGenerationTests.cs (changed test)
→ 1 directly: Infrastructure.Tests

🧪 tests/Infrastructure.Tests/WorkflowScripts/ExtensionReleaseWorkflowTests.cs (changed test)
→ 1 directly: Infrastructure.Tests

🧪 tests/Infrastructure.Tests/WorkflowScripts/LintHandwrittenWorkflowsScriptTests.cs (changed test)
→ 1 directly: Infrastructure.Tests

Job reasons

none


Selection computed for commit ec3f028.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain for the test-infrastructure changes addressing the executable-writing race.

No open issues 🎉

What changed in this PR

Addresses the Unix “Text file busy” race in test-created executables by moving file writing and permission setting into a child process. The change is limited to test infrastructure; product and packaging behavior are unchanged.

Changes:

  • Adds a shared executable-writing helper with a plain file-write path on Windows.
  • Migrates fake executable creation across infrastructure and CI-analysis tests, linking the helper into the separate analysis test project.
  • Adds concurrent-process regression coverage and content/permission round-trip checks.
File Description
tests/​Infrastructure.Tests/​WorkflowScripts/​LintHandwrittenWorkflowsScriptTests.cs Uses the helper to create the fake actionlint executable.
tests/​Infrastructure.Tests/​WorkflowScripts/​ExtensionReleaseWorkflowTests.cs Migrates fake gh creation and removes the separate chmod runner.
tests/​Infrastructure.Tests/​TemplateManifest/​TemplateManifestGenerationTests.cs Uses the synchronous helper for the Unix dotnet.sh stub.
tests/​Infrastructure.Tests/​Pipelines/​NixCliPackageTests.cs Migrates both fake curl executables while preserving permissions.
tests/​Infrastructure.Tests/​ExecutableFileTests.cs Adds concurrent-launch regression and content/permission tests.
tests/​Infrastructure.Tests/​ExecutableFile.cs Adds shared synchronous and asynchronous executable-writing helpers.
tests/​AnalyzeCiFailure.Tests/​AnalyzeCiFailureWorkflowTests.cs Routes fake executable creation through the shared helper.
tests/​AnalyzeCiFailure.Tests/​AnalyzeCiFailure.Tests.csproj Links the helper into the CI-analysis test project.

Review effort: Balanced

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@davidfowl David Fowler (davidfowl) removed the agent:working An AI agent is actively working on this issue or PR label Nov 4, 2026
@davidfowl
David Fowler (davidfowl) marked this pull request as ready for review November 4, 2026 04:34

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving Unix executable-fixture writing into a child process addresses the writable-descriptor inheritance race without changing the fake scripts or their expected results. Existing argument, exit-code, and output assertions remain, and the added regression still requires the intended nonzero stub exit. No product behavior or failure expectation is relaxed.

@joperezr
Jose Perez Rodriguez (joperezr) merged commit 87ef7df into main Nov 5, 2026
98 of 101 checks passed
@microsoft-github-policy-service microsoft-github-policy-service Bot added this to the 17.0 milestone Nov 5, 2026
@aspire-repo-bot

Copy link
Copy Markdown
Contributor

✅ No documentation update needed.

docs_optional → test_only (proven false positive). Triggered signal: pr_body_has_security_marker — evidence is PR-body prose about NuGet's vulnerability audit being unable to reach nuget.org locally, not a security change. All 8 changed files match tests/** (AnalyzeCiFailure.Tests, Infrastructure.Tests); only_test_or_build_changes is true. No user-facing change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI Failure] Flaky: Infrastructure.Tests fake executables fail with exit 126 (Text file busy) – LastSuccessfulMainRunFallsBackToOlderWindow

3 participants