diff --git a/src/NuGet.Core/NuGet.Common/ConcurrencyUtilities.cs b/src/NuGet.Core/NuGet.Common/ConcurrencyUtilities.cs index 8e5fa50644c..d62ac47b150 100644 --- a/src/NuGet.Core/NuGet.Common/ConcurrencyUtilities.cs +++ b/src/NuGet.Core/NuGet.Common/ConcurrencyUtilities.cs @@ -36,7 +36,19 @@ public async static Task ExecuteWithFileLockedAsync(string filePath, { try { - // This file is deleted when the stream is closed. + FileOptions options; + if (RuntimeEnvironmentHelper.IsWindows) + { + + // This file is deleted when the stream is closed. + options = FileOptions.DeleteOnClose; + } + else + { + // FileOptions.DeleteOnClose causes concurrency issues on Mac OS X and Linux. + options = FileOptions.None; + } + // Sync operations have shown much better performance than FileOptions.Asynchronous fs = new FileStream( lockPath, @@ -44,7 +56,7 @@ public async static Task ExecuteWithFileLockedAsync(string filePath, FileAccess.ReadWrite, FileShare.None, bufferSize: 32, - options: FileOptions.DeleteOnClose); + options: options); } catch (DirectoryNotFoundException) { diff --git a/test/NuGet.Core.Tests/NuGet.Common.Test/ConcurrencyUtilitiesTests.cs b/test/NuGet.Core.Tests/NuGet.Common.Test/ConcurrencyUtilitiesTests.cs index bd181349d92..59793d89256 100644 --- a/test/NuGet.Core.Tests/NuGet.Common.Test/ConcurrencyUtilitiesTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Common.Test/ConcurrencyUtilitiesTests.cs @@ -1,9 +1,11 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Xunit; +using NuGet.Test.Utility; namespace NuGet.Common.Test { @@ -13,37 +15,57 @@ public class ConcurrencyUtilitiesTests public async Task ConcurrencyUtilities_LockStress() { // Arrange - var sem = new ManualResetEventSlim(); - var threads = 1000; - var tasks = new Stack>(threads); - var path = Path.Combine(Path.GetTempPath(), "ConcurrencyUtilities_LockStress"); - var token = CancellationToken.None; - Func> action = (ct) => { - // Wait till all threads are ready - sem.Wait(); - return Task.FromResult(true); - }; - - while (tasks.Count < threads) - { - var task = Task.Run(async () => await ConcurrencyUtilities.ExecuteWithFileLockedAsync( - path, - action, - token)); - - tasks.Push(task); - } - - // Act - // Release all the threads at once - sem.Set(); - await Task.WhenAll(tasks); - - // Assert - while (tasks.Count > 0) + using(var testDirectory = TestFileSystemUtility.CreateRandomTestFolder()) { - // Verify everything finished without errors - Assert.True(await tasks.Pop()); + // This is the path that uniquely identifies the system-wide mutex. + var path = Path.Combine(testDirectory, "ConcurrencyUtilities_LockStress_Verification"); + + // This is a semaphore use to verify the lock. + var verificationSemaphore = new SemaphoreSlim(1); + + // Iterate a lot, to increase confidence. + const int threads = 50; + const int iterations = 10; + + // This is the action that is execute inside of the lock. + Func> lockedActionAsync = async lockedToken => + { + var acquired = await verificationSemaphore.WaitAsync(0); + if (!acquired) + { + return false; + } + + // Hold the lock for a little bit. + await Task.Delay(TimeSpan.FromMilliseconds(1)); + + verificationSemaphore.Release(); + return true; + }; + + // Loop the same action, over and over. + Func>> loopAsync = async thread => + { + var loopResults = new List(); + foreach (var iteration in Enumerable.Range(0, iterations)) + { + var result = await ConcurrencyUtilities.ExecuteWithFileLockedAsync( + path, + lockedActionAsync, + CancellationToken.None); + loopResults.Add(result); + } + + return loopResults; + }; + + // Act + var tasks = Enumerable.Range(0, threads).Select(loopAsync); + var results = (await Task.WhenAll(tasks)).SelectMany(r => r).ToArray(); + + // Assert + Assert.Equal(threads * iterations, results.Length); + Assert.DoesNotContain(false, results); } }