Skip to content

Fix concurrent tool installation race conditions with named mutex #51834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+247 −14
Open

Fix concurrent tool installation race conditions with named mutex #51834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
078c682
Initial plan
Copilot Nov 20, 2025
75fc903
Add mutex protection for concurrent tool installations
Copilot Nov 20, 2025
e779d23
Improve mutex UX with initial quick check and waiting message
Copilot Nov 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/Cli/dotnet/CliStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,12 @@ setx PATH "%PATH%;{0}"
<data name="FailedToFindStagedToolPackage" xml:space="preserve">
<value>Failed to find staged tool package '{0}'.</value>
</data>
<data name="ToolInstallationTimeout" xml:space="preserve">
<value>Timeout waiting for concurrent installation of tool '{0}' version '{1}' to complete. Please try again.</value>
</data>
<data name="ToolInstallationWaiting" xml:space="preserve">
<value>Another installation of tool '{0}' version '{1}' is in progress. Waiting for it to complete... (Press Ctrl+C to cancel)</value>
</data>
<data name="ColumnMaxWidthMustBeGreaterThanZero" xml:space="preserve">
<value>Column maximum width must be greater than zero.</value>
</data>
Expand Down
54 changes: 44 additions & 10 deletions src/Cli/dotnet/ToolPackage/ToolPackageDownloaderBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,26 +272,60 @@ protected void DownloadTool(
string? targetFramework,
VerbosityOptions verbosity)
{
// Use a named mutex to serialize concurrent installations of the same tool package
string mutexName = GetToolInstallMutexName(packageId, packageVersion);
using var mutex = new Mutex(false, mutexName);
Comment on lines +275 to +277
Copy link
Contributor

@KalleOlaviNiemitalo KalleOlaviNiemitalo Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry about the scenario where a computer running Windows has two Jenkins agents installed as services with separate user accounts and separate file-system directories, and the agents run dotnet tool install on the same tool in parallel. Then this code will construct the same mutexName string in both processes, and because all services run in session 0, they will attempt to open the same mutex object; but because they have separate user accounts, the DACL of the mutex might not allow the second open.

Copy link
Contributor

@KalleOlaviNiemitalo KalleOlaviNiemitalo Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'd suggest including a hash of the directory path where the tool is going to be installed.


if (!IsPackageInstalled(packageId, packageVersion, packageDownloadDir.Value))
try
{
DownloadAndExtractPackage(packageId, nugetPackageDownloader, packageDownloadDir.Value, packageVersion, packageSourceLocation, includeUnlisted: givenSpecificVersion, verbosity: verbosity);
}
// First try a quick check to see if the mutex is immediately available
if (!mutex.WaitOne(TimeSpan.FromMilliseconds(50)))
{
// Mutex is held by another process - inform the user
Reporter.Error.WriteLine(string.Format(CliStrings.ToolInstallationWaiting, packageId, packageVersion));
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The waiting message is written to Reporter.Error, but this is informational rather than an error. According to the PR description, this message is meant to provide "transparency about what's happening." Consider using Reporter.Output instead, as this is normal operational information, not an error condition.

Reporter.Output.WriteLine(string.Format(CliStrings.ToolInstallationWaiting, packageId, packageVersion));
Suggested change
Reporter.Error.WriteLine(string.Format(CliStrings.ToolInstallationWaiting, packageId, packageVersion));
Reporter.Output.WriteLine(string.Format(CliStrings.ToolInstallationWaiting, packageId, packageVersion));

Copilot uses AI. Check for mistakes.

CreateAssetFile(packageId, packageVersion, packageDownloadDir, Path.Combine(assetFileDirectory.Value, ToolPackageInstance.AssetsFileName), _runtimeJsonPath, verbosity, targetFramework);
// Now wait for the longer duration
if (!mutex.WaitOne(TimeSpan.FromMinutes(5)))
{
throw new ToolPackageException(string.Format(CliStrings.ToolInstallationTimeout, packageId, packageVersion));
}
}
Comment on lines +279 to +292
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mutex acquisition logic should handle AbandonedMutexException, which can be thrown when a previous process holding the mutex terminated abnormally without releasing it. When this exception is caught, the calling thread has acquired the mutex and can safely proceed with the installation. Consider wrapping the WaitOne calls in a try-catch block that handles AbandonedMutexException.

Example:

try
{
    if (!mutex.WaitOne(TimeSpan.FromMilliseconds(50)))
    {
        // ... existing code ...
    }
}
catch (AbandonedMutexException)
{
    // Mutex was abandoned by another process, but we now own it
    // This is safe to proceed
}

Copilot uses AI. Check for mistakes.

// Also download RID-specific package if needed
if (ResolveRidSpecificPackage(packageId, packageVersion, packageDownloadDir, assetFileDirectory, verbosity) is PackageId ridSpecificPackage)
{
if (!IsPackageInstalled(ridSpecificPackage, packageVersion, packageDownloadDir.Value))
if (!IsPackageInstalled(packageId, packageVersion, packageDownloadDir.Value))
{
DownloadAndExtractPackage(ridSpecificPackage, nugetPackageDownloader, packageDownloadDir.Value, packageVersion, packageSourceLocation, includeUnlisted: true, verbosity: verbosity);
DownloadAndExtractPackage(packageId, nugetPackageDownloader, packageDownloadDir.Value, packageVersion, packageSourceLocation, includeUnlisted: givenSpecificVersion, verbosity: verbosity);
}

CreateAssetFile(ridSpecificPackage, packageVersion, packageDownloadDir, Path.Combine(assetFileDirectory.Value, ToolPackageInstance.RidSpecificPackageAssetsFileName), _runtimeJsonPath, verbosity, targetFramework);
CreateAssetFile(packageId, packageVersion, packageDownloadDir, Path.Combine(assetFileDirectory.Value, ToolPackageInstance.AssetsFileName), _runtimeJsonPath, verbosity, targetFramework);

// Also download RID-specific package if needed
if (ResolveRidSpecificPackage(packageId, packageVersion, packageDownloadDir, assetFileDirectory, verbosity) is PackageId ridSpecificPackage)
{
if (!IsPackageInstalled(ridSpecificPackage, packageVersion, packageDownloadDir.Value))
{
DownloadAndExtractPackage(ridSpecificPackage, nugetPackageDownloader, packageDownloadDir.Value, packageVersion, packageSourceLocation, includeUnlisted: true, verbosity: verbosity);
}

CreateAssetFile(ridSpecificPackage, packageVersion, packageDownloadDir, Path.Combine(assetFileDirectory.Value, ToolPackageInstance.RidSpecificPackageAssetsFileName), _runtimeJsonPath, verbosity, targetFramework);
}
}
finally
{
mutex.ReleaseMutex();
}
Comment on lines +312 to 315
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ReleaseMutex() call in the finally block can throw ApplicationException if the mutex is not currently owned by the calling thread (e.g., if an exception was thrown before the mutex was acquired, or if an AbandonedMutexException occurred but wasn't properly caught). This could mask the original exception. Consider checking if the mutex was successfully acquired before attempting to release it.

Example:

bool mutexAcquired = false;
try
{
    if (!mutex.WaitOne(TimeSpan.FromMilliseconds(50)))
    {
        // ...
        mutexAcquired = mutex.WaitOne(TimeSpan.FromMinutes(5));
    }
    else
    {
        mutexAcquired = true;
    }
    
    if (mutexAcquired)
    {
        // ... installation logic ...
    }
}
finally
{
    if (mutexAcquired)
    {
        mutex.ReleaseMutex();
    }
}

Copilot uses AI. Check for mistakes.
}

private static string GetToolInstallMutexName(PackageId packageId, NuGetVersion packageVersion)
{
// Create a mutex name in the format: tool-install-{packageId}-{packageVersion}
// Replace characters that are invalid in mutex names with underscores
string safeName = $"tool-install-{packageId}-{packageVersion.ToNormalizedString()}"
.Replace('/', '_')
.Replace('\\', '_');

return safeName;
}
Comment on lines +318 to +327
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mutex name sanitization only replaces / and \ characters, but mutex names on Windows have additional restrictions. Windows mutex names:

  1. Cannot exceed 260 characters
  2. Cannot contain certain special characters beyond / and \

Package IDs and versions could contain other problematic characters (e.g., + in semver build metadata). Consider using a more robust sanitization approach or creating a hash-based name for long/complex package identifiers.

Example:

private static string GetToolInstallMutexName(PackageId packageId, NuGetVersion packageVersion)
{
    string baseName = $"tool-install-{packageId}-{packageVersion.ToNormalizedString()}";
    
    // If the name is too long or contains problematic characters, use a hash
    if (baseName.Length > 200 || !IsValidMutexName(baseName))
    {
        using var sha256 = SHA256.Create();
        var hash = Convert.ToBase64String(sha256.ComputeHash(Encoding.UTF8.GetBytes(baseName)))
            .Replace('/', '_')
            .Replace('+', '-');
        return $"tool-install-{hash}";
    }
    
    return baseName.Replace('/', '_').Replace('\\', '_');
}

Copilot uses AI. Check for mistakes.

public bool TryGetDownloadedTool(
PackageId packageId,
NuGetVersion packageVersion,
Expand Down
10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.cs.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.de.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.es.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.fr.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.it.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.ja.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.ko.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.pl.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.pt-BR.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.ru.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.tr.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.zh-Hans.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.zh-Hant.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading