From 48294142361891e3bc6f049eb3d8945db773fddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Sat, 16 May 2026 13:16:53 +0200 Subject: [PATCH 1/2] Address review feedback from PR #8263 - Fix doc comment in TrxTestResultExtractor: the serializer per-record cap is 64 MiB, not 16 MiB. - Apply the null-forgiving operator consistently on the bare `lastError` rethrows in `TrxResultStreamingStore.WriteRecordWithRetryAsync` so the nullable-flow analysis stays satisfied across all rethrow paths. - Switch the TRX streaming-store writer task from `Task.Run` to `ITask.RunLongRunning` so the `BlockingCollection.TryTake` poll loop no longer blocks a shared threadpool thread. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Streaming/TrxResultStreamingStore.cs | 9 ++++++--- .../Streaming/TrxTestResultExtractor.cs | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs b/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs index 9a514d8e13..c6a1202984 100644 --- a/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs +++ b/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs @@ -66,7 +66,10 @@ internal TrxResultStreamingStore(string filePath, IFileSystem fileSystem, ITask _logger = logger; _batchSize = batchSize; _flushIntervalMs = flushIntervalMs; - _writerTask = task.Run(WriteLoopAsync, CancellationToken.None); + // BlockingCollection.TryTake blocks the calling thread for up to _flushIntervalMs when + // the queue is idle. Running the writer on a dedicated long-running thread instead of the + // shared threadpool keeps it from starving threadpool consumers while it sleeps on the queue. + _writerTask = task.RunLongRunning(WriteLoopAsync, "TRX streaming store writer", CancellationToken.None); } /// @@ -357,7 +360,7 @@ private async Task WriteRecordWithRetryAsync(BinaryWriter writer, TrxTestResult } catch { - ExceptionDispatchInfo.Capture(lastError).Throw(); + ExceptionDispatchInfo.Capture(lastError!).Throw(); } try @@ -366,7 +369,7 @@ private async Task WriteRecordWithRetryAsync(BinaryWriter writer, TrxTestResult } catch (OperationCanceledException) { - ExceptionDispatchInfo.Capture(lastError).Throw(); + ExceptionDispatchInfo.Capture(lastError!).Throw(); } } diff --git a/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs b/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs index 5d15d51f14..9865f9b15b 100644 --- a/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs +++ b/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs @@ -10,7 +10,7 @@ namespace Microsoft.Testing.Extensions.TrxReport.Abstractions.Streaming; internal static class TrxTestResultExtractor { // Cap individual stdout/stderr/stack-trace fields when projecting into the streaming DTO. - // A single TRX result with multi-MB output trips the serializer's 16 MiB record cap (which exists + // A single TRX result with multi-MB output trips the serializer's 64 MiB record cap (which exists // to detect corruption), at which point ReadAll cannot continue past the offending record because // there is no sync marker. Truncating at the source preserves the rest of the run; downstream TRX // consumers (Azure DevOps, VS) also choke on multi-MB output fields. The chosen cap is well below From 6fbd7316b4a30c9945546cbc21e042b803ec26c4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 12:04:33 +0000 Subject: [PATCH 2/2] Keep TRX writer loop synchronous on long-running thread Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com> --- .../Streaming/TrxResultStreamingStore.cs | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs b/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs index c6a1202984..b488fd4293 100644 --- a/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs +++ b/src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs @@ -161,7 +161,8 @@ await _logger.LogWarningAsync( } } - private async Task WriteLoopAsync() +#pragma warning disable VSTHRD103 // The writer runs on a dedicated long-running thread; synchronous waits keep queue polling off the threadpool. + private Task WriteLoopAsync() { var batch = new List(_batchSize); try @@ -182,7 +183,7 @@ private async Task WriteLoopAsync() batch.Add(next); } - await WriteBatchAsync(batch).ConfigureAwait(false); + WriteBatch(batch); batch.Clear(); } } @@ -219,32 +220,29 @@ private async Task WriteLoopAsync() Interlocked.Add(ref _droppedCount, discarded); } - await _logger.LogErrorAsync( + _logger.LogErrorAsync( $"TRX streaming store writer faulted; intermediate file may be incomplete. {discarded} record(s) were dropped from the in-memory queue and will not appear in the TRX.", - ex).ConfigureAwait(false); + ex).GetAwaiter().GetResult(); } finally { try { - if (_fileStream is not null) - { - await _fileStream.Stream.FlushAsync().ConfigureAwait(false); - } + _fileStream?.Stream.Flush(); -#pragma warning disable VSTHRD103 // BinaryWriter / IFileStream do not implement IAsyncDisposable. _writer?.Dispose(); _fileStream?.Dispose(); -#pragma warning restore VSTHRD103 } catch (Exception ex) { - await _logger.LogErrorAsync("Failed to close TRX streaming store file.", ex).ConfigureAwait(false); + _logger.LogErrorAsync("Failed to close TRX streaming store file.", ex).GetAwaiter().GetResult(); } } + + return Task.CompletedTask; } - private async Task WriteBatchAsync(List batch) + private void WriteBatch(List batch) { EnsureFileOpen(); ApplicationStateGuard.Ensure(_writer is not null); @@ -262,7 +260,7 @@ private async Task WriteBatchAsync(List batch) long preRecordPosition = rawStream.Position; try { - await WriteRecordWithRetryAsync(_writer, batch[i], rawStream, preRecordPosition).ConfigureAwait(false); + WriteRecordWithRetry(_writer, batch[i], rawStream, preRecordPosition); written++; } catch (Exception ex) @@ -270,9 +268,9 @@ private async Task WriteBatchAsync(List batch) int unwritten = batch.Count - written; Interlocked.Add(ref _droppedCount, unwritten); - await _logger.LogErrorAsync( + _logger.LogErrorAsync( $"Failed to write TRX record {i + 1}/{batch.Count} after {MaxWriteRetries} retries; truncating to last good record. {unwritten} record(s) from this batch will not appear in the TRX.", - ex).ConfigureAwait(false); + ex).GetAwaiter().GetResult(); try { rawStream.Seek(preRecordPosition, SeekOrigin.Begin); @@ -301,9 +299,9 @@ await _logger.LogErrorAsync( Interlocked.Add(ref _droppedCount, additionalDropped); } - await _logger.LogErrorAsync( + _logger.LogErrorAsync( $"Failed to truncate TRX streaming store after write failure; marking store faulted. {additionalDropped} additional record(s) from the queue were dropped.", - truncEx).ConfigureAwait(false); + truncEx).GetAwaiter().GetResult(); return; } @@ -315,11 +313,11 @@ await _logger.LogErrorAsync( try { - await rawStream.FlushAsync().ConfigureAwait(false); + rawStream.Flush(); } catch (Exception ex) { - await _logger.LogErrorAsync("Failed to flush TRX streaming store; records remain in OS buffer.", ex).ConfigureAwait(false); + _logger.LogErrorAsync("Failed to flush TRX streaming store; records remain in OS buffer.", ex).GetAwaiter().GetResult(); } if (written > 0) @@ -328,7 +326,7 @@ await _logger.LogErrorAsync( } } - private async Task WriteRecordWithRetryAsync(BinaryWriter writer, TrxTestResult record, Stream rawStream, long preRecordPosition) + private void WriteRecordWithRetry(BinaryWriter writer, TrxTestResult record, Stream rawStream, long preRecordPosition) { // Mirrors the retry policy of TrxReportEngine.RetryWhenIOExceptionAsync but bounded so a // permanently broken file does not stall the writer indefinitely. Critically: each retry @@ -365,7 +363,7 @@ private async Task WriteRecordWithRetryAsync(BinaryWriter writer, TrxTestResult try { - await _task.Delay(TimeSpan.FromMilliseconds(RetryBaseDelayMs * attempt), _disposeCts.Token).ConfigureAwait(false); + _task.Delay(TimeSpan.FromMilliseconds(RetryBaseDelayMs * attempt), _disposeCts.Token).GetAwaiter().GetResult(); } catch (OperationCanceledException) { @@ -375,6 +373,7 @@ private async Task WriteRecordWithRetryAsync(BinaryWriter writer, TrxTestResult ExceptionDispatchInfo.Capture(lastError!).Throw(); } +#pragma warning restore VSTHRD103 // Must only be called from the writer thread. Lazy because most test runs may not produce results // before they hit cancellation/discovery; we don't want to provision a file we never use.