Skip to content

Commit 6ccaf09

Browse files
ilonatommyilonatommy
authored
[release/9.0-staging][wbt] Prevent InvalidOperationException on TestOutputHelper logging. (#116916)
* Prevent InvalidOperationException when process output handlers try to write to xUnit TestOutputHelper after test context has expired. * Don't run WBT with NodeJS. * Prevent all other tests with template: "wasmconsole" from running as they also randomly timeout.
1 parent f956a80 commit 6ccaf09

File tree

13 files changed

+115
-397
lines changed

13 files changed

+115
-397
lines changed

‎eng/Versions.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@
263263
<MicrosoftSymbolStoreVersion>1.0.406601</MicrosoftSymbolStoreVersion>
264264
<!-- sdk version, for testing workloads -->
265265
<!-- <SdkVersionForWorkloadTesting>$(MicrosoftDotNetApiCompatTaskVersion)</SdkVersionForWorkloadTesting> -->
266-
<SdkVersionForWorkloadTesting>9.0.106</SdkVersionForWorkloadTesting>
266+
<SdkVersionForWorkloadTesting>9.0.107</SdkVersionForWorkloadTesting>
267267
<runtimewinx64MicrosoftNETCoreRuntimeWasmNodeTransportPackageVersion>9.0.0-alpha.1.24175.1</runtimewinx64MicrosoftNETCoreRuntimeWasmNodeTransportPackageVersion>
268268
<EmsdkPackageVersion>$(MicrosoftNETRuntimeEmscriptenVersion)</EmsdkPackageVersion>
269269
<NodePackageVersion>$(runtimewinx64MicrosoftNETCoreRuntimeWasmNodeTransportPackageVersion)</NodePackageVersion>

‎eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -233,17 +233,16 @@ jobs:
233233
publishArtifactsForWorkload: true
234234
publishWBT: true
235235

236-
# disabled: https://github.com/dotnet/runtime/issues/116492
237-
# - template: /eng/pipelines/common/templates/wasm-build-only.yml
238-
# parameters:
239-
# platforms:
240-
# - browser_wasm
241-
# - browser_wasm_win
242-
# nameSuffix: MultiThreaded
243-
# extraBuildArgs: /p:WasmEnableThreads=true /p:AotHostArchitecture=x64 /p:AotHostOS=$(_hostedOS)
244-
# condition: ne(variables['wasmMultiThreadedBuildOnlyNeededOnDefaultPipeline'], true)
245-
# publishArtifactsForWorkload: true
246-
# publishWBT: false
236+
- template: /eng/pipelines/common/templates/wasm-build-only.yml
237+
parameters:
238+
platforms:
239+
- browser_wasm
240+
- browser_wasm_win
241+
nameSuffix: MultiThreaded
242+
extraBuildArgs: /p:WasmEnableThreads=true /p:AotHostArchitecture=x64 /p:AotHostOS=$(_hostedOS)
243+
condition: ne(variables['wasmMultiThreadedBuildOnlyNeededOnDefaultPipeline'], true)
244+
publishArtifactsForWorkload: true
245+
publishWBT: false
247246

248247
# Browser Wasm.Build.Tests
249248
- template: /eng/pipelines/common/templates/browser-wasm-build-tests.yml

‎eng/pipelines/runtime-official.yml‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -364,25 +364,23 @@ extends:
364364
parameters:
365365
name: MonoRuntimePacks
366366

367-
# disabled: https://github.com/dotnet/runtime/issues/116492
368-
# - template: /eng/pipelines/common/platform-matrix.yml
369-
# parameters:
370-
# jobTemplate: /eng/pipelines/common/global-build-job.yml
371-
# buildConfig: release
372-
# runtimeFlavor: mono
373-
# platforms:
374-
# - browser_wasm
375-
# jobParameters:
376-
# templatePath: 'templates-official'
377-
# buildArgs: -s mono+libs+host+packs -c $(_BuildConfig) /p:WasmEnableThreads=true /p:AotHostArchitecture=x64 /p:AotHostOS=$(_hostedOS)
378-
# nameSuffix: Mono_multithread
379-
# isOfficialBuild: ${{ variables.isOfficialBuild }}
380-
# runtimeVariant: multithread
381-
# postBuildSteps:
382-
# - template: /eng/pipelines/common/upload-intermediate-artifacts-step.yml
383-
# parameters:
384-
# name: MonoRuntimePacks
385-
367+
- template: /eng/pipelines/common/platform-matrix.yml
368+
parameters:
369+
jobTemplate: /eng/pipelines/common/global-build-job.yml
370+
buildConfig: release
371+
runtimeFlavor: mono
372+
platforms:
373+
- browser_wasm
374+
jobParameters:
375+
templatePath: 'templates-official'
376+
buildArgs: -s mono+libs+host+packs -c $(_BuildConfig) /p:WasmEnableThreads=true /p:AotHostArchitecture=x64 /p:AotHostOS=$(_hostedOS)
377+
nameSuffix: Mono_multithread
378+
isOfficialBuild: ${{ variables.isOfficialBuild }}
379+
runtimeVariant: multithread
380+
postBuildSteps:
381+
- template: /eng/pipelines/common/upload-intermediate-artifacts-step.yml
382+
parameters:
383+
name: MonoRuntimePacks
386384

387385
# Build Mono AOT offset headers once, for consumption elsewhere
388386
#

‎eng/pipelines/runtime.yml‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -888,17 +888,16 @@ extends:
888888
publishArtifactsForWorkload: true
889889
publishWBT: true
890890

891-
# disabled: https://github.com/dotnet/runtime/issues/116492
892-
# - template: /eng/pipelines/common/templates/wasm-build-only.yml
893-
# parameters:
894-
# platforms:
895-
# - browser_wasm
896-
# - browser_wasm_win
897-
# condition: or(eq(variables.isRollingBuild, true), eq(variables.wasmSingleThreadedBuildOnlyNeededOnDefaultPipeline, true))
898-
# nameSuffix: MultiThreaded
899-
# extraBuildArgs: /p:WasmEnableThreads=true /p:AotHostArchitecture=x64 /p:AotHostOS=$(_hostedOS)
900-
# publishArtifactsForWorkload: true
901-
# publishWBT: false
891+
- template: /eng/pipelines/common/templates/wasm-build-only.yml
892+
parameters:
893+
platforms:
894+
- browser_wasm
895+
- browser_wasm_win
896+
condition: or(eq(variables.isRollingBuild, true), eq(variables.wasmSingleThreadedBuildOnlyNeededOnDefaultPipeline, true))
897+
nameSuffix: MultiThreaded
898+
extraBuildArgs: /p:WasmEnableThreads=true /p:AotHostArchitecture=x64 /p:AotHostOS=$(_hostedOS)
899+
publishArtifactsForWorkload: true
900+
publishWBT: false
902901

903902
# Browser Wasm.Build.Tests
904903
- template: /eng/pipelines/common/templates/browser-wasm-build-tests.yml

‎src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,7 @@ public static (int exitCode, string buildOutput) RunProcess(string path,
487487
_testOutput.WriteLine($"WorkingDirectory: {workingDir}");
488488
StringBuilder outputBuilder = new();
489489
object syncObj = new();
490+
bool isDisposed = false;
490491

491492
var processStartInfo = new ProcessStartInfo
492493
{
@@ -542,11 +543,13 @@ public static (int exitCode, string buildOutput) RunProcess(string path,
542543
using CancellationTokenSource cts = new();
543544
cts.CancelAfter(timeoutMs ?? s_defaultPerTestTimeoutMs);
544545

545-
await process.WaitForExitAsync(cts.Token);
546-
547-
if (cts.IsCancellationRequested)
546+
try
547+
{
548+
await process.WaitForExitAsync(cts.Token);
549+
}
550+
catch (OperationCanceledException)
548551
{
549-
// process didn't exit
552+
// process didn't exit within timeout
550553
process.Kill(entireProcessTree: true);
551554
lock (syncObj)
552555
{
@@ -560,6 +563,12 @@ public static (int exitCode, string buildOutput) RunProcess(string path,
560563
// https://learn.microsoft.com/dotnet/api/system.diagnostics.process.waitforexit?view=net-5.0#System_Diagnostics_Process_WaitForExit_System_Int32_
561564
process.WaitForExit();
562565

566+
// Mark as disposed before detaching handlers to prevent further TestOutput access
567+
lock (syncObj)
568+
{
569+
isDisposed = true;
570+
}
571+
563572
process.ErrorDataReceived -= logStdErr;
564573
process.OutputDataReceived -= logStdOut;
565574
process.CancelErrorRead();
@@ -573,17 +582,24 @@ public static (int exitCode, string buildOutput) RunProcess(string path,
573582
}
574583
catch (Exception ex)
575584
{
576-
_testOutput.WriteLine($"-- exception -- {ex}");
585+
// Mark as disposed before writing to avoid potential race condition
586+
lock (syncObj)
587+
{
588+
isDisposed = true;
589+
}
590+
TryWriteToTestOutput(_testOutput, $"-- exception -- {ex}", outputBuilder);
577591
throw;
578592
}
579593

580594
void LogData(string label, string? message)
581595
{
582596
lock (syncObj)
583597
{
598+
if (isDisposed)
599+
return;
584600
if (message != null)
585601
{
586-
_testOutput.WriteLine($"{label} {message}");
602+
TryWriteToTestOutput(_testOutput, $"{label} {message}", outputBuilder, label);
587603
}
588604
outputBuilder.AppendLine($"{label} {message}");
589605
}
@@ -627,6 +643,24 @@ public static string AddItemsPropertiesToProject(string projectFile, string? ext
627643
return projectFile;
628644
}
629645

646+
private static void TryWriteToTestOutput(ITestOutputHelper testOutput, string message, StringBuilder? outputBuffer = null, string? warningPrefix = null)
647+
{
648+
try
649+
{
650+
testOutput.WriteLine(message);
651+
}
652+
catch (InvalidOperationException)
653+
{
654+
// Test context has expired, but we still want to capture output in buffer
655+
// for potential debugging purposes
656+
if (outputBuffer != null)
657+
{
658+
string prefix = warningPrefix ?? "";
659+
outputBuffer.AppendLine($"{prefix}[WARNING: Test context expired, subsequent output may be incomplete]");
660+
}
661+
}
662+
}
663+
630664
public void Dispose()
631665
{
632666
if (_projectDir != null && _enablePerTestCleanup)

‎src/mono/wasm/Wasm.Build.Tests/Common/RunHost.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,6 @@ public enum RunHost
1515
Firefox = 8,
1616
NodeJS = 16,
1717

18-
All = V8 | NodeJS | Chrome//| Firefox//Safari
18+
All = V8 | Chrome//| NodeJS//Firefox//Safari
1919
}
2020
}

‎src/mono/wasm/Wasm.Build.Tests/Common/ToolCommand.cs‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,12 @@ public virtual void Dispose()
110110
}
111111

112112
protected virtual string GetFullArgs(params string[] args) => string.Join(" ", args);
113-
114113
private async Task<CommandResult> ExecuteAsyncInternal(string executable, string args)
115114
{
116115
var output = new List<string>();
117116
CurrentProcess = CreateProcess(executable, args);
118-
DataReceivedEventHandler errorHandler = (s, e) =>
117+
118+
void HandleDataReceived(DataReceivedEventArgs e, DataReceivedEventHandler? additionalHandler)
119119
{
120120
if (e.Data == null || isDisposed)
121121
return;
@@ -127,24 +127,13 @@ private async Task<CommandResult> ExecuteAsyncInternal(string executable, string
127127

128128
string msg = $"[{_label}] {e.Data}";
129129
output.Add(msg);
130-
_testOutput.WriteLine(msg);
131-
ErrorDataReceived?.Invoke(s, e);
130+
TryWriteToTestOutput(msg, output);
131+
additionalHandler?.Invoke(this, e);
132132
}
133-
};
134-
135-
DataReceivedEventHandler outputHandler = (s, e) =>
136-
{
137-
lock (_lock)
138-
{
139-
if (e.Data == null || isDisposed)
140-
return;
133+
}
141134

142-
string msg = $"[{_label}] {e.Data}";
143-
output.Add(msg);
144-
_testOutput.WriteLine(msg);
145-
OutputDataReceived?.Invoke(s, e);
146-
}
147-
};
135+
DataReceivedEventHandler errorHandler = (s, e) => HandleDataReceived(e, ErrorDataReceived);
136+
DataReceivedEventHandler outputHandler = (s, e) => HandleDataReceived(e, OutputDataReceived);
148137

149138
CurrentProcess.ErrorDataReceived += errorHandler;
150139
CurrentProcess.OutputDataReceived += outputHandler;
@@ -165,6 +154,20 @@ private async Task<CommandResult> ExecuteAsyncInternal(string executable, string
165154
string.Join(System.Environment.NewLine, output));
166155
}
167156

157+
private void TryWriteToTestOutput(string message, List<string> output)
158+
{
159+
try
160+
{
161+
_testOutput.WriteLine(message);
162+
}
163+
catch (InvalidOperationException)
164+
{
165+
// Test context may have expired, continue without logging to test output
166+
// Add a marker to the output buffer so we know this happened
167+
output.Add($"[{_label}] [WARNING: Test context expired, subsequent output may be incomplete]");
168+
}
169+
}
170+
168171
private Process CreateProcess(string executable, string args)
169172
{
170173
var psi = new ProcessStartInfo

‎src/mono/wasm/Wasm.Build.Tests/IcuShardingTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ public IcuShardingTests(ITestOutputHelper output, SharedBuildPerTestClassFixture
3535
.UnwrapItemsAsArrays();
3636

3737
[Theory]
38-
[MemberData(nameof(IcuExpectedAndMissingCustomShardTestData), parameters: new object[] { false, RunHost.NodeJS | RunHost.Chrome })]
39-
[MemberData(nameof(IcuExpectedAndMissingCustomShardTestData), parameters: new object[] { true, RunHost.NodeJS | RunHost.Chrome })]
38+
[MemberData(nameof(IcuExpectedAndMissingCustomShardTestData), parameters: new object[] { false, RunHost.Chrome })]
39+
[MemberData(nameof(IcuExpectedAndMissingCustomShardTestData), parameters: new object[] { true, RunHost.Chrome })]
4040
public void CustomIcuShard(BuildArgs buildArgs, string shardName, string testedLocales, bool onlyPredefinedCultures, RunHost host, string id) =>
4141
TestIcuShards(buildArgs, shardName, testedLocales, host, id, onlyPredefinedCultures);
4242

‎src/mono/wasm/Wasm.Build.Tests/IcuShardingTests2.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ public IcuShardingTests2(ITestOutputHelper output, SharedBuildPerTestClassFixtur
3232

3333

3434
[Theory]
35-
[MemberData(nameof(IcuExpectedAndMissingShardFromRuntimePackTestData), parameters: new object[] { false, RunHost.NodeJS | RunHost.Chrome })]
36-
[MemberData(nameof(IcuExpectedAndMissingShardFromRuntimePackTestData), parameters: new object[] { true, RunHost.NodeJS | RunHost.Chrome })]
35+
[MemberData(nameof(IcuExpectedAndMissingShardFromRuntimePackTestData), parameters: new object[] { false, RunHost.Chrome })]
36+
[MemberData(nameof(IcuExpectedAndMissingShardFromRuntimePackTestData), parameters: new object[] { true, RunHost.Chrome })]
3737
public void DefaultAvailableIcuShardsFromRuntimePack(BuildArgs buildArgs, string shardName, string testedLocales, RunHost host, string id) =>
3838
TestIcuShards(buildArgs, shardName, testedLocales, host, id);
3939
}

‎src/mono/wasm/Wasm.Build.Tests/IcuTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ public IcuTests(ITestOutputHelper output, SharedBuildPerTestClassFixture buildCo
3737
.UnwrapItemsAsArrays();
3838

3939
[Theory]
40-
[MemberData(nameof(FullIcuWithInvariantTestData), parameters: new object[] { false, RunHost.NodeJS | RunHost.Chrome })]
41-
[MemberData(nameof(FullIcuWithInvariantTestData), parameters: new object[] { true, RunHost.NodeJS | RunHost.Chrome })]
40+
[MemberData(nameof(FullIcuWithInvariantTestData), parameters: new object[] { false, RunHost.Chrome })]
41+
[MemberData(nameof(FullIcuWithInvariantTestData), parameters: new object[] { true, RunHost.Chrome })]
4242
public void FullIcuFromRuntimePackWithInvariant(BuildArgs buildArgs, bool invariant, bool fullIcu, string testedLocales, RunHost host, string id)
4343
{
4444
string projectName = $"fullIcuInvariant_{fullIcu}_{invariant}_{buildArgs.Config}_{buildArgs.AOT}";
@@ -60,8 +60,8 @@ public void FullIcuFromRuntimePackWithInvariant(BuildArgs buildArgs, bool invari
6060
}
6161

6262
[Theory]
63-
[MemberData(nameof(FullIcuWithICustomIcuTestData), parameters: new object[] { false, RunHost.NodeJS | RunHost.Chrome })]
64-
[MemberData(nameof(FullIcuWithICustomIcuTestData), parameters: new object[] { true, RunHost.NodeJS | RunHost.Chrome })]
63+
[MemberData(nameof(FullIcuWithICustomIcuTestData), parameters: new object[] { false, RunHost.Chrome })]
64+
[MemberData(nameof(FullIcuWithICustomIcuTestData), parameters: new object[] { true, RunHost.Chrome })]
6565
public void FullIcuFromRuntimePackWithCustomIcu(BuildArgs buildArgs, bool fullIcu, RunHost host, string id)
6666
{
6767
string projectName = $"fullIcuCustom_{fullIcu}_{buildArgs.Config}_{buildArgs.AOT}";

0 commit comments

Comments
 (0)