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..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,63 +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 - { - // 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 - { - System.IO.File.Move(temporaryFilePath, filePath); - temporaryFilePath = null; // Mark as successfully moved - 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) + // 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 failed (likely file is locked). Retry a few times with small delay. - for (int retry = 1; retry < 3; retry++) + if (attempt > 1) { - 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)) @@ -280,6 +275,8 @@ private bool SaveAtomically(AbsolutePath filePath, string contentsAsString, Enco } } } + + return !Log.HasLoggedErrors; } ///