Skip to content
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.

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

Revert non-determinism in Nuget local sources #19074

Merged
merged 1 commit into from
Mar 16, 2024
Merged
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Nikola Milosavljevic <[email protected]>
Date: Sat, 16 Mar 2024 05:17:27 +0000
Subject: [PATCH] Revert non-determinism in NuGet local sources

Revert of https://github.com/NuGet/NuGet.Client/commit/f54c58fdfce13feb68b846bd7a7e50058298afbc
---
.../SourceRepositoryDependencyProvider.cs | 64 ++++++++++++-------
1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs
index 4afc27f81..501a51985 100644
--- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs
+++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs
@@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
+using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
@@ -35,8 +36,11 @@ public class SourceRepositoryDependencyProvider : IRemoteDependencyProvider
private bool _isFallbackFolderSource;
private bool _useLegacyAssetTargetFallbackBehavior;

- private readonly TaskResultCache<LibraryRangeCacheKey, LibraryDependencyInfo> _dependencyInfoCache = new();
- private readonly TaskResultCache<LibraryRange, LibraryIdentity> _libraryMatchCache = new();
+ private readonly ConcurrentDictionary<LibraryRangeCacheKey, AsyncLazy<LibraryDependencyInfo>> _dependencyInfoCache
+ = new ConcurrentDictionary<LibraryRangeCacheKey, AsyncLazy<LibraryDependencyInfo>>();
+
+ private readonly ConcurrentDictionary<LibraryRange, AsyncLazy<LibraryIdentity>> _libraryMatchCache
+ = new ConcurrentDictionary<LibraryRange, AsyncLazy<LibraryIdentity>>();

// Limiting concurrent requests to limit the amount of files open at a time.
private readonly static SemaphoreSlim _throttle = GetThrottleSemaphoreSlim(EnvironmentVariableWrapper.Instance);
@@ -202,15 +206,23 @@ internal static SemaphoreSlim GetThrottleSemaphoreSlim(IEnvironmentVariableReade

cancellationToken.ThrowIfCancellationRequested();

- try
+ AsyncLazy<LibraryIdentity> result = null;
+
+ var action = new AsyncLazy<LibraryIdentity>(async () =>
+ await FindLibraryCoreAsync(libraryRange, cacheContext, logger, cancellationToken));
+
+ if (cacheContext.RefreshMemoryCache)
{
- LibraryIdentity result = await _libraryMatchCache.GetOrAddAsync(
- libraryRange,
- cacheContext.RefreshMemoryCache,
- static state => state.caller.FindLibraryCoreAsync(state.libraryRange, state.cacheContext, state.logger, state.cancellationToken),
- (caller: this, libraryRange, cacheContext, logger, cancellationToken), cancellationToken);
+ result = _libraryMatchCache.AddOrUpdate(libraryRange, action, (k, v) => action);
+ }
+ else
+ {
+ result = _libraryMatchCache.GetOrAdd(libraryRange, action);
+ }

- return result;
+ try
+ {
+ return await result;
}
catch (FatalProtocolException e)
{
@@ -224,7 +236,6 @@ internal static SemaphoreSlim GetThrottleSemaphoreSlim(IEnvironmentVariableReade
throw;
}
}
-
return null;
}

@@ -234,6 +245,7 @@ internal static SemaphoreSlim GetThrottleSemaphoreSlim(IEnvironmentVariableReade
ILogger logger,
CancellationToken cancellationToken)
{
+
await EnsureResource(cancellationToken);

if (libraryRange.VersionRange?.MinVersion != null && libraryRange.VersionRange.IsMinInclusive && !libraryRange.VersionRange.IsFloating)
@@ -310,7 +322,7 @@ internal static SemaphoreSlim GetThrottleSemaphoreSlim(IEnvironmentVariableReade
/// is either <see langword="null" /> or empty.</exception>
/// <exception cref="OperationCanceledException">Thrown if <paramref name="cancellationToken" />
/// is cancelled.</exception>
- public Task<LibraryDependencyInfo> GetDependenciesAsync(
+ public async Task<LibraryDependencyInfo> GetDependenciesAsync(
LibraryIdentity libraryIdentity,
NuGetFramework targetFramework,
SourceCacheContext cacheContext,
@@ -339,13 +351,23 @@ internal static SemaphoreSlim GetThrottleSemaphoreSlim(IEnvironmentVariableReade

cancellationToken.ThrowIfCancellationRequested();

- LibraryRangeCacheKey key = new(libraryIdentity, targetFramework);
+ AsyncLazy<LibraryDependencyInfo> result = null;
+
+ var action = new AsyncLazy<LibraryDependencyInfo>(async () =>
+ await GetDependenciesCoreAsync(libraryIdentity, targetFramework, cacheContext, logger, cancellationToken));
+
+ var key = new LibraryRangeCacheKey(libraryIdentity, targetFramework);

- return _dependencyInfoCache.GetOrAddAsync(
- key,
- cacheContext.RefreshMemoryCache,
- static state => state.caller.GetDependenciesCoreAsync(state.libraryIdentity, state.targetFramework, state.cacheContext, state.logger, state.cancellationToken),
- (caller: this, libraryIdentity, targetFramework, cacheContext, logger, cancellationToken), cancellationToken);
+ if (cacheContext.RefreshMemoryCache)
+ {
+ result = _dependencyInfoCache.AddOrUpdate(key, action, (k, v) => action);
+ }
+ else
+ {
+ result = _dependencyInfoCache.GetOrAdd(key, action);
+ }
+
+ return await result;
}

private async Task<LibraryDependencyInfo> GetDependenciesCoreAsync(
@@ -390,12 +412,10 @@ internal static SemaphoreSlim GetThrottleSemaphoreSlim(IEnvironmentVariableReade
_throttle?.Release();
}

- LibraryDependencyInfo libraryDependencyInfo = null;
-
if (packageInfo == null)
{
// Package was not found
- libraryDependencyInfo = LibraryDependencyInfo.CreateUnresolved(match, targetFramework);
+ return LibraryDependencyInfo.CreateUnresolved(match, targetFramework);
}
else
{
@@ -407,10 +427,8 @@ internal static SemaphoreSlim GetThrottleSemaphoreSlim(IEnvironmentVariableReade

IEnumerable<LibraryDependency> dependencyGroup = GetDependencies(packageInfo, targetFramework);

- libraryDependencyInfo = LibraryDependencyInfo.Create(originalIdentity, targetFramework, dependencies: dependencyGroup);
+ return LibraryDependencyInfo.Create(originalIdentity, targetFramework, dependencies: dependencyGroup);
}
-
- return libraryDependencyInfo;
}

/// <summary>