Repository navigation
Fix ambiguous SDK uninstall matching and missing --install-path - #270
Merged
Merged
Conversation
Two related dotnetup SDK uninstall bugs: - FindTrackedSdkSpec now cross-checks sibling installations' SdkFeatureBand so a coarse channel spec (e.g. 11.0.1xx) that could resolve to more than one sibling prerelease SDK (e.g. preview.7 vs rc.2) is treated as ambiguous and blocks the uninstall instead of guessing, with an updated alert explaining why and pointing at Untrack as the safe path. - DotnetUpArguments.SdkUninstall/RuntimeUninstall (and the DotnetUpService/ DotnetSdk.razor call sites) never passed --install-path, so uninstall requests silently targeted dotnetup's own hardcoded default root instead of wherever the tracked spec actually lives. Threaded the spec's own InstallRoot through to fix this. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sherpa was scanning every installed SDK's workload state concurrently and unthrottled (Task.WhenAll over all targets) whenever the workload inventory refreshed, e.g. on app startup. Logs confirmed a `dotnet workload list --machine-readable` invocation for feature band 11.0.100-rc.1 stalled past the hard 45s timeout during a burst of five simultaneous `dotnet` invocations right after launch, even though the exact same command completed in ~1s when run in isolation with the same DOTNET_ROOT/PATH. Concurrent `dotnet` CLI invocations can contend for shared per-user resources (NuGet HTTP cache locks, first-run experience markers, etc.), which is consistent with one invocation serializing behind the others long enough to hit the timeout. Limit concurrent inventory reads to 2 in flight at a time, following the existing SemaphoreSlim throttling pattern already used in PublishProfileService. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Navigating to Doctor could log an unhandled JS interop exception
("JS object instance with ID 1 does not exist (has it been
disposed?).") during component disposal. The failing call originated
from OnAfterRenderAsync's first-render ensureStylesheets/bindPopover
JS interop, which had no guard against the component being disposed
while an await was still in flight -- unlike DisposeAsync, which
already handles this exact JSException/JSDisconnectedException
pattern.
Add a _disposed flag checked before and after each JS interop await
in OnAfterRenderAsync, and wrap the method body in the same
disposed-object exception handling already used in DisposeAsync, so a
disposal race no longer surfaces as an unhandled exception.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Clicking Fix on a "Workload Set" update (or a version tag) could appear to do nothing. The app log showed why: resolving the workload update request runs several `dotnet workload search version` invocations that can take tens of seconds in a contended/slow dotnet environment, and nothing disabled the Fix button or gave feedback while that ran. A user who thought their click hadn't registered would click again, and the second call raced ProcessModalService's single in-flight guard, throwing "A process modal is already being shown" -- as an unobserved task exception with no visible symptom. - Doctor.razor: FixDependency now guards re-entrancy and immediately shows the loading state (matching the existing full-scan behavior) before dispatching to any fix path, so a slow fix visibly runs and can't be double-clicked into this race. UpdateToVersion (also reachable directly from the version-tag click handlers) sets the same loading state, and those version tags are now disabled/inert while a fix is in progress. Also catch the "already showing" InvalidOperationException there so, if it does happen, it surfaces as a toast instead of an unobserved exception. - ProcessModalService.ShowProcessAsync: wrap the body in try/finally so IsVisible/CurrentRequest are always released even if pushing the native modal page or awaiting completion throws, so a single failure can't permanently wedge every future Fix click behind this guard. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
GetInstalledSdkVersions treated any version-named folder under sdk/ as an installed SDK. Leftovers from interrupted installs or partial uninstalls were therefore reported as installed. Only count a folder when it contains dotnet.dll, dotnet.runtimeconfig.json and Sdks/. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
Why
Uninstalling a tracked .NET SDK channel (e.g.
11.0.1xx) via dotnetup could fail or, worse, target the wrong install root or the wrong SDK entirely, depending on what else was installed:--install-path, so they silently used dotnetup's own hardcoded default install root instead of the root the tracked spec actually lives in. If the user's real SDKs live at a different (e.g. user-configured default) install path, the uninstall command fails with "No .NET SDK install spec found", even though the spec clearly exists.11.0.1xxcan match more than one installed prerelease SDK sharing that coarse version (e.g. apreview.7build alongside anrc.2build). The existing matching logic only compared major/minor/patch-hundreds and ignored prerelease labels, so it could silently resolve to the wrong sibling SDK.While testing this, several related Doctor / SDK management issues turned up and are fixed here too (see below).
Approach
DotnetSdkPresentationBuilder.FindTrackedSdkSpecnow accepts the full list of installations and cross-checks each sibling'sSdkFeatureBand(already used elsewhere for workload isolation). If a coarse spec could ambiguously resolve to siblings with different prerelease bands, it returns null instead of guessing, andDotnetSdk.razorshows an alert directing the user to use Untrack instead.DotnetUpArguments.SdkUninstall/RuntimeUninstallnow accept an optionalinstallPathand emit--install-pathwhen provided.DotnetUpServiceand theDotnetSdk.razoruninstall/untrack call sites now pass the tracked spec's ownInstallRoot, so the command always targets the root the spec is actually tracked in.Additional fixes
DotnetWorkloadService.GetInventoriesAsyncrandotnet workloadcommands for every SDK feature band at the same time. The concurrent CLI processes contended badly enough thatdotnet workload list --machine-readablecould exceed the 45s timeout. Reads are now throttled to 2 at a time with aSemaphoreSlim, the same patternPublishProfileServiceuses.BackgroundTaskCenter.OnAfterRenderAsyncmade JS interop calls without the disposal guardsDisposeAsyncalready had, so a disposal race surfaced as "JS object instance with ID 1 does not exist". It now checks a_disposedflag and catches the same disposed-object exceptions.ProcessModalService.ShowProcessAsyncnow always releases its visibility gate in afinally.LocalSdkService.GetInstalledSdkVersionscounted any version-named folder undersdk/as installed. It now requiresdotnet.dll,dotnet.runtimeconfig.jsonandSdks/, so leftovers from partial installs or uninstalls are skipped.Notes for reviewers
DotnetSdkPresentationBuilderTests),--install-pathargument construction (DotnetUpArgumentsTests) and incomplete-SDK filtering (LocalSdkServiceTests). The Core suite passes (649 tests).LocalSdkServiceTests.GetInstalledManifestAsync_ResolvesManifestFromRemappedFeatureBandfails on my machine both with and without these changes, because it reads the host's real SDK state.