From 56c5b56e51518191d6accca10dd9a9d345d53dce Mon Sep 17 00:00:00 2001 From: huulinh99 Date: Wed, 1 Apr 2026 13:50:54 +0700 Subject: [PATCH 1/7] Fix race condition in WriteLinesToFile transactional mode (#13323) --- src/Tasks.UnitTests/WriteLinesToFile_Tests.cs | 2 +- src/Tasks/FileIO/WriteLinesToFile.cs | 21 ++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/Tasks.UnitTests/WriteLinesToFile_Tests.cs b/src/Tasks.UnitTests/WriteLinesToFile_Tests.cs index 55f0efb9870..d783c58fd65 100644 --- a/src/Tasks.UnitTests/WriteLinesToFile_Tests.cs +++ b/src/Tasks.UnitTests/WriteLinesToFile_Tests.cs @@ -453,7 +453,7 @@ public void TransactionalModeHandlesConcurrentWritesSuccessfully() } [Fact] - public void TransactionalModePreservesAllData() + public void TransactionalModeSucceedsWithConcurrentOverwrites() { using (var testEnv = TestEnvironment.Create(_output)) { diff --git a/src/Tasks/FileIO/WriteLinesToFile.cs b/src/Tasks/FileIO/WriteLinesToFile.cs index 27dccd84925..b359c638d3b 100644 --- a/src/Tasks/FileIO/WriteLinesToFile.cs +++ b/src/Tasks/FileIO/WriteLinesToFile.cs @@ -215,12 +215,23 @@ private bool SaveAtomically(AbsolutePath filePath, string contentsAsString, Enco temporaryFilePath = null; // Mark as successfully moved return !Log.HasLoggedErrors; } - catch (IOException moveEx) + catch (IOException) { - // Move failed, log and return - string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); - Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, moveEx.Message, lockedFileMessage); - return !Log.HasLoggedErrors; + // File may have been created by another thread between Replace and Move. + // Retry with Replace since the file now exists. + try + { + System.IO.File.Replace(temporaryFilePath, filePath, null, true); + temporaryFilePath = null; // Mark as successfully replaced + return !Log.HasLoggedErrors; + } + catch (IOException moveEx) + { + // Move failed, log and return + string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); + Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, moveEx.Message, lockedFileMessage); + return !Log.HasLoggedErrors; + } } } catch (IOException) From 6ed6b985b78a49baed06156153645755f67fa089 Mon Sep 17 00:00:00 2001 From: Nguyen Huu Linh <43189610+huulinhnguyen-dev@users.noreply.github.com> Date: Wed, 1 Apr 2026 13:59:14 +0700 Subject: [PATCH 2/7] Update src/Tasks/FileIO/WriteLinesToFile.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Tasks/FileIO/WriteLinesToFile.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Tasks/FileIO/WriteLinesToFile.cs b/src/Tasks/FileIO/WriteLinesToFile.cs index b359c638d3b..bc5e5b575a4 100644 --- a/src/Tasks/FileIO/WriteLinesToFile.cs +++ b/src/Tasks/FileIO/WriteLinesToFile.cs @@ -225,11 +225,11 @@ private bool SaveAtomically(AbsolutePath filePath, string contentsAsString, Enco temporaryFilePath = null; // Mark as successfully replaced return !Log.HasLoggedErrors; } - catch (IOException moveEx) + catch (IOException replaceEx) { - // Move failed, log and return + // Replace failed, log and return string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); - Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, moveEx.Message, lockedFileMessage); + Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, replaceEx.Message, lockedFileMessage); return !Log.HasLoggedErrors; } } From 16ac14c6e59878470dace2c3b5afb76d415f8321 Mon Sep 17 00:00:00 2001 From: huulinh99 Date: Wed, 1 Apr 2026 14:04:29 +0700 Subject: [PATCH 3/7] Fix WriteLinesToFile race condition: only retry Replace when destination exists --- src/Tasks/FileIO/WriteLinesToFile.cs | 35 +++++++++++++++++----------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/Tasks/FileIO/WriteLinesToFile.cs b/src/Tasks/FileIO/WriteLinesToFile.cs index b359c638d3b..ac0e8e7ec26 100644 --- a/src/Tasks/FileIO/WriteLinesToFile.cs +++ b/src/Tasks/FileIO/WriteLinesToFile.cs @@ -215,23 +215,30 @@ private bool SaveAtomically(AbsolutePath filePath, string contentsAsString, Enco temporaryFilePath = null; // Mark as successfully moved return !Log.HasLoggedErrors; } - catch (IOException) + catch (IOException moveEx) { - // File may have been created by another thread between Replace and Move. - // Retry with Replace since the file now exists. - try + // Only retry with Replace if the destination now exists (concurrent write race). + if (System.IO.File.Exists(filePath)) { - System.IO.File.Replace(temporaryFilePath, filePath, null, true); - temporaryFilePath = null; // Mark as successfully replaced - return !Log.HasLoggedErrors; - } - catch (IOException moveEx) - { - // Move failed, log and return - string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); - Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, moveEx.Message, lockedFileMessage); - return !Log.HasLoggedErrors; + try + { + System.IO.File.Replace(temporaryFilePath, filePath, null, true); + temporaryFilePath = null; // Mark as successfully replaced + return !Log.HasLoggedErrors; + } + catch (IOException replaceEx) + { + // Both attempts failed; log the original move error as the root cause. + string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); + Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, moveEx.Message + " " + replaceEx.Message, lockedFileMessage); + return !Log.HasLoggedErrors; + } } + + // Destination doesn't exist; move failed for a different reason. + string lockedFileDiagnostics = LockCheck.GetLockedFileMessage(filePath); + Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, moveEx.Message, lockedFileDiagnostics); + return !Log.HasLoggedErrors; } } catch (IOException) From c7bc50fcea3db7e5c72fc0a1cab02d3fd244318f Mon Sep 17 00:00:00 2001 From: Nguyen Huu Linh <43189610+huulinhnguyen-dev@users.noreply.github.com> Date: Wed, 1 Apr 2026 14:11:38 +0700 Subject: [PATCH 4/7] Update src/Tasks/FileIO/WriteLinesToFile.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Tasks/FileIO/WriteLinesToFile.cs | 41 ++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/Tasks/FileIO/WriteLinesToFile.cs b/src/Tasks/FileIO/WriteLinesToFile.cs index ac0e8e7ec26..f50465894d5 100644 --- a/src/Tasks/FileIO/WriteLinesToFile.cs +++ b/src/Tasks/FileIO/WriteLinesToFile.cs @@ -220,19 +220,38 @@ private bool SaveAtomically(AbsolutePath filePath, string contentsAsString, Enco // Only retry with Replace if the destination now exists (concurrent write race). if (System.IO.File.Exists(filePath)) { - try - { - System.IO.File.Replace(temporaryFilePath, filePath, null, true); - temporaryFilePath = null; // Mark as successfully replaced - return !Log.HasLoggedErrors; - } - catch (IOException replaceEx) + IOException lastReplaceException = null; + + // Retry Replace a few times with a small delay, mirroring the later Replace retry logic. + for (int retry = 0; retry < 3; retry++) { - // Both attempts failed; log the original move error as the root cause. - string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); - Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, moveEx.Message + " " + replaceEx.Message, lockedFileMessage); - return !Log.HasLoggedErrors; + try + { + if (retry > 0) + { + System.Threading.Thread.Sleep(10); + } + + System.IO.File.Replace(temporaryFilePath, filePath, null, true); + temporaryFilePath = null; // Mark as successfully replaced + return !Log.HasLoggedErrors; + } + catch (IOException replaceEx) + { + lastReplaceException = replaceEx; + // Continue to next retry. + } } + + // All Replace retries failed; log the original move error as the root cause. + string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); + string replaceMessage = lastReplaceException != null ? lastReplaceException.Message : string.Empty; + Log.LogErrorWithCodeFromResources( + "WriteLinesToFile.ErrorOrWarning", + filePath.OriginalValue, + moveEx.Message + (string.IsNullOrEmpty(replaceMessage) ? string.Empty : " " + replaceMessage), + lockedFileMessage); + return !Log.HasLoggedErrors; } // Destination doesn't exist; move failed for a different reason. From e407fd18a20acacf100e554593f63f14ef93f32d Mon Sep 17 00:00:00 2001 From: Nguyen Huu Linh <43189610+huulinhnguyen-dev@users.noreply.github.com> Date: Wed, 1 Apr 2026 14:19:05 +0700 Subject: [PATCH 5/7] Update src/Tasks/FileIO/WriteLinesToFile.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Tasks/FileIO/WriteLinesToFile.cs | 39 +++++++++++++++++----------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/Tasks/FileIO/WriteLinesToFile.cs b/src/Tasks/FileIO/WriteLinesToFile.cs index f50465894d5..a2ab4c11a08 100644 --- a/src/Tasks/FileIO/WriteLinesToFile.cs +++ b/src/Tasks/FileIO/WriteLinesToFile.cs @@ -220,29 +220,38 @@ private bool SaveAtomically(AbsolutePath filePath, string contentsAsString, Enco // Only retry with Replace if the destination now exists (concurrent write race). if (System.IO.File.Exists(filePath)) { - IOException lastReplaceException = null; - // Retry Replace a few times with a small delay, mirroring the later Replace retry logic. - for (int retry = 0; retry < 3; retry++) + bool TryReplaceWithRetry(string sourcePath, string destinationPath, out IOException lastReplaceException) { - try + lastReplaceException = null; + + for (int retry = 0; retry < 3; retry++) { - if (retry > 0) + try { - System.Threading.Thread.Sleep(10); - } + if (retry > 0) + { + System.Threading.Thread.Sleep(10); + } - System.IO.File.Replace(temporaryFilePath, filePath, null, true); - temporaryFilePath = null; // Mark as successfully replaced - return !Log.HasLoggedErrors; - } - catch (IOException replaceEx) - { - lastReplaceException = replaceEx; - // Continue to next retry. + System.IO.File.Replace(sourcePath, destinationPath, null, true); + return true; + } + catch (IOException replaceEx) + { + lastReplaceException = replaceEx; + // Continue to next retry. + } } + + return false; } + if (TryReplaceWithRetry(temporaryFilePath, filePath, out IOException lastReplaceException)) + { + temporaryFilePath = null; // Mark as successfully replaced + return !Log.HasLoggedErrors; + } // All Replace retries failed; log the original move error as the root cause. string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); string replaceMessage = lastReplaceException != null ? lastReplaceException.Message : string.Empty; From 9e37e8c4a84487179922bc47232de8f68fb6445a Mon Sep 17 00:00:00 2001 From: huulinh99 Date: Wed, 1 Apr 2026 15:26:28 +0700 Subject: [PATCH 6/7] Fix WriteLinesToFile race condition: only retry Replace when destination exists --- src/Tasks/FileIO/WriteLinesToFile.cs | 44 +++++----------------------- 1 file changed, 8 insertions(+), 36 deletions(-) diff --git a/src/Tasks/FileIO/WriteLinesToFile.cs b/src/Tasks/FileIO/WriteLinesToFile.cs index a2ab4c11a08..ac0e8e7ec26 100644 --- a/src/Tasks/FileIO/WriteLinesToFile.cs +++ b/src/Tasks/FileIO/WriteLinesToFile.cs @@ -220,47 +220,19 @@ private bool SaveAtomically(AbsolutePath filePath, string contentsAsString, Enco // Only retry with Replace if the destination now exists (concurrent write race). if (System.IO.File.Exists(filePath)) { - // Retry Replace a few times with a small delay, mirroring the later Replace retry logic. - bool TryReplaceWithRetry(string sourcePath, string destinationPath, out IOException lastReplaceException) + try { - lastReplaceException = null; - - for (int retry = 0; retry < 3; retry++) - { - try - { - if (retry > 0) - { - System.Threading.Thread.Sleep(10); - } - - System.IO.File.Replace(sourcePath, destinationPath, null, true); - return true; - } - catch (IOException replaceEx) - { - lastReplaceException = replaceEx; - // Continue to next retry. - } - } - - return false; + System.IO.File.Replace(temporaryFilePath, filePath, null, true); + temporaryFilePath = null; // Mark as successfully replaced + return !Log.HasLoggedErrors; } - - if (TryReplaceWithRetry(temporaryFilePath, filePath, out IOException lastReplaceException)) + catch (IOException replaceEx) { - temporaryFilePath = null; // Mark as successfully replaced + // Both attempts failed; log the original move error as the root cause. + string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); + Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, moveEx.Message + " " + replaceEx.Message, lockedFileMessage); return !Log.HasLoggedErrors; } - // All Replace retries failed; log the original move error as the root cause. - string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); - string replaceMessage = lastReplaceException != null ? lastReplaceException.Message : string.Empty; - Log.LogErrorWithCodeFromResources( - "WriteLinesToFile.ErrorOrWarning", - filePath.OriginalValue, - moveEx.Message + (string.IsNullOrEmpty(replaceMessage) ? string.Empty : " " + replaceMessage), - lockedFileMessage); - return !Log.HasLoggedErrors; } // Destination doesn't exist; move failed for a different reason. From af749295666ff04c959b0aa94691bede00295a8a Mon Sep 17 00:00:00 2001 From: Nguyen Huu Linh Date: Fri, 10 Apr 2026 11:00:52 +0700 Subject: [PATCH 7/7] Fix WriteLinesToFile race condition: use File.Move with overwrite instead of Replace --- src/Tasks/FileIO/WriteLinesToFile.cs | 113 +++++++++++---------------- 1 file changed, 46 insertions(+), 67 deletions(-) diff --git a/src/Tasks/FileIO/WriteLinesToFile.cs b/src/Tasks/FileIO/WriteLinesToFile.cs index ac0e8e7ec26..6f76e46f3eb 100644 --- a/src/Tasks/FileIO/WriteLinesToFile.cs +++ b/src/Tasks/FileIO/WriteLinesToFile.cs @@ -22,6 +22,29 @@ public class WriteLinesToFile : TaskExtension, IIncrementalTask, IMultiThreadabl // Default encoding taken from System.IO.WriteAllText() private static readonly Encoding s_defaultEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); + /// + /// Moves to , overwriting it if it exists. + /// Uses Microsoft.IO.File on .NET Framework to access the overwrite overload, + /// which is available natively in System.IO.File on .NET 6+. + /// + private static void MoveFileWithOverwrite(string source, string destination) + { +#if NETFRAMEWORK + // Microsoft.IO.Redist backports File.Move(overwrite) to .NET Framework. + Microsoft.IO.File.Move(source, destination, overwrite: true); +#elif NET + // File.Move(overwrite) is available natively on .NET 5+. + System.IO.File.Move(source, destination, overwrite: true); +#else + // netstandard2.0: overwrite overload unavailable; delete destination first. + if (System.IO.File.Exists(destination)) + { + System.IO.File.Delete(destination); + } + System.IO.File.Move(source, destination); +#endif + } + /// public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback; @@ -181,8 +204,10 @@ private bool ExecuteTransactional(AbsolutePath filePath, string directoryPath, s } /// - /// Saves content to file atomically using a temporary file, following the Visual Studio editor pattern. - /// This is for overwrite mode where we write the entire content. + /// Saves content to file atomically using a temporary file. + /// Writes to a temp file first, then moves it to the target with overwrite, + /// which handles both the "target exists" and "target doesn't exist" cases in a + /// single call and eliminates the race window present in a check-then-act pattern. /// private bool SaveAtomically(AbsolutePath filePath, string contentsAsString, Encoding encoding) { @@ -197,81 +222,33 @@ private bool SaveAtomically(AbsolutePath filePath, string contentsAsString, Enco // Write content to temporary file System.IO.File.WriteAllText(temporaryFilePath, contentsAsString, encoding); - // Attempt to atomically replace target file with temporary file - try + // Atomically move temp file to target, overwriting if it already exists. + // Using overwrite: true handles concurrent writes without a race condition — + // both "target doesn't exist" and "target already exists" cases are covered + // by a single operation, with no window between them. + const int maxAttempts = 3; + Exception lastException = null; + for (int attempt = 1; attempt <= maxAttempts; attempt++) { - // Replace the contents of filePath with the contents of the temporary using File.Replace - // to preserve the various attributes of the original file. - System.IO.File.Replace(temporaryFilePath, filePath, null, true); - temporaryFilePath = null; // Mark as successfully replaced - return !Log.HasLoggedErrors; - } - catch (FileNotFoundException) - { - // The target file doesn't exist, which is fine. Move the temp file to target. - try + if (attempt > 1) { - System.IO.File.Move(temporaryFilePath, filePath); - temporaryFilePath = null; // Mark as successfully moved - return !Log.HasLoggedErrors; - } - catch (IOException moveEx) - { - // Only retry with Replace if the destination now exists (concurrent write race). - if (System.IO.File.Exists(filePath)) - { - try - { - System.IO.File.Replace(temporaryFilePath, filePath, null, true); - temporaryFilePath = null; // Mark as successfully replaced - return !Log.HasLoggedErrors; - } - catch (IOException replaceEx) - { - // Both attempts failed; log the original move error as the root cause. - string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); - Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, moveEx.Message + " " + replaceEx.Message, lockedFileMessage); - return !Log.HasLoggedErrors; - } - } - - // Destination doesn't exist; move failed for a different reason. - string lockedFileDiagnostics = LockCheck.GetLockedFileMessage(filePath); - Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, moveEx.Message, lockedFileDiagnostics); - return !Log.HasLoggedErrors; - } - } - catch (IOException) - { - // Replace failed (likely file is locked). Retry a few times with small delay. - for (int retry = 1; retry < 3; retry++) - { - try - { - System.Threading.Thread.Sleep(10); - System.IO.File.Replace(temporaryFilePath, filePath, null, true); - temporaryFilePath = null; // Mark as successfully replaced - return !Log.HasLoggedErrors; - } - catch (IOException) - { - // Continue to next retry - } + // Log the retry so concurrency issues are visible when diagnosing builds. + Log.LogMessageFromResources(MessageImportance.High, "WriteLinesToFile.Retry", + filePath.OriginalValue, attempt, maxAttempts, lastException.Message); + System.Threading.Thread.Sleep(10); } - // Retries exhausted. Try simple write as fallback. try { - System.IO.File.WriteAllText(filePath, contentsAsString, encoding); - temporaryFilePath = null; // Mark temp as not needed + MoveFileWithOverwrite(temporaryFilePath, filePath); + temporaryFilePath = null; return !Log.HasLoggedErrors; } - catch (Exception fallbackEx) when (ExceptionHandling.IsIoRelatedException(fallbackEx)) + catch (Exception ex) when (attempt < maxAttempts && ExceptionHandling.IsIoRelatedException(ex)) { - string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath); - Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath.OriginalValue, fallbackEx.Message, lockedFileMessage); - return !Log.HasLoggedErrors; + lastException = ex; } + // On the last attempt, the exception propagates to the outer handler. } } catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) @@ -298,6 +275,8 @@ private bool SaveAtomically(AbsolutePath filePath, string contentsAsString, Enco } } } + + return !Log.HasLoggedErrors; } ///