Repository navigation
Conversation
dotnet#134972 made SemaphoreSlim.Wait throw PlatformNotSupportedException even when uncontended, so these test classes were gated on IsMultithreadingSupported (commit cdc8d45) even though Roslyn's workspace semaphore usage here never actually blocks. dotnet#135335 fixes the uncontended case; once it merges, drop that condition. UpgradeToGeneratedRegexAnalyzerTests still needs outbound network access (resolving reference assemblies from nuget.org), unrelated to the semaphore bug, so give it its own WASI skip with an accurate reason instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @dotnet/area-extensions-configuration |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #135335 — do not merge before it. This PR by itself will fail WASI CI; the tests it un-gates need #135335's fix to stop hitting
PlatformNotSupportedException.Background
#134972 made
SemaphoreSlim.WaitthrowPlatformNotSupportedExceptionon single-threaded runtimes even when the semaphore is available and the call would return immediately without blocking. We worked around it incdc8d452ae8("Gate Roslyn workspace tests on IsMultithreadingSupported") by addingIsMultithreadingSupportedto theConditionalClasson 5 Roslyn-workspace-using test classes, since Roslyn's internal workspace locking usesSemaphoreSlim.PlatformDetection.IsMultithreadingSupportedmaps toRuntimeFeature.IsMultithreadingSupported, a real capability flag — WASI stays single-threaded, so it's stillfalseafter #135335. That PR only changes whether an uncontended synchronous wait throws, not whether real threading exists. So simply merging #135335 doesn't re-enable these tests; the gate itself still statically skips them.What this does
Validated on a local build of #135335 applied on top of upstream main, on the CoreCLR WASI interpreter lane, with the
IsMultithreadingSupportedcondition temporarily dropped from each class:ConfigurationBindingGeneratorTests(Microsoft.Extensions.Configuration.Binder)IncrementalTests(same project)EmitterTests(Microsoft.Extensions.Options, SourceGeneration.Unit.Tests)EmitterTests(Microsoft.Extensions.Options, SourceGenerationTests)UpgradeToGeneratedRegexAnalyzerTests(System.Text.RegularExpressions)The first four classes' Roslyn workspace usage in these scenarios never actually contends, so they pass once the uncontended-wait fix is in place. This PR drops
IsMultithreadingSupportedfrom theirConditionalClass(keepingHasAssemblyFileswhere present), relying on #135335 for correctness.UpgradeToGeneratedRegexAnalyzerTestsfails for an unrelated reason: it needs outbound network access to resolve reference assemblies fromapi.nuget.orgvia Roslyn'sReferenceAssemblies.ResolveAsync, which the WASI test sandbox doesn't provide (FatalProtocolExceptionwrapping a socketsPlatformNotSupportedException, plus a missing-HOMENuGet failure). It keepsHasAssemblyFilesand gets its ownSkipOnPlatform(TestPlatforms.Wasi, ...)with an accurate reason instead of the misleading semaphore comment.Validation
Build:
./build.sh -s clr+libs+host+packs -os wasi -arch wasm -rf CoreCLR -c Release, clean, 0 warnings/errors.Test command pattern:
./dotnet.sh build <project.csproj> /t:Test /p:TargetOS=wasi /p:TargetArchitecture=wasm /p:RuntimeFlavor=CoreCLR /p:Configuration=Release.This was run in a separate validation session, not in this PR's own checkout; the diff here was not itself built/tested against #135335 (doing so would require #135335 applied locally, which isn't appropriate for this branch). Re-validate once #135335 merges and this is rebased.
Note
This PR description was generated with GitHub Copilot.