Serialize concurrent .NET tool operations with file locks#51834
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request implements concurrent tool installation protection using named mutexes to prevent "file being used by another process" errors when multiple dotnet CLI processes attempt to install the same .NET tool simultaneously.
Key changes:
- Adds named mutex synchronization (
tool-install-{packageId}-{packageVersion}) to serialize concurrent installations per package/version combination - Implements two-stage mutex acquisition with user notification for better UX
- Adds comprehensive test coverage for concurrent installation scenarios
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Cli/dotnet/ToolPackage/ToolPackageDownloaderBase.cs | Core implementation: wraps DownloadTool method with named mutex, includes timeout handling and mutex name generation |
| src/Cli/dotnet/CliStrings.resx | Adds two new error messages: ToolInstallationTimeout and ToolInstallationWaiting for user communication |
| src/Cli/dotnet/xlf/*.xlf | Adds localization entries for the new error messages across 13 language files (all marked as state="new") |
| test/Microsoft.DotNet.PackageInstall.Tests/ToolPackageDownloaderTests.cs | Adds GivenConcurrentInstallationsTheyDoNotConflict test with concurrent Task.Run executions; includes minor formatting fixes (alphabetized using statements, whitespace cleanup) |
|
Due to lack of recent activity, this PR has been labeled as 'Stale'. It will be closed if no further activity occurs within 7 more days. Any new comment will remove the label. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
- Add 50ms initial WaitOne to quickly check if mutex is available - Display informative message when waiting for another installation - Tell users they can press Ctrl+C to cancel - Maintains 5-minute total timeout for full wait Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
e779d23 to
137d0f7
Compare
Release the mutex only when the current thread acquired it so timeout and acquisition failures are not masked. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
Treat AbandonedMutexException as successful acquisition so a prior installer crash does not block the next installation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
Hash the normalized install root, package ID, and version into a bounded mutex name so unrelated agents and directories do not contend. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
Flow System.CommandLine cancellation through install, update, restore, execute, and dnx so Ctrl+C interrupts a contended mutex wait. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
Keep the per-package mutex through the staged package move, package validation, and rollback. Recheck the destination after acquiring the mutex so a concurrent installer reports the existing-package conflict instead of racing the final move. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
Preserve the legacy downloader overload while adding a cancellation-aware overload, and add focused coverage for contention, timeout, cancellation, abandonment, scoped mutex identities, and complete global installation serialization. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
Move invocation cancellation from command constructors to Execute methods and propagate it through install, update, restore, and tool execution paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
Use one InstallPackage overload with an optional trailing cancellation token. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
Return Task<int> from cancellation-aware Execute overloads so command parser actions can return command tasks directly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
This comment has been minimized.
This comment has been minimized.
Replace thread-affine named mutexes with persistent FileShare.None locks, keep locks through ambient transaction completion, and propagate cancellation through asynchronous install, update, restore, execute, and uninstall paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
🔴 Build Failure Analysis — MSTEST0049 errors (all 10 legs)All 10 build legs fail with
Root CauseThis PR changed Files modified by this PR that have errors
Other files with pre-existing errors (NOT modified by this PR, but also failing)The following test files were last touched by PR #54646 ("Resolve analyzers from the analyzers group in project.assets.json"), which likely activated the MSTEST0049 analyzer. They have pre-existing violations that were already present on the base branch but now become build errors:
FixFor every call site flagged by MSTEST0049, pass Example pattern — before: var package = downloader.InstallPackage(
new PackageLocation(nugetConfig: testDir.WithFile("NuGet.config")),
packageId: TestPackageId,
verbosity: TestVerbosity,
versionRange: VersionRange.Parse(TestPackageVersion),
targetFramework: _testTargetframework,
isGlobalTool: true,
verifySignatures: false);After: var package = downloader.InstallPackage(
new PackageLocation(nugetConfig: testDir.WithFile("NuGet.config")),
packageId: TestPackageId,
verbosity: TestVerbosity,
versionRange: VersionRange.Parse(TestPackageVersion),
targetFramework: _testTargetframework,
isGlobalTool: true,
verifySignatures: false,
cancellationToken: TestContext.CancellationToken);For // Before
Action secondCall = () => downloader.InstallPackage(..., verifySignatures: false);
secondCall.Should().Throw<ToolPackageException>()...;
// After
Action secondCall = () => downloader.InstallPackage(..., verifySignatures: false,
cancellationToken: TestContext.CancellationToken);
secondCall.Should().Throw<ToolPackageException>()...;
|
Pass MSTest's cancellation token to the new cancellable tool APIs so analyzer enforcement does not fail CI. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 815bde8e-0ac4-435d-8ca5-b0d0159ab079
Concurrent tool operations can race while downloading packages, mutating the global tool store, or creating shared shims. This change serializes those operations with persistent file locks opened using
FileShare.None.Changes
DOTNET_CLI_HOMEdifferences.--disable-parallelbehavior for tool restore and revalidateupdate --allentries under the store lock.When contention is detected, the CLI reports that another installation is in progress and waits up to the existing timeout. Users can cancel the wait with Ctrl+C.
Fixes #51831