From 3cadfac3ab26755d625c3944d1932ccbb9e10906 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Wed, 16 Oct 2024 12:22:53 +0100 Subject: [PATCH 01/10] try to add retry to RemoveDirFixed --- .../Tasks/RemoveDirFixed.cs | 34 +++++++++++++++++-- .../Tasks/RemoveDirTests.cs | 12 +++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs index c456b87eefd..8657daefbc1 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs @@ -28,6 +28,8 @@ using System; using System.Collections.Generic; using System.IO; +using System.Threading; +using System.Runtime.InteropServices; using Microsoft.Build.Framework; using Xamarin.Android.Tools; using Microsoft.Android.Build.Tasks; @@ -38,6 +40,11 @@ public class RemoveDirFixed : AndroidTask { public override string TaskPrefix => "RDF"; + const int DEFAULT_DIRECTORY_DELETE_RETRY_DELAY_MS = 1000; + const int DEFAULT_REMOVEDIRFIXED_RETRIES = 3; + const int ERROR_ACCESS_DENIED = -2147024891; + const int ERROR_SHARING_VIOLATION = -2147024864; + public override bool RunTask () { var temporaryRemovedDirectories = new List (Directories.Length); @@ -55,10 +62,31 @@ public override bool RunTask () } catch (UnauthorizedAccessException ex) { // if that fails we probably have readonly files (or locked files) // so try to make them writable and try again. + Log.LogDebugMessage ("error: " + ex); try { - Files.SetDirectoryWriteable (fullPath); - Directory.Delete (fullPath, true); - temporaryRemovedDirectories.Add (directory); + int retryCount = 0; + while (retryCount <= DEFAULT_REMOVEDIRFIXED_RETRIES) { + try { + Files.SetDirectoryWriteable (fullPath); + Directory.Delete (fullPath, true); + temporaryRemovedDirectories.Add (directory); + break; + } catch (Exception e) { + switch (e) { + case UnauthorizedAccessException: + case IOException: + int code = Marshal.GetHRForException(e); + if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount == DEFAULT_REMOVEDIRFIXED_RETRIES) { + throw; + }; + break; + default: + throw; + } + } + Thread.Sleep(DEFAULT_DIRECTORY_DELETE_RETRY_DELAY_MS); + retryCount++; + } } catch (Exception inner) { Log.LogUnhandledException (TaskPrefix, ex); Log.LogUnhandledException (TaskPrefix, inner); diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs index bf93d07faca..1fba8d1e1f1 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs @@ -83,5 +83,17 @@ public void LongPath () Assert.AreEqual (1, task.RemovedDirectories.Length, "Changes should have been made."); DirectoryAssert.DoesNotExist (tempDirectory); } + + [Test] + public void DirectoryInUse () + { + var file = NewFile (); + var task = CreateTask (); + using (var f = File.OpenWrite (file)) { + Assert.IsFalse (task.Execute (), "task.Execute() should have failed."); + Assert.AreEqual (0, task.RemovedDirectories.Length, "Changes should not have been made."); + DirectoryAssert.Exists (tempDirectory); + } + } } } From 201e6c0504ef6fdb948aa41dd4126939131ff8c5 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Wed, 16 Oct 2024 13:24:52 +0100 Subject: [PATCH 02/10] move to Smoke Test --- .../Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs index 1fba8d1e1f1..66825df0172 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs @@ -84,7 +84,7 @@ public void LongPath () DirectoryAssert.DoesNotExist (tempDirectory); } - [Test] + [Test, Category ("SmokeTests")] public void DirectoryInUse () { var file = NewFile (); From ff7d045cc6e295697a7e178a744066bfb633b40f Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Thu, 17 Oct 2024 09:17:50 +0100 Subject: [PATCH 03/10] add a test --- .../Tasks/RemoveDirFixed.cs | 2 +- .../Tasks/RemoveDirTests.cs | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs index 8657daefbc1..86a9135a3a3 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs @@ -41,7 +41,7 @@ public class RemoveDirFixed : AndroidTask public override string TaskPrefix => "RDF"; const int DEFAULT_DIRECTORY_DELETE_RETRY_DELAY_MS = 1000; - const int DEFAULT_REMOVEDIRFIXED_RETRIES = 3; + const int DEFAULT_REMOVEDIRFIXED_RETRIES = 10; const int ERROR_ACCESS_DENIED = -2147024891; const int ERROR_SHARING_VIOLATION = -2147024864; diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs index 66825df0172..765dbfbae80 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs @@ -7,6 +7,8 @@ using Xamarin.Android.Tasks; using Xamarin.Android.Tools; using Microsoft.Android.Build.Tasks; +using TPL = System.Threading.Tasks; +using System.Threading; namespace Xamarin.Android.Build.Tests { @@ -87,6 +89,10 @@ public void LongPath () [Test, Category ("SmokeTests")] public void DirectoryInUse () { + if (OS.IsMac) { + Assert.Ignore ("This is not an issue on macos."); + return; + } var file = NewFile (); var task = CreateTask (); using (var f = File.OpenWrite (file)) { @@ -95,5 +101,28 @@ public void DirectoryInUse () DirectoryAssert.Exists (tempDirectory); } } + + [Test, Category ("SmokeTests")] + public async TPL.Task DirectoryInUseWithRetry () + { + if (OS.IsMac) { + Assert.Ignore ("This is not an issue on macos."); + return; + } + var file = NewFile (); + var task = CreateTask (); + var ev = new ManualResetEvent (false); + var t = TPL.Task.Run (async () => { + using (var f = File.OpenWrite (file)) { + ev.Set (); + await TPL.Task.Delay (2500); + } + }); + ev.WaitOne (); + Assert.IsTrue (task.Execute (), "task.Execute() should have failed."); + Assert.AreEqual (1, task.RemovedDirectories.Length, "Changes should have been made."); + DirectoryAssert.DoesNotExist (tempDirectory); + await t; + } } } From 49ca8fbd26dbf85f79c2e537e314df5ce8a9a1fb Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Thu, 17 Oct 2024 14:00:22 +0100 Subject: [PATCH 04/10] fix the test --- src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs index 86a9135a3a3..f5df2b6fefe 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs @@ -59,7 +59,14 @@ public override bool RunTask () // try to do a normal "fast" delete of the directory. Directory.Delete (fullPath, true); temporaryRemovedDirectories.Add (directory); - } catch (UnauthorizedAccessException ex) { + } catch (Exception ex) { + switch (ex) { + case UnauthorizedAccessException: + case IOException: + break; + default: + throw; + } // if that fails we probably have readonly files (or locked files) // so try to make them writable and try again. Log.LogDebugMessage ("error: " + ex); From 24246e0c54c839a3748ae0c468dcbb51e4d5690d Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Thu, 17 Oct 2024 14:10:07 +0100 Subject: [PATCH 05/10] Fix error message --- .../Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs index 765dbfbae80..20a828e8f4c 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs @@ -119,7 +119,7 @@ public async TPL.Task DirectoryInUseWithRetry () } }); ev.WaitOne (); - Assert.IsTrue (task.Execute (), "task.Execute() should have failed."); + Assert.IsTrue (task.Execute (), "task.Execute() should have succeeded."); Assert.AreEqual (1, task.RemovedDirectories.Length, "Changes should have been made."); DirectoryAssert.DoesNotExist (tempDirectory); await t; From fa7ed1b175f57a574516ae5136a1961716fd688b Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Thu, 17 Oct 2024 15:57:13 +0100 Subject: [PATCH 06/10] Rework RemoveDirFixed to retry --- .../Tasks/RemoveDirFixed.cs | 75 ++++++------------- 1 file changed, 24 insertions(+), 51 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs index f5df2b6fefe..03ad323ed3c 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs @@ -55,64 +55,37 @@ public override bool RunTask () Log.LogDebugMessage ($"Directory did not exist: {fullPath}"); continue; } + int retryCount = 0; try { - // try to do a normal "fast" delete of the directory. - Directory.Delete (fullPath, true); - temporaryRemovedDirectories.Add (directory); - } catch (Exception ex) { - switch (ex) { - case UnauthorizedAccessException: - case IOException: - break; - default: - throw; - } - // if that fails we probably have readonly files (or locked files) - // so try to make them writable and try again. - Log.LogDebugMessage ("error: " + ex); - try { - int retryCount = 0; - while (retryCount <= DEFAULT_REMOVEDIRFIXED_RETRIES) { - try { + while (retryCount <= DEFAULT_REMOVEDIRFIXED_RETRIES) { + try { + // try to do a normal "fast" delete of the directory. + // only do the set writable on the second attempt + if (retryCount == 1) Files.SetDirectoryWriteable (fullPath); - Directory.Delete (fullPath, true); - temporaryRemovedDirectories.Add (directory); - break; - } catch (Exception e) { - switch (e) { - case UnauthorizedAccessException: - case IOException: - int code = Marshal.GetHRForException(e); - if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount == DEFAULT_REMOVEDIRFIXED_RETRIES) { - throw; - }; - break; - default: + Directory.Delete (fullPath, true); + temporaryRemovedDirectories.Add (directory); + } catch (Exception e) { + switch (e) { + case DirectoryNotFoundException: + if (OS.IsWindows) { + fullPath = Files.ToLongPath (fullPath); + Log.LogDebugMessage ("Trying long path: " + fullPath); + } + break; + case UnauthorizedAccessException: + case IOException: + int code = Marshal.GetHRForException(e); + if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount == DEFAULT_REMOVEDIRFIXED_RETRIES) { throw; - } + }; + break; + default: + throw; } Thread.Sleep(DEFAULT_DIRECTORY_DELETE_RETRY_DELAY_MS); retryCount++; } - } catch (Exception inner) { - Log.LogUnhandledException (TaskPrefix, ex); - Log.LogUnhandledException (TaskPrefix, inner); - } - } catch (DirectoryNotFoundException ex) { - // This could be a file inside the directory over MAX_PATH. - // We can attempt using the \\?\ syntax. - if (OS.IsWindows) { - try { - fullPath = Files.ToLongPath (fullPath); - Log.LogDebugMessage ("Trying long path: " + fullPath); - Directory.Delete (fullPath, true); - temporaryRemovedDirectories.Add (directory); - } catch (Exception inner) { - Log.LogUnhandledException (TaskPrefix, ex); - Log.LogUnhandledException (TaskPrefix, inner); - } - } else { - Log.LogUnhandledException (TaskPrefix, ex); } } catch (Exception ex) { Log.LogUnhandledException (TaskPrefix, ex); From b4bb9d2f07c1b2a8f49696784e72076bb5e80638 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Thu, 17 Oct 2024 16:14:50 +0100 Subject: [PATCH 07/10] throw after retry --- src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs index 03ad323ed3c..07e9c89e523 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs @@ -72,6 +72,8 @@ public override bool RunTask () fullPath = Files.ToLongPath (fullPath); Log.LogDebugMessage ("Trying long path: " + fullPath); } + if (retryCount == DEFAULT_REMOVEDIRFIXED_RETRIES) + throw; break; case UnauthorizedAccessException: case IOException: From fc0ad868dee123b9ac218b8d7b452a0367220205 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Thu, 17 Oct 2024 16:46:18 +0100 Subject: [PATCH 08/10] ff --- src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs index 07e9c89e523..30c158ddb81 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs @@ -71,10 +71,9 @@ public override bool RunTask () if (OS.IsWindows) { fullPath = Files.ToLongPath (fullPath); Log.LogDebugMessage ("Trying long path: " + fullPath); + break; } - if (retryCount == DEFAULT_REMOVEDIRFIXED_RETRIES) - throw; - break; + throw; case UnauthorizedAccessException: case IOException: int code = Marshal.GetHRForException(e); From d04b11a3ecc64a01935e8c3ccef5e738ce49f074 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Thu, 17 Oct 2024 17:19:23 +0100 Subject: [PATCH 09/10] ff --- src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs index 30c158ddb81..36b790ebe71 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs @@ -65,6 +65,7 @@ public override bool RunTask () Files.SetDirectoryWriteable (fullPath); Directory.Delete (fullPath, true); temporaryRemovedDirectories.Add (directory); + break; } catch (Exception e) { switch (e) { case DirectoryNotFoundException: From 2a492229f6c876687d95b51264ba4c61920e59df Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 22 Oct 2024 17:02:15 +0100 Subject: [PATCH 10/10] Pull in xamarin-android-tools --- external/xamarin-android-tools | 2 +- .../Tasks/RemoveDirFixed.cs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/external/xamarin-android-tools b/external/xamarin-android-tools index ab2165daf27..60fae1924e2 160000 --- a/external/xamarin-android-tools +++ b/external/xamarin-android-tools @@ -1 +1 @@ -Subproject commit ab2165daf27d4fcb29e88bc022e0ab0be33aff69 +Subproject commit 60fae1924e2d71f31dc1ed05f7346fd7874d6636 diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs index 36b790ebe71..da33725f0e6 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs @@ -40,8 +40,6 @@ public class RemoveDirFixed : AndroidTask { public override string TaskPrefix => "RDF"; - const int DEFAULT_DIRECTORY_DELETE_RETRY_DELAY_MS = 1000; - const int DEFAULT_REMOVEDIRFIXED_RETRIES = 10; const int ERROR_ACCESS_DENIED = -2147024891; const int ERROR_SHARING_VIOLATION = -2147024864; @@ -56,8 +54,10 @@ public override bool RunTask () continue; } int retryCount = 0; + int attempts = Files.GetFileWriteRetryAttempts (); + int delay = Files.GetFileWriteRetryDelay (); try { - while (retryCount <= DEFAULT_REMOVEDIRFIXED_RETRIES) { + while (retryCount <= attempts) { try { // try to do a normal "fast" delete of the directory. // only do the set writable on the second attempt @@ -78,14 +78,14 @@ public override bool RunTask () case UnauthorizedAccessException: case IOException: int code = Marshal.GetHRForException(e); - if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount == DEFAULT_REMOVEDIRFIXED_RETRIES) { + if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount >= attempts) { throw; }; break; default: throw; } - Thread.Sleep(DEFAULT_DIRECTORY_DELETE_RETRY_DELAY_MS); + Thread.Sleep (delay); retryCount++; } }