Skip to content

[tests] Don't crash macOS test runner on late log output - #26321

Merged
dalexsoto merged 2 commits into
mainfrom
dev/alex/fix-macos-test-log-stream-race
Jul 28, 2026
Merged

[tests] Don't crash macOS test runner on late log output#26321
dalexsoto merged 2 commits into
mainfrom
dev/alex/fix-macos-test-log-stream-race

Conversation

@dalexsoto

Copy link
Copy Markdown
Member

Summary

  • prevent late asynchronous log stream callbacks from writing after the diagnostic StreamWriter is closed
  • bound SIGINT, force-termination, and output-drain waits so cleanup cannot hang test jobs
  • report drain, flush, and disposal failures without failing an otherwise successful test run

Root cause

The macOS 27 test leg twice completed all suites successfully, then failed while shutting down diagnostic logging:

5 suites passed, 0 suites failed.
Unhandled exception. System.ObjectDisposedException: Cannot write to a closed TextWriter.

WaitForExit (10_000) waited for the process but not the asynchronous output handlers, so buffered DataReceived callbacks could arrive after writer disposal.

This was split from #26280 at reviewer request so the infrastructure fix can flow through main independently.

Validation

  • built scripts/run-packaged-macos-tests/run-packaged-macos-tests.csproj with 0 warnings and 0 errors
  • independent GPT-5.6 Sol and Claude Opus 4.8 max-reasoning concurrency reviews

dalexsoto and others added 2 commits July 28, 2026 02:32
…rives late

The 'arm64 - Mac Golden Gate (27) macOS tests' job failed twice on this PR even
though every test passed:

    5 suites passed, 0 suites failed.
    Wrote 792232215 bytes to .../DiagnosticReports/system.log
    Unhandled exception. System.ObjectDisposedException: Cannot write to a closed TextWriter.
       at System.IO.StreamWriter.WriteLine(String value)
       at Program.<>c__DisplayClass0_1.<<Main>$>b__11(Object _, DataReceivedEventArgs e)
       at System.Diagnostics.AsyncStreamReader.FlushMessageQueue(Boolean rethrowInNewThread)

We run 'log stream' with both stdout and stderr redirected to asynchronous
handlers that write to a StreamWriter. On shutdown we SIGINT it and call
WaitForExit (10_000), but that overload only waits for the process to exit, not
for the asynchronous handlers to finish delivering buffered output - the same
caveat ExecuteWithTimeout already documents. So a queued line could still be
delivered after the writer was disposed and throw on a thread pool thread,
which is unhandled and fails the job even when all the tests passed. The
several hundred megabytes of 'log stream' output make the window easy to hit
on a loaded bot.

Guard the writes with a flag set under the same lock the handlers already take,
so a late callback becomes a no-op instead of an unhandled exception, and stop
reading once we're done. Also drain the handlers before disposing so we don't
lose the tail of the log, but do it on a background thread with a timeout: the
parameterless WaitForExit () can block indefinitely if anything still holds the
redirected pipes, and hanging the job for hours would be worse than the crash
this fixes.

Note this is a test-infrastructure fix rather than a ScreenCaptureKit change.
It is included here because the bug reproducibly blocks this PR's CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ebf4376b-3b4c-4492-acef-18607e6f4ea1
Bound every log-stream shutdown step so test completion cannot hang indefinitely, and force-terminate the specific log process if SIGINT is ignored.

Report drain, flush, and disposal failures instead of silently truncating diagnostics or allowing cleanup exceptions to fail an otherwise successful test run.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f09fc2f9-a071-47c6-84ff-02aa45e9e0c0
Copilot AI review requested due to automatic review settings July 28, 2026 07:20
@dalexsoto
dalexsoto requested a review from rolfbjarne July 28, 2026 07:20
dalexsoto added a commit that referenced this pull request Jul 28, 2026
Move the macOS test-runner race fix to #26321 targeting main, as requested in review.

Keep this PR focused on the ScreenCaptureKit binding update; the infrastructure fix can flow from main to xcode27.0 independently if needed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f09fc2f9-a071-47c6-84ff-02aa45e9e0c0

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.

Pull request overview

This PR hardens the macOS packaged test runner’s diagnostic logging shutdown so that late asynchronous log stream output callbacks can’t crash the runner after all test suites have already completed.

Changes:

  • Prevent ObjectDisposedException by gating log stream output handlers on a logStreamWriterClosed flag synchronized via the writer lock.
  • Bound shutdown waits (SIGINT → timed wait → Kill fallback → bounded output drain) to avoid cleanup hangs.
  • Convert shutdown/flush/dispose failures into warnings so a successful test run doesn’t fail during teardown.

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

✅ API diff for current PR / commit

NET (empty diffs)

✅ API diff vs stable

NET (empty diffs)

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 0809d947500231dac78916349249ad5b7ecc6919 [PR build]

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

🚀 [CI Build #0809d94] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 203 tests passed 🎉

Tests counts

✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 5 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. Html Report (VSDrops) Download
✅ linker (iOS): All 15 tests passed. Html Report (VSDrops) Download
✅ linker (MacCatalyst): All 15 tests passed. Html Report (VSDrops) Download
✅ linker (macOS): All 21 tests passed. Html Report (VSDrops) Download
✅ linker (tvOS): All 15 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 19 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 18 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 19 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 19 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ sharpie: All 1 tests passed. Html Report (VSDrops) Download
✅ windows: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

macOS tests

✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Ventura (13): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sequoia (15): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Tahoe (26): All 5 tests passed. Html Report (VSDrops) Download

Linux Build Verification

Linux build succeeded

Pipeline on Agent
Hash: 0809d947500231dac78916349249ad5b7ecc6919 [PR build]

@dalexsoto
dalexsoto merged commit 57a4b79 into main Jul 28, 2026
57 checks passed
@dalexsoto
dalexsoto deleted the dev/alex/fix-macos-test-log-stream-race branch July 28, 2026 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants