From 60fae1924e2d71f31dc1ed05f7346fd7874d6636 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 22 Oct 2024 16:57:55 +0100 Subject: [PATCH] [Microsoft.Android.Build.BaseTasks] retry when copying files (#245) Context: https://github.com/dotnet/android/issues/9133 Context: https://learn.microsoft.com/visualstudio/msbuild/copy-task?view=vs-2022 We sometimes get collisions between the Design-Time-Build (or AntiVirus) and our main build. This can result in errors such as: Error (active) XALNS7019 System.UnauthorizedAccessException: Access to the path 'D:\Projects\MauiApp2\obj\Debug\net9.0-android\android\assets\armeabi-v7a\MauiApp2.dll' is denied. at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath) at System.IO.File.InternalDelete(String path, Boolean checkHost) at Microsoft.Android.Build.Tasks.Files.CopyIfChanged(String source, String destination) in /Users/runner/work/1/s/xamarin-android/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/Files.cs:line 125 at Xamarin.Android.Tasks.MonoAndroidHelper.CopyAssemblyAndSymbols(String source, String destination) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs:line 344 at Xamarin.Android.Tasks.LinkAssembliesNoShrink.CopyIfChanged(ITaskItem source, ITaskItem destination) in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs:line 161 at Xamarin.Android.Tasks.LinkAssembliesNoShrink.RunTask() in /Users/runner/work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs:line 76 at Microsoft.Android.Build.Tasks.AndroidTask.Execute() in /Users/runner/work/1/s/xamarin-android/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AndroidTask.cs:line 25 MauiApp2 (net9.0-android) C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\34.99.0-preview.6.340\tools\Xamarin.Android.Common.targets 1407 If we look at the [MSBuild `` task][0] we see that it has a retry system in the cases of `UnauthorizedAccessException` or `IOException` when the code is `ACCESS_DENIED` or `ERROR_SHARING_VIOLATION`. The `` task also has public `Retries` and `RetryDelayMilliseconds` properties to control behavior. Duplicate that kind of logic into our `Files.Copy*IfChanged()` helper methods. This should give our builds a bit more resiliency to these kinds of issues. Instead of adding new `Files.Copy*IfChanged()` method overloads which accept "retries" and "retryDelay" parameters, we instead use environment variables to allow overriding these values: * `DOTNET_ANDROID_FILE_WRITE_RETRY_ATTEMPTS`: The number of times to try to retry a copy operation; corresponds to the `Copy.Retries` MSBuild task property. The default value is 10. * `DOTNET_ANDROID_FILE_WRITE_RETRY_DELAY_MS`: The amount of time, in milliseconds, to delay between attempted copies; corresponds to the `Copy.RetryDelayMilliseconds` MSBuild task property. The default value is 1000 ms. [0]: https://github.com/dotnet/msbuild/blob/main/src/Tasks/Copy.cs#L897 --- .../Files.cs | 99 +++++++++++++++++++ .../Microsoft.Android.Build.BaseTasks.csproj | 1 + .../FilesTests.cs | 30 ++++++ 3 files changed, 130 insertions(+) diff --git a/src/Microsoft.Android.Build.BaseTasks/Files.cs b/src/Microsoft.Android.Build.BaseTasks/Files.cs index 9172847b..77b6e313 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Files.cs +++ b/src/Microsoft.Android.Build.BaseTasks/Files.cs @@ -9,11 +9,25 @@ using System.Text; using Xamarin.Tools.Zip; using Microsoft.Build.Utilities; +using System.Threading; +using System.Reflection.Metadata; +using System.Runtime.InteropServices; +using System.Collections; namespace Microsoft.Android.Build.Tasks { public static class Files { + const int ERROR_ACCESS_DENIED = -2147024891; + const int ERROR_SHARING_VIOLATION = -2147024864; + + const int DEFAULT_FILE_WRITE_RETRY_ATTEMPTS = 10; + + const int DEFAULT_FILE_WRITE_RETRY_DELAY_MS = 1000; + + static int fileWriteRetry = -1; + static int fileWriteRetryDelay = -1; + /// /// Windows has a MAX_PATH limit of 260 characters /// See: https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#maximum-path-length-limitation @@ -28,6 +42,37 @@ public static class Files public static readonly Encoding UTF8withoutBOM = new UTF8Encoding (encoderShouldEmitUTF8Identifier: false); readonly static byte[] Utf8Preamble = Encoding.UTF8.GetPreamble (); + /// + /// Checks for the environment variable DOTNET_ANDROID_FILE_WRITE_RETRY_ATTEMPTS to + /// see if a custom value for the number of times to retry writing a file has been + /// set. + /// + /// The value of DOTNET_ANDROID_FILE_WRITE_RETRY_ATTEMPTS or the default of DEFAULT_FILE_WRITE_RETRY_ATTEMPTS + public static int GetFileWriteRetryAttempts () + { + if (fileWriteRetry == -1) { + var retryVariable = Environment.GetEnvironmentVariable ("DOTNET_ANDROID_FILE_WRITE_RETRY_ATTEMPTS"); + if (string.IsNullOrEmpty (retryVariable) || !int.TryParse (retryVariable, out fileWriteRetry)) + fileWriteRetry = DEFAULT_FILE_WRITE_RETRY_ATTEMPTS; + } + return fileWriteRetry; + } + + /// + /// Checks for the environment variable DOTNET_ANDROID_FILE_WRITE_RETRY_DELAY_MS to + /// see if a custom value for the delay between trying to write a file has been + /// set. + /// + /// The value of DOTNET_ANDROID_FILE_WRITE_RETRY_DELAY_MS or the default of DEFAULT_FILE_WRITE_RETRY_DELAY_MS + public static int GetFileWriteRetryDelay () + { + if (fileWriteRetryDelay == -1) { + var delayVariable = Environment.GetEnvironmentVariable ("DOTNET_ANDROID_FILE_WRITE_RETRY_DELAY_MS"); + if (string.IsNullOrEmpty (delayVariable) || !int.TryParse (delayVariable, out fileWriteRetryDelay)) + fileWriteRetryDelay = DEFAULT_FILE_WRITE_RETRY_DELAY_MS; + } + return fileWriteRetryDelay; + } /// /// Converts a full path to a \\?\ prefixed path that works on all Windows machines when over 260 characters /// NOTE: requires a *full path*, use sparingly @@ -111,6 +156,33 @@ public static bool ArchiveZip (string target, Action archiver) } public static bool CopyIfChanged (string source, string destination) + { + int retryCount = 0; + int attempts = GetFileWriteRetryAttempts (); + int delay = GetFileWriteRetryDelay (); + while (retryCount <= attempts) { + try { + return CopyIfChangedOnce (source, destination); + } catch (Exception e) { + switch (e) { + case UnauthorizedAccessException: + case IOException: + int code = Marshal.GetHRForException (e); + if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount == attempts) { + throw; + }; + break; + default: + throw; + } + } + retryCount++; + Thread.Sleep (delay); + } + return false; + } + + public static bool CopyIfChangedOnce (string source, string destination) { if (HasFileChanged (source, destination)) { var directory = Path.GetDirectoryName (destination); @@ -157,6 +229,33 @@ public static bool CopyIfBytesChanged (byte[] bytes, string destination) } public static bool CopyIfStreamChanged (Stream stream, string destination) + { + int retryCount = 0; + int attempts = GetFileWriteRetryAttempts (); + int delay = GetFileWriteRetryDelay (); + while (retryCount <= attempts) { + try { + return CopyIfStreamChangedOnce (stream, destination); + } catch (Exception e) { + switch (e) { + case UnauthorizedAccessException: + case IOException: + int code = Marshal.GetHRForException (e); + if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount >= attempts) { + throw; + }; + break; + default: + throw; + } + } + retryCount++; + Thread.Sleep (delay); + } + return false; + } + + public static bool CopyIfStreamChangedOnce (Stream stream, string destination) { if (HasStreamChanged (stream, destination)) { var directory = Path.GetDirectoryName (destination); diff --git a/src/Microsoft.Android.Build.BaseTasks/Microsoft.Android.Build.BaseTasks.csproj b/src/Microsoft.Android.Build.BaseTasks/Microsoft.Android.Build.BaseTasks.csproj index f3602ce5..e60a7b46 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Microsoft.Android.Build.BaseTasks.csproj +++ b/src/Microsoft.Android.Build.BaseTasks/Microsoft.Android.Build.BaseTasks.csproj @@ -9,6 +9,7 @@ true ..\..\product.snk $(VendorPrefix)Microsoft.Android.Build.BaseTasks$(VendorSuffix) + 12.0 diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index e1953cb5..db02d2f7 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -6,6 +6,8 @@ using System.IO; using System.Runtime.InteropServices; using System.Text; +using System.Threading; +using System.Threading.Tasks; using Xamarin.Tools.Zip; using Microsoft.Android.Build.Tasks; @@ -436,6 +438,34 @@ public void CopyIfStreamChanged_CasingChange () } } + [Test] + public async Task CopyIfChanged_LockedFile () + { + var dest = NewFile (contents: "foo", fileName: "foo_locked"); + var src = NewFile (contents: "foo0", fileName: "foo"); + using (var file = File.OpenWrite (dest)) { + Assert.Throws (() => Files.CopyIfChanged (src, dest)); + } + src = NewFile (contents: "foo1", fileName: "foo"); + Assert.IsTrue (Files.CopyIfChanged (src, dest)); + src = NewFile (contents: "foo2", fileName: "foo"); + dest = NewFile (contents: "foo", fileName: "foo_locked2"); + var ev = new ManualResetEvent (false); + var task = Task.Run (async () => { + var file = File.Open (dest, FileMode.OpenOrCreate, FileAccess.Write, FileShare.Read); + try { + ev.Set (); + await Task.Delay (2500); + } finally { + file.Close(); + file.Dispose (); + } + }); + ev.WaitOne (); + Assert.IsTrue (Files.CopyIfChanged (src, dest)); + await task; + } + [Test] public void ExtractAll () {