[quality-improver] Browser/WASM Blocking-Wait Contract Compliance: FileLoggerProvider.Dispose() IsBrowser() branch still blocks

Start in src/Platform/Microsoft.Testing.Platform/Logging/FileLoggerProvider.cs around lines 229-246, where the OperatingSystem.IsBrowser() branch calls the synchronous FileLogger.Dispose(). Compare it with FileLogger.DisposeAsync() in FileLogger.cs (around lines 430-470) and with DisposeHelper.DisposeAsync. The issue leaves open whether to delete the branch or add a test, so confirm the maintainers' choice first. Done means the chosen contract is applied and a test in test/UnitTests/Microsoft.Testing.Platform.UnitTests/Logging/ locks it in.

🎯 Repository Quality Improvement Report — Browser/WASM Blocking-Wait Contract Compliance

Analysis Date: 2026-10-14
Focus Area: Single-threaded WASM blocking-call contract compliance
Strategy Type: Custom

Executive Summary

Microsoft.Testing.Platform explicitly supports running on single-threaded WebAssembly
runtimes (browser-wasm / wasi-wasm) and documents a strict contract for it:
RuntimeFeatureHelper.IsMultiThreaded is "the single condition to branch on" before any
blocking wait or thread-creation primitive, because a blocking wait on the sole WASM
thread either deadlocks or throws PlatformNotSupportedException. Several commits in
docs/Changelog-Platform.md (TRX streaming store, slow-test reporters, Azure DevOps
report) show the team repeatedly fixing exactly this class of bug for other components.

Auditing every .Wait() / GetAwaiter().GetResult() / SemaphoreSlim.Wait() call site
reachable from the core Microsoft.Testing.Platform assembly (which does ship a
browser/wasi-capable build) turned up one remaining inconsistency:
FileLoggerProvider.Dispose() carries a dedicated OperatingSystem.IsBrowser() branch
that is clearly intended to special-case the single-threaded runtime, but the branch
still calls the fully synchronous FileLogger.Dispose(), which blocks on
_logLoop.Wait(_flushTimeout) whenever SyncFlush is false (the only value legal on
browser, since the constructor throws PlatformNotSupportedException for
SyncFlush: true on browser). In other words, the one branch written specifically to
handle IsBrowser() routes straight into the one blocking-wait path the class exists to
avoid on that runtime, instead of into the already-implemented async drain
(FileLogger.DisposeAsync()).

In production this branch is not currently exercised: every live caller
(CommonTestHost, TestApplication, TestApplicationBuilder) disposes
FileLoggerProvider through DisposeHelper.DisposeAsync, which prefers
IAsyncDisposable.DisposeAsync() over IDisposable.Dispose() on NETCOREAPP targets.
That makes this a latent trap rather than an active incident: any future caller that
disposes a FileLoggerProvider synchronously (a unit test, a new shutdown path, or a
refactor that narrows the static type to IDisposable) will reintroduce the exact
PlatformNotSupportedException class the team has fixed repeatedly elsewhere, and will
do so inside code that looks — from its IsBrowser() check — like it was already
handling that case correctly.

🤖 Suggested Improvement Tasks

Task 1: Remove or correct the dead/dangerous IsBrowser() branch in FileLoggerProvider.Dispose()

Priority: Medium
Estimated Effort: Small

In src/Platform/Microsoft.Testing.Platform/Logging/FileLoggerProvider.cs, the
IsBrowser() branch in Dispose() (lines 231-235) calls the synchronous
FileLogger.Dispose(), which blocks on _logLoop.Wait(...) — a call that is only safe
to make when SyncFlush is true, but SyncFlush: true is rejected with
PlatformNotSupportedException on browser at FileLoggerOptions construction time.
Since DisposeHelper.DisposeAsync (the sole production caller) already prefers
FileLoggerProvider.DisposeAsync() over Dispose() whenever the object implements
IAsyncDisposable on NETCOREAPP, the synchronous Dispose() method — and its
IsBrowser() special case — is not reached by any current caller. Either:
(a) remove the special case and leave a comment documenting that Dispose() is only
expected to be called on non-wasm targets (reachable guarantee enforced by callers), or
(b) add a regression test that constructs a FileLoggerProvider with SyncFlush: false
and directly calls the synchronous Dispose() under a simulated OperatingSystem.IsBrowser()
to prove the fix, if the team wants Dispose() itself to remain safe to call on wasm.
Add or update a unit test in
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Logging/ to lock in whichever
contract is chosen so the inconsistency cannot silently return.

Task 2: Add a BannedSymbols/analyzer-style trap for "blocking wait reachable from a wasm-capable project without a guard"

Priority: Low
Estimated Effort: Medium

This audit required a manual, project-by-project walk of every .Wait(),
GetAwaiter().GetResult(), and SemaphoreSlim.Wait() call site in the ~10 projects that
declare <SupportedPlatform Include="browser" /> / wasi, cross-referencing each
against [UnsupportedOSPlatform] attributes, #if conditionals, and
RuntimeFeatureHelper.IsMultiThreaded checks. A lightweight Roslyn analyzer (or an
addition to the existing internal analyzer suite) that flags a blocking wait inside a
type/member reachable on browser/wasi without a corresponding guard would convert
this one-off manual pass into a standing CI check, preventing regressions like the one
in Task 1 and catching any future additions to the ~10 wasm-capable extension projects.

Task 3: Document the "browser-safe disposal" contract alongside RuntimeFeatureHelper.IsMultiThreaded

Priority: Low
Estimated Effort: Small

RuntimeFeatureHelper.IsMultiThreaded's XML doc already explains the two established
fallbacks for scheduling work (skip vs. run inline), but does not explicitly call out
disposal/teardown code as a third category that needs the same discipline — Dispose()
methods are a common place for a blocking wait to be added without the author
remembering the wasm constraint, precisely because IDisposable.Dispose() itself is
synchronous and "just block briefly" feels natural. Add a short <para> to the existing
remarks (or a new <see cref="IAsyncDisposable"/> cross-reference) noting that
Dispose() implementations reachable on single-threaded wasm must either not block, or
must only be reached when DisposeAsync()/IAsyncDisposable is also implemented and
guaranteed to be preferred by every caller (as DisposeHelper.DisposeAsync does).

📊 Historical Context

🎯 Recommendations

Immediate Actions (This Week)

1. Delete or correct the IsBrowser() branch in FileLoggerProvider.Dispose() — Priority: Medium

Short-term Actions (This Month)

1. Consider an analyzer rule for guard-free blocking waits in wasm-capable projects — Priority: Low
2. Extend RuntimeFeatureHelper.IsMultiThreaded docs to call out disposal code explicitly — Priority: Low

Next analysis: 2026-10-15 — Focus area selected based on diversity algorithm

🤖 Automated content by GitHub Copilot. Generated by the Repository Quality Improver workflow. · copilot · auto · 411.4 AIC · ⌖ 9.22 AIC · ⊞ 17.5K · [◷]( · ◷)

gh aw add githubnext/agentics/workflows/repository-quality-improver.md@main

• expires on Oct 16, 2026, 10:38 PM UTC