Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] Add retry ability to RemoveDirFixed (#9409
Browse files Browse the repository at this point in the history 
)

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
  • Loading branch information
@dellis1972 @jonathanpeppers
dellis1972 authored and jonathanpeppers committed Oct 23, 2024
1 parent 152295c commit 278e101
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 25 deletions.
60 changes: 35 additions & 25 deletions src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand Down
41 changes: 41 additions & 0 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
}
}
}

0 comments on commit 278e101

Please sign in to comment.