Fix incremental history saving when the history queue is full

PSReadLine/History.cs

@@ -257,12 +257,12 @@ private void IncrementalHistoryWrite()
                 i -= 1;
             }
 
-            WriteHistoryRange(i + 1, _history.Count - 1, File.AppendText);
+            WriteHistoryRange(i + 1, _history.Count - 1, overwritten: false);
         }
 
         private void SaveHistoryAtExit()
         {
-            WriteHistoryRange(0, _history.Count - 1, File.CreateText);
+            WriteHistoryRange(0, _history.Count - 1, overwritten: true);
         }
 
         private int historyErrorReportedCount;
@@ -321,18 +321,20 @@ private bool WithHistoryFileMutexDo(int timeout, Action action)
             return true;
         }
 
-        private void WriteHistoryRange(int start, int end, Func<string, StreamWriter> fileOpener)
+        private void WriteHistoryRange(int start, int end, bool overwritten)
         {
             WithHistoryFileMutexDo(100, () =>
             {
-                if (!MaybeReadHistoryFile())
-                    return;
-
                 bool retry = true;
+                // Get the new content since the last sync.
+                List<string> historyLines = overwritten ? null : ReadHistoryFileIncrementally();
+
+                try
+                {
                     retry_after_creating_directory:
                     try
                     {
-                    using (var file = fileOpener(Options.HistorySavePath))
+                        using (var file = overwritten ? File.CreateText(Options.HistorySavePath) : File.AppendText(Options.HistorySavePath))
                         {
                             for (var i = start; i <= end; i++)
                             {
@@ -359,14 +361,26 @@ private void WriteHistoryRange(int start, int end, Func<string, StreamWriter> fi
                             goto retry_after_creating_directory;
                         }
                     }
-            });
                 }
-
-        private bool MaybeReadHistoryFile()
+                finally
                 {
-            if (Options.HistorySaveStyle == HistorySaveStyle.SaveIncrementally)
+                    if (historyLines != null)
                     {
-                return WithHistoryFileMutexDo(1000, () =>
+                        // Populate new history from other sessions to the history queue after we are done
+                        // with writing the specified range to the file.
+                        // We do it at this point to make sure the range of history items from 'start' to
+                        // 'end' do not get changed before the writing to the file.
+                        UpdateHistoryFromFile(historyLines, fromDifferentSession: true, fromInitialRead: false);
+                    }
+                }
+            });
+        }
+
+        /// <summary>
+        /// Helper method to read the incremental part of the history file.
+        /// Note: the call to this method should be guarded by the mutex that protects the history file.
+        /// </summary>
+        private List<string> ReadHistoryFileIncrementally()
         {
             var fileInfo = new FileInfo(Options.HistorySavePath);
             if (fileInfo.Exists && fileInfo.Length != _historyFileLastSavedSize)
@@ -382,9 +396,24 @@ private bool MaybeReadHistoryFile()
                         historyLines.Add(sr.ReadLine());
                     }
                 }
-                        UpdateHistoryFromFile(historyLines, fromDifferentSession: true, fromInitialRead: false);
 
                 _historyFileLastSavedSize = fileInfo.Length;
+                return historyLines.Count > 0 ? historyLines : null;
+            }
+
+            return null;
+        }
+
+        private bool MaybeReadHistoryFile()
+        {
+            if (Options.HistorySaveStyle == HistorySaveStyle.SaveIncrementally)
+            {
+                return WithHistoryFileMutexDo(1000, () =>
+                {
+                    List<string> historyLines = ReadHistoryFileIncrementally();
+                    if (historyLines != null)
+                    {
+                        UpdateHistoryFromFile(historyLines, fromDifferentSession: true, fromInitialRead: false);
                     }
                 });
             }

test/HistoryTest.cs

@@ -2,6 +2,7 @@
 using System.IO;
 using System.Linq;
 using System.Management.Automation;
+using System.Reflection;
 using Microsoft.PowerShell;
 using Xunit;
 
@@ -33,6 +34,60 @@ public void History()
             Test("dir c*", Keys(_.UpArrow, _.UpArrow, _.DownArrow));
         }
 
+        [SkippableFact]
+        public void ParallelHistorySaving()
+        {
+            TestSetup(KeyMode.Cmd);
+
+            string historySavingFile = Path.GetTempFileName();
+            var options = new SetPSReadLineOption {
+                HistorySaveStyle = HistorySaveStyle.SaveIncrementally,
+                MaximumHistoryCount = 3,
+            };
+
+            typeof(SetPSReadLineOption)
+                .GetField("_historySavePath", BindingFlags.Instance | BindingFlags.NonPublic)
+                .SetValue(options, historySavingFile);
+
+            PSConsoleReadLine.SetOptions(options);
+
+            // Set the initial history items.
+            string[] initialHistoryItems = new[] { "gcm help", "dir ~" };
+            SetHistory(initialHistoryItems);
+
+            // The initial history items should be saved to file.
+            string[] text = File.ReadAllLines(historySavingFile);
+            Assert.Equal(initialHistoryItems.Length, text.Length);
+            for (int i = 0; i < text.Length; i++)
+            {
+                Assert.Equal(initialHistoryItems[i], text[i]);
+            }
+
+            // Add another line to the file to mimic the new history saving from a different session.
+            using (var file = File.AppendText(historySavingFile))
+            {
+                file.WriteLine("cd Downloads");
+            }
+
+            PSConsoleReadLine.AddToHistory("cd Documents");
+
+            string[] expectedSavedLines = new[] { "gcm help", "dir ~", "cd Downloads", "cd Documents" };
+            text = File.ReadAllLines(historySavingFile);
+            Assert.Equal(expectedSavedLines.Length, text.Length);
+            for (int i = 0; i < text.Length; i++)
+            {
+                Assert.Equal(expectedSavedLines[i], text[i]);
+            }
+
+            string[] expectedHistoryItems = new[] { "dir ~", "cd Documents", "cd Downloads" };
+            var historyItems = PSConsoleReadLine.GetHistoryItems();
+            Assert.Equal(expectedHistoryItems.Length, historyItems.Length);
+            for (int i = 0; i < historyItems.Length; i++)
+            {
+                Assert.Equal(expectedHistoryItems[i], historyItems[i].CommandLine);
+            }
+        }
+
         [SkippableFact]
         public void SensitiveHistoryDefaultBehavior()
         {