Fix ToolTask EOF wait to be STA-safe via CountdownEvent (MSB4018 in AspNetCompiler)

[YuliiaKovalova]

Summary

Fixes an MSB4018 crash where ToolTask-derived tasks fail with:

System.NotSupportedException: WaitAll for multiple handles on a STA thread is not supported.
   at System.Threading.WaitHandle.WaitAll(...)
   at Microsoft.Build.Utilities.ToolTask.WaitForProcessExit(Process proc)

This reproduces with tasks that execute on an STA thread (e.g. AspNetCompiler via Microsoft.Web.Publishing.AspNetCompileMerge.targets).

Root cause

WaitForProcessExit (under the Wave18_6 change wave) waited for both the stdout and stderr EOF events with:

WaitHandle[] eofEvents = [_standardOutputEOF, _standardErrorEOF];
bool allEOFReceived = WaitHandle.WaitAll(eofEvents, TimeSpan.FromSeconds(eofTimeoutSec));

WaitHandle.WaitAll over multiple handles is not supported on STA threads and throws NotSupportedException, surfacing to the user as MSB4018.

Fix

Replace the two stdout/stderr ManualResetEvents with a single CountdownEvent initialized to a count of 2 (one per stream). Each stream signals it once when its AsyncStreamReader reaches EOF (Data=null), and WaitForProcessExit awaits it with CountdownEvent.Wait, which is STA-safe (it relies on a single-handle wait rather than WaitAll over multiple handles):

// init (under Wave18_6 guard)
_eofCountdown = new CountdownEvent(2); // one count each for stdout and stderr EOF

// EOF callback (once per stream)
lock (_eventCloseLock)
{
    if (!_eventsDisposed && _eofCountdown is { IsSet: false })
    {
        _eofCountdown.Signal();
    }
}

// wait
bool allEOFReceived = _eofCountdown.Wait(TimeSpan.FromSeconds(eofTimeoutSec));

The success/timeout semantics are unchanged: allEOFReceived is true only when both streams reached EOF within the timeout; on timeout the task still drains partial output and logs ToolTask.PipeEOFTimeout. Signaling and disposal remain serialized by _eventCloseLock, and the IsSet guard prevents over-signaling.

Validation

• Microsoft.Build.Utilities builds clean (0 warnings, 0 errors).
• Reviewed for races: CountdownEvent.Signal() cannot throw (serialized + IsSet guarded), no spurious 30s timeout on normal exit, no deadlock, and the Wave18_6-disabled path leaves _eofCountdown null and untouched.

[Copilot]

Pull request overview

This PR fixes an MSB4018 crash when ToolTask runs on an STA thread by avoiding WaitHandle.WaitAll (unsupported for multiple handles on STA) in the Wave18_6 WaitForProcessExit EOF-wait logic, while preserving the original bounded timeout behavior.

Changes:

• Replaces WaitHandle.WaitAll over stdout/stderr EOF events with sequential WaitOne calls (STA-safe) under Wave18_6.
• Preserves the same overall EOF timeout budget by tracking elapsed time and applying the remaining timeout to the second wait.

[github-actions]

MSBuild 24-Dimension Review — PR #13917

Fix summary: Replaces WaitHandle.WaitAll([stdout, stderr], 30s) with sequential WaitOne calls sharing a single Stopwatch-tracked budget. Correctly resolves NotSupportedException thrown on STA threads (.NET Framework).

#	Dimension	Verdict
4	Test Coverage	🟠 1 MAJOR
18	Documentation Accuracy	🔵 1 NIT

✅ 22/24 dimensions clean.

---

Key validations (all confirmed correct)

Semantics preservation (Dim 1, 22): The sequential approach with a pre-started Stopwatch is mathematically identical to WaitAll. If stdout signals at time T1, then remainingMs = 30s − T1, so stderr deadline = T1 + remainingMs = T0 + 30s — the same absolute deadline as WaitAll. Proof holds for all cases: both events already signaled, stdout timeout, stderr timeout, WaitOne(0) non-blocking poll.

Integer cast safety (Dim 22): (int)Math.Max(0, eofTimeoutMs − eofTimer.ElapsedMilliseconds) — value is provably in [0, 30000], so (int) cast is safe with no overflow path.

Thread safety (Dim 13): ManualResetEvent stays signaled once Set() is called. If stderr EOF fires before the second WaitOne, the poll returns true immediately regardless of remaining time. No race condition.

Allocation (Dim 3): Net improvement — removes the WaitHandle[] array, adds one Stopwatch instance. Negligible on a 30s-bounded code path.

ChangeWave (Dim 2): Bug fix within existing Wave18_6 gate — no new wave needed, both enabled/disabled paths preserved.

---

• Test Coverage (MAJOR) — add a [WindowsOnlyFact] that runs a ToolTask on an explicit STA thread and asserts NotSupportedException is not thrown
• Documentation (NIT) — clarify in the comment that the STA restriction applies to .NET Framework only

Note

🔒 Integrity filter blocked 2 items

The following items were blocked because they don't meet the GitHub integrity level.

• #13917 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #13917 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Expert Code Review (on open) for issue #13917 · ● 5.1M

[JanProvaznik]

creating an experimental insertion to manual test the aspcompiler scenario

[AlesProkop]

2026-06-04 Validation.

1. Created Web Forms Site using template, did not change anything.
2. Opened the site in the fixed VS code
3. Precompile during publishing checked
4. Publish to folder failed due to Circular file references are not allowed. unrelated
5. Second publish works without the AspNetCompiler issue.
   VS Version: 18.9.0 Canary [11903.26.main]