From 18cd5971ecb582b9b017eac374636d230737f241 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Mon, 14 Oct 2024 16:49:56 +0100 Subject: [PATCH 01/19] Add a 3 attempt retry to copy files --- .../Files.cs | 58 +++++++++++++++++++ .../Microsoft.Android.Build.BaseTasks.csproj | 1 + .../FilesTests.cs | 11 ++++ 3 files changed, 70 insertions(+) diff --git a/src/Microsoft.Android.Build.BaseTasks/Files.cs b/src/Microsoft.Android.Build.BaseTasks/Files.cs index 9172847b..a40f3478 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Files.cs +++ b/src/Microsoft.Android.Build.BaseTasks/Files.cs @@ -9,6 +9,10 @@ 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 { @@ -110,7 +114,36 @@ public static bool ArchiveZip (string target, Action archiver) return changed; } + const int DEFAULT_COPYIFCHANGED_RETRIES = 3; + const int ERROR_ACCESS_DENIED = 5; + const int DEFAULT_FILE_WRITE_RETRY_DELAY_MS = 1000; + public static bool CopyIfChanged (string source, string destination) + { + int retryCount = 0; + while (retryCount < DEFAULT_COPYIFCHANGED_RETRIES) { + try { + return CopyIfChangedRetry (source, destination); + } catch (Exception e) { + switch (e) { + case UnauthorizedAccessException: + case IOException: + int code = Marshal.GetHRForException(e); + if (code != ERROR_ACCESS_DENIED || retryCount == DEFAULT_COPYIFCHANGED_RETRIES) { + throw; + }; + break; + default: + throw; + } + } + retryCount++; + Thread.Sleep(DEFAULT_FILE_WRITE_RETRY_DELAY_MS); + } + return false; + } + + public static bool CopyIfChangedRetry (string source, string destination) { if (HasFileChanged (source, destination)) { var directory = Path.GetDirectoryName (destination); @@ -157,6 +190,31 @@ public static bool CopyIfBytesChanged (byte[] bytes, string destination) } public static bool CopyIfStreamChanged (Stream stream, string destination) + { + int retryCount = 0; + while (retryCount < DEFAULT_COPYIFCHANGED_RETRIES) { + try { + return CopyIfStreamChangedRetry (stream, destination); + } catch (Exception e) { + switch (e) { + case UnauthorizedAccessException: + case IOException: + int code = Marshal.GetHRForException(e); + if (code != ERROR_ACCESS_DENIED || retryCount == DEFAULT_COPYIFCHANGED_RETRIES) { + throw; + }; + break; + default: + throw; + } + } + retryCount++; + Thread.Sleep(DEFAULT_FILE_WRITE_RETRY_DELAY_MS); + } + return false; + } + + public static bool CopyIfStreamChangedRetry (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..f9ebfca9 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) + latest diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index e1953cb5..7cfe593d 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -436,6 +436,17 @@ public void CopyIfStreamChanged_CasingChange () } } + [Test] + public void CopyIfChanged_LockedFile () + { + var dest = NewFile (contents: "foo", fileName: "foo_locked"); + var src = NewFile (contents: "foo", fileName: "foo"); + using (var file = File.OpenWrite (dest)) { + Assert.IsFalse (Files.CopyIfChanged (src, dest)); + } + Assert.IsTrue (Files.CopyIfChanged (src, dest)); + } + [Test] public void ExtractAll () { From ad2256cba4545a4240d127bd0469e849fb117db2 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 08:50:39 +0100 Subject: [PATCH 02/19] Try to add a file lock test --- .../FilesTests.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index 7cfe593d..5f3d9674 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -6,6 +6,7 @@ using System.IO; using System.Runtime.InteropServices; using System.Text; +using System.Threading.Tasks; using Xamarin.Tools.Zip; using Microsoft.Android.Build.Tasks; @@ -442,9 +443,16 @@ public void CopyIfChanged_LockedFile () var dest = NewFile (contents: "foo", fileName: "foo_locked"); var src = NewFile (contents: "foo", fileName: "foo"); using (var file = File.OpenWrite (dest)) { - Assert.IsFalse (Files.CopyIfChanged (src, dest)); + Assert.Throws (() => Files.CopyIfChanged (src, dest)); } Assert.IsTrue (Files.CopyIfChanged (src, dest)); + src = NewFile (contents: "foo2", fileName: "foo"); + Task.Run (async () => { + using (var file = File.OpenWrite (dest)) { + await Task.Delay (500); + } + }); + Assert.IsTrue (Files.CopyIfChanged (src, dest)); } [Test] From f21811c13e5e4d5229a9b8c419df4fd75710f7b6 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 08:55:58 +0100 Subject: [PATCH 03/19] Try to add a file lock test --- tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index 5f3d9674..1e973fba 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -441,10 +441,11 @@ public void CopyIfStreamChanged_CasingChange () public void CopyIfChanged_LockedFile () { var dest = NewFile (contents: "foo", fileName: "foo_locked"); - var src = NewFile (contents: "foo", fileName: "foo"); + 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"); Task.Run (async () => { From 19ef288837d1106a980adc2be7cce53d1eb0dd1a Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 09:00:41 +0100 Subject: [PATCH 04/19] Fix formatting --- src/Microsoft.Android.Build.BaseTasks/Files.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Android.Build.BaseTasks/Files.cs b/src/Microsoft.Android.Build.BaseTasks/Files.cs index a40f3478..6b27d97a 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Files.cs +++ b/src/Microsoft.Android.Build.BaseTasks/Files.cs @@ -128,7 +128,7 @@ public static bool CopyIfChanged (string source, string destination) switch (e) { case UnauthorizedAccessException: case IOException: - int code = Marshal.GetHRForException(e); + int code = Marshal.GetHRForException (e); if (code != ERROR_ACCESS_DENIED || retryCount == DEFAULT_COPYIFCHANGED_RETRIES) { throw; }; @@ -138,7 +138,7 @@ public static bool CopyIfChanged (string source, string destination) } } retryCount++; - Thread.Sleep(DEFAULT_FILE_WRITE_RETRY_DELAY_MS); + Thread.Sleep (DEFAULT_FILE_WRITE_RETRY_DELAY_MS); } return false; } @@ -199,7 +199,7 @@ public static bool CopyIfStreamChanged (Stream stream, string destination) switch (e) { case UnauthorizedAccessException: case IOException: - int code = Marshal.GetHRForException(e); + int code = Marshal.GetHRForException (e); if (code != ERROR_ACCESS_DENIED || retryCount == DEFAULT_COPYIFCHANGED_RETRIES) { throw; }; @@ -209,7 +209,7 @@ public static bool CopyIfStreamChanged (Stream stream, string destination) } } retryCount++; - Thread.Sleep(DEFAULT_FILE_WRITE_RETRY_DELAY_MS); + Thread.Sleep (DEFAULT_FILE_WRITE_RETRY_DELAY_MS); } return false; } From 835cb0683d3e9b83f04557cf9bed570e495e0ead Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 09:11:01 +0100 Subject: [PATCH 05/19] Try to add a file lock test --- tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index 1e973fba..c998a00a 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -450,7 +450,7 @@ public void CopyIfChanged_LockedFile () src = NewFile (contents: "foo2", fileName: "foo"); Task.Run (async () => { using (var file = File.OpenWrite (dest)) { - await Task.Delay (500); + await Task.Delay (200); } }); Assert.IsTrue (Files.CopyIfChanged (src, dest)); From 331c76b9c8e644ffe2743da0241951693a575f01 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 09:17:37 +0100 Subject: [PATCH 06/19] Try to add a file lock test --- .../FilesTests.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index c998a00a..d494b8df 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -438,7 +438,7 @@ public void CopyIfStreamChanged_CasingChange () } [Test] - public void CopyIfChanged_LockedFile () + public async Task CopyIfChanged_LockedFile () { var dest = NewFile (contents: "foo", fileName: "foo_locked"); var src = NewFile (contents: "foo0", fileName: "foo"); @@ -448,12 +448,17 @@ public void CopyIfChanged_LockedFile () src = NewFile (contents: "foo1", fileName: "foo"); Assert.IsTrue (Files.CopyIfChanged (src, dest)); src = NewFile (contents: "foo2", fileName: "foo"); - Task.Run (async () => { - using (var file = File.OpenWrite (dest)) { + var task = Task.Run (async () => { + var file = File.OpenWrite (dest); + try { await Task.Delay (200); + } finally { + file.Close(); + file.Dispose (); } }); Assert.IsTrue (Files.CopyIfChanged (src, dest)); + await task; } [Test] From 1002567c423f7932c89d4987636506a1f4a97009 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 09:44:00 +0100 Subject: [PATCH 07/19] Rework test --- tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index d494b8df..110d7544 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -449,7 +449,7 @@ public async Task CopyIfChanged_LockedFile () Assert.IsTrue (Files.CopyIfChanged (src, dest)); src = NewFile (contents: "foo2", fileName: "foo"); var task = Task.Run (async () => { - var file = File.OpenWrite (dest); + var file = File.Open (dest, FileMode.OpenOrCreate, FileAccess.Write, FileShare.Read); try { await Task.Delay (200); } finally { From 0c0524365c23a001dc1918cbf980787d026d6eb7 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 16:33:05 +0100 Subject: [PATCH 08/19] Add new methods to get the retry attempts and delay --- .../Files.cs | 53 +++++++++++++++---- .../Microsoft.Android.Build.BaseTasks.csproj | 2 +- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.Android.Build.BaseTasks/Files.cs b/src/Microsoft.Android.Build.BaseTasks/Files.cs index 6b27d97a..cc128afa 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Files.cs +++ b/src/Microsoft.Android.Build.BaseTasks/Files.cs @@ -18,6 +18,12 @@ namespace Microsoft.Android.Build.Tasks { public static class Files { + const int ERROR_ACCESS_DENIED = 5; + + const int DEFAULT_FILE_WRITE_RETRY_ATTEMPTS = 10; + + const int DEFAULT_FILE_WRITE_RETRY_DELAY_MS = 1000; + /// /// 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 @@ -32,6 +38,33 @@ 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 () + { + var retryVariable = Environment.GetEnvironmentVariable ("DOTNET_ANDROID_FILE_WRITE_RETRY_ATTEMPTS"); + if (!string.IsNullOrEmpty (retryVariable) && int.TryParse (retryVariable, out int retry)) + return retry; + return DEFAULT_FILE_WRITE_RETRY_ATTEMPTS; + } + + /// + /// 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 () + { + var delayVariable = Environment.GetEnvironmentVariable ("DOTNET_ANDROID_FILE_WRITE_RETRY_DELAY_MS"); + if (!string.IsNullOrEmpty (delayVariable) && int.TryParse (delayVariable, out int delay)) + return delay; + return DEFAULT_FILE_WRITE_RETRY_DELAY_MS; + } /// /// 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 @@ -114,14 +147,12 @@ public static bool ArchiveZip (string target, Action archiver) return changed; } - const int DEFAULT_COPYIFCHANGED_RETRIES = 3; - const int ERROR_ACCESS_DENIED = 5; - const int DEFAULT_FILE_WRITE_RETRY_DELAY_MS = 1000; - public static bool CopyIfChanged (string source, string destination) { int retryCount = 0; - while (retryCount < DEFAULT_COPYIFCHANGED_RETRIES) { + int attempts = GetFileWriteRetryAttempts (); + int delay = GetFileWriteRetryDelay (); + while (retryCount < attempts) { try { return CopyIfChangedRetry (source, destination); } catch (Exception e) { @@ -129,7 +160,7 @@ public static bool CopyIfChanged (string source, string destination) case UnauthorizedAccessException: case IOException: int code = Marshal.GetHRForException (e); - if (code != ERROR_ACCESS_DENIED || retryCount == DEFAULT_COPYIFCHANGED_RETRIES) { + if (code != ERROR_ACCESS_DENIED || retryCount == attempts) { throw; }; break; @@ -138,7 +169,7 @@ public static bool CopyIfChanged (string source, string destination) } } retryCount++; - Thread.Sleep (DEFAULT_FILE_WRITE_RETRY_DELAY_MS); + Thread.Sleep (delay); } return false; } @@ -192,7 +223,9 @@ public static bool CopyIfBytesChanged (byte[] bytes, string destination) public static bool CopyIfStreamChanged (Stream stream, string destination) { int retryCount = 0; - while (retryCount < DEFAULT_COPYIFCHANGED_RETRIES) { + int attempts = GetFileWriteRetryAttempts (); + int delay = GetFileWriteRetryDelay (); + while (retryCount < attempts) { try { return CopyIfStreamChangedRetry (stream, destination); } catch (Exception e) { @@ -200,7 +233,7 @@ public static bool CopyIfStreamChanged (Stream stream, string destination) case UnauthorizedAccessException: case IOException: int code = Marshal.GetHRForException (e); - if (code != ERROR_ACCESS_DENIED || retryCount == DEFAULT_COPYIFCHANGED_RETRIES) { + if (code != ERROR_ACCESS_DENIED || retryCount == attempts) { throw; }; break; @@ -209,7 +242,7 @@ public static bool CopyIfStreamChanged (Stream stream, string destination) } } retryCount++; - Thread.Sleep (DEFAULT_FILE_WRITE_RETRY_DELAY_MS); + Thread.Sleep (delay); } return false; } 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 f9ebfca9..fea20d0c 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Microsoft.Android.Build.BaseTasks.csproj +++ b/src/Microsoft.Android.Build.BaseTasks/Microsoft.Android.Build.BaseTasks.csproj @@ -9,7 +9,7 @@ true ..\..\product.snk $(VendorPrefix)Microsoft.Android.Build.BaseTasks$(VendorSuffix) - latest + 13.0 From 88e66fd39ee924878b3aebe312db542d0ddd6977 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 19:01:10 +0100 Subject: [PATCH 09/19] Bump test delay --- tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index 110d7544..e3f056fb 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -451,7 +451,7 @@ public async Task CopyIfChanged_LockedFile () var task = Task.Run (async () => { var file = File.Open (dest, FileMode.OpenOrCreate, FileAccess.Write, FileShare.Read); try { - await Task.Delay (200); + await Task.Delay (2500); } finally { file.Close(); file.Dispose (); From 5cf1fd9470e1e7c71ad39ba8c40bd1b4325f357e Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 15 Oct 2024 15:03:47 -0400 Subject: [PATCH 10/19] Update Microsoft.Android.Build.BaseTasks.csproj $(LangVersion)=13 breaks CI: ``` Invalid option '13.0' for /langversion. Use '/langversion:?' to list supported values. ``` Let's try 12.0? --- .../Microsoft.Android.Build.BaseTasks.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 fea20d0c..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,7 +9,7 @@ true ..\..\product.snk $(VendorPrefix)Microsoft.Android.Build.BaseTasks$(VendorSuffix) - 13.0 + 12.0 From a878a46527a46137fb651ec62994070fcea1213f Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 20:27:57 +0100 Subject: [PATCH 11/19] cache the values --- .../Files.cs | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Android.Build.BaseTasks/Files.cs b/src/Microsoft.Android.Build.BaseTasks/Files.cs index cc128afa..1147d844 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Files.cs +++ b/src/Microsoft.Android.Build.BaseTasks/Files.cs @@ -24,6 +24,9 @@ public static class Files 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 @@ -46,10 +49,12 @@ public static class Files /// The value of DOTNET_ANDROID_FILE_WRITE_RETRY_ATTEMPTS or the default of DEFAULT_FILE_WRITE_RETRY_ATTEMPTS public static int GetFileWriteRetryAttempts () { - var retryVariable = Environment.GetEnvironmentVariable ("DOTNET_ANDROID_FILE_WRITE_RETRY_ATTEMPTS"); - if (!string.IsNullOrEmpty (retryVariable) && int.TryParse (retryVariable, out int retry)) - return retry; - return DEFAULT_FILE_WRITE_RETRY_ATTEMPTS; + 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; } /// @@ -60,10 +65,12 @@ public static int GetFileWriteRetryAttempts () /// The value of DOTNET_ANDROID_FILE_WRITE_RETRY_DELAY_MS or the default of DEFAULT_FILE_WRITE_RETRY_DELAY_MS public static int GetFileWriteRetryDelay () { - var delayVariable = Environment.GetEnvironmentVariable ("DOTNET_ANDROID_FILE_WRITE_RETRY_DELAY_MS"); - if (!string.IsNullOrEmpty (delayVariable) && int.TryParse (delayVariable, out int delay)) - return delay; - return DEFAULT_FILE_WRITE_RETRY_DELAY_MS; + 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 From adbcb05574e933ceaaa5c8554e6e455a2194fccf Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 22:04:46 +0100 Subject: [PATCH 12/19] wait for file lock --- tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index e3f056fb..5d6c6a02 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -457,6 +457,7 @@ public async Task CopyIfChanged_LockedFile () file.Dispose (); } }); + await Task.Delay (1000); Assert.IsTrue (Files.CopyIfChanged (src, dest)); await task; } From 5d0b33126f52511fae8258869835bf85e0c341a3 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 22:37:43 +0100 Subject: [PATCH 13/19] wait for file lock --- tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index 5d6c6a02..82ee1c6d 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -6,6 +6,7 @@ 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; @@ -448,16 +449,18 @@ public async Task CopyIfChanged_LockedFile () src = NewFile (contents: "foo1", fileName: "foo"); Assert.IsTrue (Files.CopyIfChanged (src, dest)); src = NewFile (contents: "foo2", fileName: "foo"); + 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 (); } }); - await Task.Delay (1000); + ev.WaitOne (); Assert.IsTrue (Files.CopyIfChanged (src, dest)); await task; } From f33483f1d52aa4b01b9fade6e9ac5ba9c20b011c Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 22:43:07 +0100 Subject: [PATCH 14/19] wait for file lock --- tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs index 82ee1c6d..db02d2f7 100644 --- a/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/FilesTests.cs @@ -449,6 +449,7 @@ public async Task CopyIfChanged_LockedFile () 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); From 09e3569e5d9172c2faca5a382212c5992bc90d66 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 22:50:51 +0100 Subject: [PATCH 15/19] Add Logging --- src/Microsoft.Android.Build.BaseTasks/Files.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Microsoft.Android.Build.BaseTasks/Files.cs b/src/Microsoft.Android.Build.BaseTasks/Files.cs index 1147d844..444be18b 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Files.cs +++ b/src/Microsoft.Android.Build.BaseTasks/Files.cs @@ -160,6 +160,7 @@ public static bool CopyIfChanged (string source, string destination) int attempts = GetFileWriteRetryAttempts (); int delay = GetFileWriteRetryDelay (); while (retryCount < attempts) { + Console.WriteLine ($"{nameof(CopyIfChanged)}: Copy Attempt {retryCount}."); try { return CopyIfChangedRetry (source, destination); } catch (Exception e) { @@ -168,6 +169,7 @@ public static bool CopyIfChanged (string source, string destination) case IOException: int code = Marshal.GetHRForException (e); if (code != ERROR_ACCESS_DENIED || retryCount == attempts) { + Console.WriteLine ($"{nameof(CopyIfChanged)}: Throwing {code} {retryCount}."); throw; }; break; @@ -176,6 +178,7 @@ public static bool CopyIfChanged (string source, string destination) } } retryCount++; + Console.WriteLine ($"{nameof(CopyIfChanged)}: Copy Sleeping {delay}."); Thread.Sleep (delay); } return false; From 7e5ea0e184c18c0b8298b592a5774b2bdaaf75a6 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 23:01:03 +0100 Subject: [PATCH 16/19] ff --- src/Microsoft.Android.Build.BaseTasks/Files.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Android.Build.BaseTasks/Files.cs b/src/Microsoft.Android.Build.BaseTasks/Files.cs index 444be18b..6bf60139 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Files.cs +++ b/src/Microsoft.Android.Build.BaseTasks/Files.cs @@ -18,7 +18,8 @@ namespace Microsoft.Android.Build.Tasks { public static class Files { - const int ERROR_ACCESS_DENIED = 5; + const int ERROR_ACCESS_DENIED = -2147024891; + const int ERROR_SHARING_VIOLATION = -2147024864; const int DEFAULT_FILE_WRITE_RETRY_ATTEMPTS = 10; @@ -168,8 +169,8 @@ public static bool CopyIfChanged (string source, string destination) case UnauthorizedAccessException: case IOException: int code = Marshal.GetHRForException (e); - if (code != ERROR_ACCESS_DENIED || retryCount == attempts) { - Console.WriteLine ($"{nameof(CopyIfChanged)}: Throwing {code} {retryCount}."); + if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount == attempts) { + Console.WriteLine ($"{nameof(CopyIfChanged)}: Throwing {e}\ncode:{code}\nretryCount:{retryCount}."); throw; }; break; @@ -243,7 +244,7 @@ public static bool CopyIfStreamChanged (Stream stream, string destination) case UnauthorizedAccessException: case IOException: int code = Marshal.GetHRForException (e); - if (code != ERROR_ACCESS_DENIED || retryCount == attempts) { + if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount == attempts) { throw; }; break; From f94ca7cdbd78da741f7904437c3bb8e4a564eaca Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 15 Oct 2024 23:15:19 +0100 Subject: [PATCH 17/19] ff --- src/Microsoft.Android.Build.BaseTasks/Files.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Android.Build.BaseTasks/Files.cs b/src/Microsoft.Android.Build.BaseTasks/Files.cs index 6bf60139..36316cc7 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Files.cs +++ b/src/Microsoft.Android.Build.BaseTasks/Files.cs @@ -160,8 +160,7 @@ public static bool CopyIfChanged (string source, string destination) int retryCount = 0; int attempts = GetFileWriteRetryAttempts (); int delay = GetFileWriteRetryDelay (); - while (retryCount < attempts) { - Console.WriteLine ($"{nameof(CopyIfChanged)}: Copy Attempt {retryCount}."); + while (retryCount <= attempts) { try { return CopyIfChangedRetry (source, destination); } catch (Exception e) { @@ -170,7 +169,6 @@ public static bool CopyIfChanged (string source, string destination) case IOException: int code = Marshal.GetHRForException (e); if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount == attempts) { - Console.WriteLine ($"{nameof(CopyIfChanged)}: Throwing {e}\ncode:{code}\nretryCount:{retryCount}."); throw; }; break; @@ -179,7 +177,6 @@ public static bool CopyIfChanged (string source, string destination) } } retryCount++; - Console.WriteLine ($"{nameof(CopyIfChanged)}: Copy Sleeping {delay}."); Thread.Sleep (delay); } return false; @@ -236,7 +233,7 @@ public static bool CopyIfStreamChanged (Stream stream, string destination) int retryCount = 0; int attempts = GetFileWriteRetryAttempts (); int delay = GetFileWriteRetryDelay (); - while (retryCount < attempts) { + while (retryCount <= attempts) { try { return CopyIfStreamChangedRetry (stream, destination); } catch (Exception e) { From 6dce7b139da9cb90a73902fe28652e171fe4c78a Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 22 Oct 2024 16:34:05 +0100 Subject: [PATCH 18/19] Rename *Retry to *Once so it makes more sense --- src/Microsoft.Android.Build.BaseTasks/Files.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Android.Build.BaseTasks/Files.cs b/src/Microsoft.Android.Build.BaseTasks/Files.cs index 36316cc7..03c9bcf7 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Files.cs +++ b/src/Microsoft.Android.Build.BaseTasks/Files.cs @@ -162,7 +162,7 @@ public static bool CopyIfChanged (string source, string destination) int delay = GetFileWriteRetryDelay (); while (retryCount <= attempts) { try { - return CopyIfChangedRetry (source, destination); + return CopyIfChangedOnce (source, destination); } catch (Exception e) { switch (e) { case UnauthorizedAccessException: @@ -182,7 +182,7 @@ public static bool CopyIfChanged (string source, string destination) return false; } - public static bool CopyIfChangedRetry (string source, string destination) + public static bool CopyIfChangedOnce (string source, string destination) { if (HasFileChanged (source, destination)) { var directory = Path.GetDirectoryName (destination); @@ -235,7 +235,7 @@ public static bool CopyIfStreamChanged (Stream stream, string destination) int delay = GetFileWriteRetryDelay (); while (retryCount <= attempts) { try { - return CopyIfStreamChangedRetry (stream, destination); + return CopyIfStreamChangedOnce (stream, destination); } catch (Exception e) { switch (e) { case UnauthorizedAccessException: @@ -255,7 +255,7 @@ public static bool CopyIfStreamChanged (Stream stream, string destination) return false; } - public static bool CopyIfStreamChangedRetry (Stream stream, string destination) + public static bool CopyIfStreamChangedOnce (Stream stream, string destination) { if (HasStreamChanged (stream, destination)) { var directory = Path.GetDirectoryName (destination); From 7ca6220178ee126b2e9b9e6c957900e26d0b6d73 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 22 Oct 2024 16:53:09 +0100 Subject: [PATCH 19/19] Update src/Microsoft.Android.Build.BaseTasks/Files.cs Co-authored-by: Jonathan Peppers --- src/Microsoft.Android.Build.BaseTasks/Files.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Android.Build.BaseTasks/Files.cs b/src/Microsoft.Android.Build.BaseTasks/Files.cs index 03c9bcf7..77b6e313 100644 --- a/src/Microsoft.Android.Build.BaseTasks/Files.cs +++ b/src/Microsoft.Android.Build.BaseTasks/Files.cs @@ -241,7 +241,7 @@ public static bool CopyIfStreamChanged (Stream stream, string destination) case UnauthorizedAccessException: case IOException: int code = Marshal.GetHRForException (e); - if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount == attempts) { + if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount >= attempts) { throw; }; break;