Make delete more robust (fix #1662) (#1667)

Author: o-l-a-v

.gitignore

@@ -4,4 +4,4 @@ srcOld/code/bin
 srcOld/code/obj
 out
 test/**/obj
-test/**/bin
+test/**/bin

src/code/InstallHelper.cs

@@ -467,7 +467,7 @@ private void MoveFilesIntoInstallPath(
                         // Delete the directory path before replacing it with the new module.
                         // If deletion fails (usually due to binary file in use), then attempt restore so that the currently
                         // installed module is not corrupted.
-                        _cmdletPassedIn.WriteVerbose($"Attempting to delete with restore on failure.'{finalModuleVersionDir}'");
+                        _cmdletPassedIn.WriteVerbose($"Attempting to delete with restore on failure. '{finalModuleVersionDir}'");
                         Utils.DeleteDirectoryWithRestore(finalModuleVersionDir);
                     }
 

src/code/Utils.cs

@@ -20,6 +20,8 @@
 using System.Security;
 using Azure.Core;
 using Azure.Identity;
+using System.Threading.Tasks;
+using System.Threading;
 
 namespace Microsoft.PowerShell.PSResourceGet.UtilClasses
 {
@@ -42,7 +44,7 @@ public enum MetadataFileType
         #region String fields
 
         public static readonly string[] EmptyStrArray = Array.Empty<string>();
-        public static readonly char[] WhitespaceSeparator = new char[]{' '};
+        public static readonly char[] WhitespaceSeparator = new char[] { ' ' };
         public const string PSDataFileExt = ".psd1";
         public const string PSScriptFileExt = ".ps1";
         private const string ConvertJsonToHashtableScript = @"
@@ -140,7 +142,7 @@ public static string[] GetStringArray(ArrayList list)
             if (list == null) { return null; }
 
             var strArray = new string[list.Count];
-            for (int i=0; i < list.Count; i++)
+            for (int i = 0; i < list.Count; i++)
             {
                 strArray[i] = list[i] as string;
             }
@@ -161,7 +163,7 @@ public static string[] ProcessNameWildcards(
             {
                 isContainWildcard = true;
                 errorMsgs = errorMsgsList.ToArray();
-                return new string[] {"*"};
+                return new string[] { "*" };
             }
 
             isContainWildcard = false;
@@ -179,8 +181,8 @@ public static string[] ProcessNameWildcards(
                     if (String.Equals(name, "*", StringComparison.InvariantCultureIgnoreCase))
                     {
                         isContainWildcard = true;
-                        errorMsgs = new string[] {};
-                        return new string[] {"*"};
+                        errorMsgs = new string[] { };
+                        return new string[] { "*" };
                     }
 
                     if (name.Contains("?") || name.Contains("["))
@@ -275,7 +277,8 @@ public static bool TryGetVersionType(
                     // eg: 2.8.8.* should translate to the version range "[2.1.3.0,2.1.3.99999]"
                     modifiedVersion = $"[{versionSplit[0]}.{versionSplit[1]}.{versionSplit[2]}.0,{versionSplit[0]}.{versionSplit[1]}.{versionSplit[2]}.999999]";
                 }
-                else {
+                else
+                {
                     error = "Argument for -Version parameter is not in the proper format";
                     return false;
                 }
@@ -469,15 +472,15 @@ public static bool TryCreateValidPSCredentialInfo(
 
             try
             {
-                if (!string.IsNullOrEmpty((string) credentialInfoCandidate.Properties[PSCredentialInfo.VaultNameAttribute]?.Value)
-                    && !string.IsNullOrEmpty((string) credentialInfoCandidate.Properties[PSCredentialInfo.SecretNameAttribute]?.Value))
+                if (!string.IsNullOrEmpty((string)credentialInfoCandidate.Properties[PSCredentialInfo.VaultNameAttribute]?.Value)
+                    && !string.IsNullOrEmpty((string)credentialInfoCandidate.Properties[PSCredentialInfo.SecretNameAttribute]?.Value))
                 {
                     PSCredential credential = null;
                     if (credentialInfoCandidate.Properties[PSCredentialInfo.CredentialAttribute] != null)
                     {
                         try
                         {
-                            credential = (PSCredential) credentialInfoCandidate.Properties[PSCredentialInfo.CredentialAttribute].Value;
+                            credential = (PSCredential)credentialInfoCandidate.Properties[PSCredentialInfo.CredentialAttribute].Value;
                         }
                         catch (Exception e)
                         {
@@ -492,8 +495,8 @@ public static bool TryCreateValidPSCredentialInfo(
                     }
 
                     repoCredentialInfo = new PSCredentialInfo(
-                        (string) credentialInfoCandidate.Properties[PSCredentialInfo.VaultNameAttribute].Value,
-                        (string) credentialInfoCandidate.Properties[PSCredentialInfo.SecretNameAttribute].Value,
+                        (string)credentialInfoCandidate.Properties[PSCredentialInfo.VaultNameAttribute].Value,
+                        (string)credentialInfoCandidate.Properties[PSCredentialInfo.SecretNameAttribute].Value,
                         credential
                     );
 
@@ -711,7 +714,7 @@ public static string GetContainerRegistryAccessTokenFromSecretManagement(
                 string password = new NetworkCredential(string.Empty, secretSecureString).Password;
                 return password;
             }
-            else if(secretValue is PSCredential psCredSecret)
+            else if (secretValue is PSCredential psCredSecret)
             {
                 string password = new NetworkCredential(string.Empty, psCredSecret.Password).Password;
                 return password;
@@ -1120,7 +1123,8 @@ private static void GetStandardPlatformPaths(
                 // paths are the same for both Linux and macOS
                 localUserDir = Path.Combine(GetHomeOrCreateTempHome(), ".local", "share", "powershell");
                 // Create the default data directory if it doesn't exist.
-                if (!Directory.Exists(localUserDir)) {
+                if (!Directory.Exists(localUserDir))
+                {
                     Directory.CreateDirectory(localUserDir);
                 }
 
@@ -1247,7 +1251,7 @@ private static bool TryReadPSDataFile(
                     result = psObject.BaseObject;
                 }
 
-                dataFileInfo = (Hashtable) result;
+                dataFileInfo = (Hashtable)result;
                 error = null;
                 return true;
             }
@@ -1369,10 +1373,10 @@ public static bool TryCreateModuleSpecification(
             validatedModuleSpecs = Array.Empty<ModuleSpecification>();
             List<ModuleSpecification> moduleSpecsList = new List<ModuleSpecification>();
 
-            foreach(Hashtable moduleSpec in moduleSpecHashtables)
+            foreach (Hashtable moduleSpec in moduleSpecHashtables)
             {
                 // ModuleSpecification(string) constructor for creating a ModuleSpecification when only ModuleName is provided.
-                if (!moduleSpec.ContainsKey("ModuleName") || String.IsNullOrEmpty((string) moduleSpec["ModuleName"]))
+                if (!moduleSpec.ContainsKey("ModuleName") || String.IsNullOrEmpty((string)moduleSpec["ModuleName"]))
                 {
                     errorList.Add(new ErrorRecord(
                         new ArgumentException($"RequiredModules Hashtable entry {moduleSpec.ToString()} is missing a key 'ModuleName' and associated value, which is required for each module specification entry"),
@@ -1384,7 +1388,7 @@ public static bool TryCreateModuleSpecification(
                 }
 
                 // At this point it must contain ModuleName key.
-                string moduleSpecName = (string) moduleSpec["ModuleName"];
+                string moduleSpecName = (string)moduleSpec["ModuleName"];
                 ModuleSpecification currentModuleSpec = null;
                 if (!moduleSpec.ContainsKey("MaximumVersion") && !moduleSpec.ContainsKey("ModuleVersion") && !moduleSpec.ContainsKey("RequiredVersion"))
                 {
@@ -1410,10 +1414,10 @@ public static bool TryCreateModuleSpecification(
                 else
                 {
                     // ModuleSpecification(Hashtable) constructor for when ModuleName + {Required,Maximum,Module}Version value is also provided.
-                    string moduleSpecMaxVersion = moduleSpec.ContainsKey("MaximumVersion") ? (string) moduleSpec["MaximumVersion"] : String.Empty;
-                    string moduleSpecModuleVersion = moduleSpec.ContainsKey("ModuleVersion") ? (string) moduleSpec["ModuleVersion"] : String.Empty;
-                    string moduleSpecRequiredVersion = moduleSpec.ContainsKey("RequiredVersion") ? (string) moduleSpec["RequiredVersion"] : String.Empty;
-                    Guid moduleSpecGuid = moduleSpec.ContainsKey("Guid") ? (Guid) moduleSpec["Guid"] : Guid.Empty;
+                    string moduleSpecMaxVersion = moduleSpec.ContainsKey("MaximumVersion") ? (string)moduleSpec["MaximumVersion"] : String.Empty;
+                    string moduleSpecModuleVersion = moduleSpec.ContainsKey("ModuleVersion") ? (string)moduleSpec["ModuleVersion"] : String.Empty;
+                    string moduleSpecRequiredVersion = moduleSpec.ContainsKey("RequiredVersion") ? (string)moduleSpec["RequiredVersion"] : String.Empty;
+                    Guid moduleSpecGuid = moduleSpec.ContainsKey("Guid") ? (Guid)moduleSpec["Guid"] : Guid.Empty;
 
                     if (String.IsNullOrEmpty(moduleSpecMaxVersion) && String.IsNullOrEmpty(moduleSpecModuleVersion) && String.IsNullOrEmpty(moduleSpecRequiredVersion))
                     {
@@ -1536,22 +1540,36 @@ public static void DeleteDirectoryWithRestore(string dirPath)
         /// </Summary>
         public static void DeleteDirectory(string dirPath)
         {
-            foreach (var dirFilePath in Directory.GetFiles(dirPath))
+            // Remove read only file attributes first
+            foreach (var dirFilePath in Directory.GetFiles(dirPath,"*",SearchOption.AllDirectories))
             {
                 if (File.GetAttributes(dirFilePath).HasFlag(FileAttributes.ReadOnly))
                 {
-                    File.SetAttributes(dirFilePath, (File.GetAttributes(dirFilePath) & ~FileAttributes.ReadOnly));
+                    File.SetAttributes(dirFilePath, File.GetAttributes(dirFilePath) & ~FileAttributes.ReadOnly);
                 }
-
-                File.Delete(dirFilePath);
             }
-
-            foreach (var dirSubPath in Directory.GetDirectories(dirPath))
+            // Delete directory recursive, try multiple times before throwing ( #1662 )
+            int maxAttempts = 5;
+            int msDelay = 5;
+            for (int attempt = 1; attempt <= maxAttempts; ++attempt)
             {
-                DeleteDirectory(dirSubPath);
+                try
+                {
+                    Directory.Delete(dirPath,true);
+                    return;
+                }
+                catch (Exception ex)
+                {
+                    if (attempt < maxAttempts && (ex is IOException || ex is UnauthorizedAccessException))
+                    {
+                        Thread.Sleep(msDelay);
+                    }
+                    else
+                    {
+                        throw;
+                    }
+                }
             }
-
-            Directory.Delete(dirPath);
         }
 
         /// <Summary>