[Xamarin.Android.Build.Tasks] Add retry ability to RemoveDirFixed (#9409

)

Fixes: #9133

Context: dotnet/android-tools@60fae19

Much to our chagrin, in .NET 6+ it is possible for Design-Time Builds
(DTBs) to run concurrently with "normal" builds, as there is nothing
within the [.NET Project System][0] or [Common Project System (CPS)][1]
which would actively *prevent* such concurrency.

Consequently, it is possible to encounter locked files and
directories during the build process, and user may see errors such as:

	Error (active)	XARDF7019	System.UnauthorizedAccessException: Access to the path 'GoogleGson.dll' is denied.
	   at System.IO.Directory.DeleteHelper(String fullPath, String userPath, Boolean recursive, Boolean throwOnTopLevelDirectoryNotFound, WIN32_FIND_DATA& data)
	   at System.IO.Directory.Delete(String fullPath, String userPath, Boolean recursive, Boolean checkHost)
	   at Xamarin.Android.Tasks.RemoveDirFixed.RunTask() in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs:line 54	MauiApp2 (net9.0-android)	C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\34.99.0-preview.6.340\tools\Xamarin.Android.Common.targets	2503

dotnet/android-tools@60fae192 introduced the concept of a "Retry" in
the cases of `UnauthorizedAccessException`s or `IOException`s when
the code is `ACCESS_DENIED` or `ERROR_SHARING_VIOLATION`.

Builds upon that work to use the API's added to add retry semantics
to the `<RemoveDirFixed/>` task. 

This also simplifies the Task somewhat as it had quite complex
exception handling. 

[0]: https://github.com/dotnet/project-system
[1]: https://github.com/microsoft/VSProjectSystem

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,9 @@ public class RemoveDirFixed : AndroidTask
 	{
 		public override string TaskPrefix => "RDF";
 
+		const int ERROR_ACCESS_DENIED = -2147024891;
+		const int ERROR_SHARING_VIOLATION = -2147024864;
+
 		public override bool RunTask ()
 		{
 			var temporaryRemovedDirectories = new List<ITaskItem> (Directories.Length);
@@ -48,36 +53,41 @@ public override bool RunTask ()
 					Log.LogDebugMessage ($"Directory did not exist: {fullPath}");
 					continue;
 				}
+				int retryCount = 0;
+				int attempts = Files.GetFileWriteRetryAttempts ();
+				int delay = Files.GetFileWriteRetryDelay ();
 				try {
-					// try to do a normal "fast" delete of the directory.
-					Directory.Delete (fullPath, true);
-					temporaryRemovedDirectories.Add (directory);
-				} catch (UnauthorizedAccessException ex) {
-					// if that fails we probably have readonly files (or locked files)
-					// so try to make them writable and try again.
-					try {
-						Files.SetDirectoryWriteable (fullPath);
-						Directory.Delete (fullPath, true);
-						temporaryRemovedDirectories.Add (directory);
-					} 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) {
+					while (retryCount <= attempts) {
 						try {
-							fullPath = Files.ToLongPath (fullPath);
-							Log.LogDebugMessage ("Trying long path: " + fullPath);
+							// 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);
-						} catch (Exception inner) {
-							Log.LogUnhandledException (TaskPrefix, ex);
-							Log.LogUnhandledException (TaskPrefix, inner);
+							break;
+						} catch (Exception e) {
+							switch (e) {
+								case DirectoryNotFoundException:
+									if (OS.IsWindows) {
+										fullPath = Files.ToLongPath (fullPath);
+										Log.LogDebugMessage ("Trying long path: " + fullPath);
+										break;
+									}
+									throw;
+								case UnauthorizedAccessException:
+								case IOException:
+									int code = Marshal.GetHRForException(e);
+									if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount >= attempts) {
+										throw;
+									};
+									break;
+								default:
+									throw;
+							}
+							Thread.Sleep (delay);
+							retryCount++;
 						}
-					} else {
-						Log.LogUnhandledException (TaskPrefix, ex);
 					}
 				} catch (Exception ex) {
 					Log.LogUnhandledException (TaskPrefix, ex);

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
 {
@@ -83,5 +85,44 @@ public void LongPath ()
 			Assert.AreEqual (1, task.RemovedDirectories.Length, "Changes should have been made.");
 			DirectoryAssert.DoesNotExist (tempDirectory);
 		}
+
+		[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)) {
+				Assert.IsFalse (task.Execute (), "task.Execute() should have failed.");
+				Assert.AreEqual (0, task.RemovedDirectories.Length, "Changes should not have been made.");
+				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 succeeded.");
+			Assert.AreEqual (1, task.RemovedDirectories.Length, "Changes should have been made.");
+			DirectoryAssert.DoesNotExist (tempDirectory);
+			await t;
+		}
 	}
 }