Skip to content

Commit 7873034

Browse files
[xabt] <RemoveDirFixed/> retry policy for temp directories (#10363)
Fixes: #10338 The `RemoveDirFixed` MSBuild task could take 8+ seconds due to excessive retry attempts (10 retries × 1000ms = up to 10 seconds) when cleaning temporary directories. **Solution Implemented**: - Added configurable `RetryAttempts` and `RetryDelayMs` properties to `RemoveDirFixed` task - Reduced retry attempts to 3 for temp directory cleanup (max ~3 seconds vs 10+ seconds) - Maintained full backward compatibility for all other usage scenarios **Files Modified**: 1. **RemoveDirFixed.cs**: Added retry configuration properties with backward compatibility 2. **Xamarin.Android.Common.targets**: Updated 2 temp cleanup calls to use `RetryAttempts="3"` 3. **RemoveDirTests.cs**: Added comprehensive tests for new functionality **Performance Impact**: Reduces temp directory cleanup time from 8+ seconds to ~3 seconds maximum, directly addressing the reported issue while preserving robust retry behavior where actually needed. Co-authored-by: Jonathan Peppers <[email protected]>
1 parent c24828c commit 7873034

File tree

3 files changed

+71
-2
lines changed

3 files changed

+71
-2
lines changed

src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ public override bool RunTask ()
5656
continue;
5757
}
5858
int retryCount = 0;
59-
int attempts = Files.GetFileWriteRetryAttempts ();
60-
int delay = Files.GetFileWriteRetryDelay ();
59+
int attempts = RetryAttempts >= 0 ? RetryAttempts : Files.GetFileWriteRetryAttempts ();
60+
int delay = RetryDelayMs >= 0 ? RetryDelayMs : Files.GetFileWriteRetryDelay ();
6161
try {
6262
while (retryCount <= attempts) {
6363
try {
@@ -105,6 +105,18 @@ public override bool RunTask ()
105105
[Required]
106106
public ITaskItem [] Directories { get; set; } = [];
107107

108+
/// <summary>
109+
/// Number of retry attempts when directory deletion fails.
110+
/// Defaults to Files.GetFileWriteRetryAttempts() if not specified.
111+
/// </summary>
112+
public int RetryAttempts { get; set; } = -1;
113+
114+
/// <summary>
115+
/// Delay in milliseconds between retry attempts.
116+
/// Defaults to Files.GetFileWriteRetryDelay() if not specified.
117+
/// </summary>
118+
public int RetryDelayMs { get; set; } = -1;
119+
108120
[Output]
109121
public ITaskItem []? RemovedDirectories { get; set; }
110122
}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/RemoveDirTests.cs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ string NewFile (string fileName = "")
4545
Directories = new [] { new TaskItem (tempDirectory) },
4646
};
4747

48+
RemoveDirFixed CreateTaskWithRetry (int retryAttempts, int retryDelayMs) => new RemoveDirFixed {
49+
BuildEngine = new MockBuildEngine (TestContext.Out, messages: messages),
50+
Directories = new [] { new TaskItem (tempDirectory) },
51+
RetryAttempts = retryAttempts,
52+
RetryDelayMs = retryDelayMs,
53+
};
54+
4855
[Test]
4956
public void NormalDelete ()
5057
{
@@ -124,5 +131,50 @@ public async TPL.Task DirectoryInUseWithRetry ()
124131
DirectoryAssert.DoesNotExist (tempDirectory);
125132
await t;
126133
}
134+
135+
[Test]
136+
public void CustomRetryAttempts ()
137+
{
138+
NewFile ();
139+
var task = CreateTaskWithRetry (retryAttempts: 3, retryDelayMs: 100);
140+
Assert.IsTrue (task.Execute (), "task.Execute() should have succeeded.");
141+
Assert.AreEqual (1, task.RemovedDirectories.Length, "Changes should have been made.");
142+
DirectoryAssert.DoesNotExist (tempDirectory);
143+
}
144+
145+
[Test]
146+
public void DefaultRetryBehavior ()
147+
{
148+
NewFile ();
149+
var task = CreateTask ();
150+
// Verify default behavior still works
151+
Assert.IsTrue (task.Execute (), "task.Execute() should have succeeded.");
152+
Assert.AreEqual (1, task.RemovedDirectories.Length, "Changes should have been made.");
153+
DirectoryAssert.DoesNotExist (tempDirectory);
154+
}
155+
156+
[Test]
157+
public async TPL.Task DirectoryInUseWithCustomRetry ()
158+
{
159+
if (!OS.IsWindows) {
160+
Assert.Ignore ("This is not an issue on non-Windows platforms.");
161+
return;
162+
}
163+
var file = NewFile ();
164+
// Test with default retry behavior
165+
var task = CreateTask ();
166+
var ev = new ManualResetEvent (false);
167+
var t = TPL.Task.Run (async () => {
168+
using (var f = File.OpenWrite (file)) {
169+
ev.Set ();
170+
await TPL.Task.Delay (800); // Should succeed on first retry with default timing
171+
}
172+
});
173+
ev.WaitOne ();
174+
Assert.IsTrue (task.Execute (), "task.Execute() should have succeeded.");
175+
Assert.AreEqual (1, task.RemovedDirectories.Length, "Changes should have been made.");
176+
DirectoryAssert.DoesNotExist (tempDirectory);
177+
await t;
178+
}
127179
}
128180
}

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
292292

293293
<_AndroidUseCLR Condition=" '$(_AndroidRuntime)' == 'CoreCLR' ">True</_AndroidUseCLR>
294294
<_AndroidUseCLR Condition=" '$(_AndroidRuntime)' != 'CoreCLR' ">False</_AndroidUseCLR>
295+
296+
<!-- Default retry attempts for temporary directory cleanup -->
297+
<_AndroidDeleteTempRetryAttempts Condition=" '$(_AndroidDeleteTempRetryAttempts)' == '' ">3</_AndroidDeleteTempRetryAttempts>
295298
</PropertyGroup>
296299

297300
<Choose>
@@ -1325,6 +1328,7 @@ because xbuild doesn't support framework reference assemblies.
13251328
<!-- Delete our temporary directory -->
13261329
<RemoveDirFixed
13271330
Directories="$(ResgenTemporaryDirectory)"
1331+
RetryAttempts="$(_AndroidDeleteTempRetryAttempts)"
13281332
ContinueOnError="WarnAndContinue"
13291333
/>
13301334

@@ -1996,6 +2000,7 @@ because xbuild doesn't support framework reference assemblies.
19962000
<!-- Delete our temporary directory -->
19972001
<RemoveDirFixed
19982002
Directories="$(AaptTemporaryDirectory)"
2003+
RetryAttempts="$(_AndroidDeleteTempRetryAttempts)"
19992004
ContinueOnError="WarnAndContinue"
20002005
/>
20012006

0 commit comments

Comments
 (0)