fix a logic error that caused AbandonedMutexException while executing…

… migrations (#4859)

src/NuGet.Core/NuGet.Common/Migrations/Migration1.cs

@@ -79,7 +79,7 @@ void AddAllParentDirectoriesUpToHome(string path)
             {
                 pathsToCheck.Add(path);
 
-                if (!path.StartsWith(homePath, StringComparison.Ordinal))
+                if (!path.StartsWith(homePath + Path.DirectorySeparatorChar, StringComparison.Ordinal))
                 {
                     return;
                 }
@@ -156,13 +156,13 @@ private static void FixPermissions(string path, PosixPermissions umask)
             {
                 PosixPermissions correctedPermissions = permissions.Value.WithUmask(umask);
                 string correctedPermissionsString = correctedPermissions.ToString();
-                Exec("chmod", correctedPermissionsString + " " + path);
+                Exec("chmod", correctedPermissionsString + " " + path.FormatWithDoubleQuotes());
             }
         }
 
         internal static PosixPermissions? GetPermissions(string path)
         {
-            string output = Exec("ls", "-ld " + path);
+            string output = Exec("ls", "-ld " + path.FormatWithDoubleQuotes());
             if (output == null)
             {
                 return null;

src/NuGet.Core/NuGet.Common/Migrations/MigrationRunner.cs

@@ -32,31 +32,48 @@ public static void Run()
                 // so use a global mutex and then check if someone else already did the work.
                 using (var mutex = new Mutex(false, "NuGet-Migrations"))
                 {
-                    bool signal = mutex.WaitOne(TimeSpan.FromMinutes(1), false);
-                    if (signal && !File.Exists(expectedMigrationFilename))
+                    if (WaitForMutex(mutex))
                     {
-                        // Only run migrations that have not already been run
-                        int highestMigrationRun = GetHighestMigrationRun(migrationsDirectory);
-                        for (int i = highestMigrationRun + 1; i < Migrations.Count; i++)
+                        if (!File.Exists(expectedMigrationFilename))
                         {
-                            try
+                            // Only run migrations that have not already been run
+                            int highestMigrationRun = GetHighestMigrationRun(migrationsDirectory);
+                            for (int i = highestMigrationRun + 1; i < Migrations.Count; i++)
                             {
-                                Migrations[i]();
-                                // Create file for every migration run, so that if an older version of NuGet is run, it doesn't try to run
-                                // migrations again.
-                                string migrationFile = Path.Combine(migrationsDirectory, (i + 1).ToString(CultureInfo.InvariantCulture));
-                                File.WriteAllText(migrationFile, string.Empty);
+                                try
+                                {
+                                    Migrations[i]();
+                                    // Create file for every migration run, so that if an older version of NuGet is run, it doesn't try to run
+                                    // migrations again.
+                                    string migrationFile = Path.Combine(migrationsDirectory, (i + 1).ToString(CultureInfo.InvariantCulture));
+                                    File.WriteAllText(migrationFile, string.Empty);
+                                }
+                                catch { }
                             }
-                            catch { }
                         }
-
                         mutex.ReleaseMutex();
                     }
                 }
             }
+
+            static bool WaitForMutex(Mutex mutex)
+            {
+                bool captured;
+
+                try
+                {
+                    captured = mutex.WaitOne(TimeSpan.FromMinutes(1), false);
+                }
+                catch (AbandonedMutexException)
+                {
+                    captured = true;
+                }
+
+                return captured;
+            }
         }
 
-        private static string GetMigrationsDirectory()
+        internal static string GetMigrationsDirectory()
         {
             string migrationsDirectory;
             if (RuntimeEnvironmentHelper.IsWindows)

src/NuGet.Core/NuGet.Common/StringExtensions.cs

@@ -0,0 +1,13 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+namespace NuGet.Common
+{
+    internal static class StringExtensions
+    {
+        internal static string FormatWithDoubleQuotes(this string s)
+        {
+            return s == null ? s : $@"""{s}""";
+        }
+    }
+}

test/NuGet.Core.Tests/NuGet.Common.Test/Migration1Tests.cs

@@ -57,11 +57,11 @@ public void EnsureExpectedPermissions_Directories_Success(string currentPermissi
         {
             using (var testDirectory = TestDirectory.Create())
             {
-                var v3cachePath = Path.Combine(testDirectory, "v3-cache");
+                var v3cachePath = Path.Combine(testDirectory, "my documents", "v3-cache");
                 var v3cacheSubDirectoryInfo = Directory.CreateDirectory(Path.Combine(v3cachePath, "subDirectory"));
                 Migration1.Exec("chmod", currentPermissions + " " + testDirectory.Path);
-                Migration1.Exec("chmod", currentPermissions + " " + v3cachePath);
-                Migration1.Exec("chmod", currentPermissions + " " + v3cacheSubDirectoryInfo.FullName);
+                Migration1.Exec("chmod", currentPermissions + " " + v3cachePath.FormatWithDoubleQuotes());
+                Migration1.Exec("chmod", currentPermissions + " " + v3cacheSubDirectoryInfo.FullName.FormatWithDoubleQuotes());
                 HashSet<string> pathsToCheck = new HashSet<string>() { testDirectory.Path, v3cachePath, v3cacheSubDirectoryInfo.FullName };
 
                 Migration1.EnsureExpectedPermissions(pathsToCheck, PosixPermissions.Parse(umask));

test/NuGet.Core.Tests/NuGet.Common.Test/MigrationRunnerTests.cs

@@ -0,0 +1,47 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Threading;
+using System.Threading.Tasks;
+using NuGet.Common.Migrations;
+using Xunit;
+
+namespace NuGet.Common.Test
+{
+    [CollectionDefinition("MigrationRunner", DisableParallelization = true)]
+    public class MigrationRunnerTests
+    {
+        [Fact]
+        public void WhenExecutedInParallelOnlyOneFileIsCreatedForEveryMigration_Success()
+        {
+            var threads = new List<Thread>();
+            int numThreads = 5;
+            int timeoutInSeconds = 90;
+
+            // Arrange
+            string directory = MigrationRunner.GetMigrationsDirectory();
+            if (Directory.Exists(directory))
+                Directory.Delete(path: directory, recursive: true);
+
+            // Act
+            for (int i = 0; i < numThreads; i++)
+            {
+                var thread = new Thread(MigrationRunner.Run);
+                thread.Start();
+                threads.Add(thread);
+            }
+
+            foreach (var thread in threads)
+            {
+                thread.Join(timeout: TimeSpan.FromSeconds(timeoutInSeconds));
+            }
+
+            // Assert
+            Assert.True(Directory.Exists(directory));
+            Assert.Equal(1, Directory.GetFiles(directory).Length);
+        }
+    }
+}

test/NuGet.Core.Tests/NuGet.Common.Test/StringExtensionsTests.cs

@@ -0,0 +1,30 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Xunit;
+
+namespace NuGet.Common.Test
+{
+    public class StringExtensionsTests
+    {
+        [Fact]
+        public void FormatWithDoubleQuotes_WhenNullIsPassedReturnsNull_Success()
+        {
+            string actual = null;
+            string formatted = actual.FormatWithDoubleQuotes();
+            Assert.Equal(actual, formatted);
+        }
+
+        [Theory]
+        [InlineData("")]
+        [InlineData("/home/user/NuGet AppData/share")]
+        [InlineData("/home/user/NuGet/share")]
+        [InlineData(@"c:\users\NuGet App\Share")]
+        [InlineData(@"c:\users\NuGet\Share")]
+        public void FormatWithDoubleQuotes_WhenNonNullStringIsPassedReturnsFormattedString_Success(string actual)
+        {
+            string formatted = actual.FormatWithDoubleQuotes();
+            Assert.Equal($@"""{actual}""", formatted);
+        }
+    }
+}