diff --git a/PSReadLine/History.cs b/PSReadLine/History.cs index 5cd1fbc9..f7f21003 100644 --- a/PSReadLine/History.cs +++ b/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,70 +321,99 @@ private bool WithHistoryFileMutexDo(int timeout, Action action) return true; } - private void WriteHistoryRange(int start, int end, Func fileOpener) + private void WriteHistoryRange(int start, int end, bool overwritten) { WithHistoryFileMutexDo(100, () => { - if (!MaybeReadHistoryFile()) - return; - bool retry = true; - retry_after_creating_directory: + // Get the new content since the last sync. + List historyLines = overwritten ? null : ReadHistoryFileIncrementally(); + try { - using (var file = fileOpener(Options.HistorySavePath)) + retry_after_creating_directory: + try { - for (var i = start; i <= end; i++) + using (var file = overwritten ? File.CreateText(Options.HistorySavePath) : File.AppendText(Options.HistorySavePath)) { - HistoryItem item = _history[i]; - item._saved = true; + for (var i = start; i <= end; i++) + { + HistoryItem item = _history[i]; + item._saved = true; - // Actually, skip writing sensitive items to file. - if (item._sensitive) { continue; } + // Actually, skip writing sensitive items to file. + if (item._sensitive) { continue; } - var line = item.CommandLine.Replace("\n", "`\n"); - file.WriteLine(line); + var line = item.CommandLine.Replace("\n", "`\n"); + file.WriteLine(line); + } + } + var fileInfo = new FileInfo(Options.HistorySavePath); + _historyFileLastSavedSize = fileInfo.Length; + } + catch (DirectoryNotFoundException) + { + // Try making the directory, but just once + if (retry) + { + retry = false; + Directory.CreateDirectory(Path.GetDirectoryName(Options.HistorySavePath)); + goto retry_after_creating_directory; } } - var fileInfo = new FileInfo(Options.HistorySavePath); - _historyFileLastSavedSize = fileInfo.Length; } - catch (DirectoryNotFoundException) + finally { - // Try making the directory, but just once - if (retry) + if (historyLines != null) { - retry = false; - Directory.CreateDirectory(Path.GetDirectoryName(Options.HistorySavePath)); - goto retry_after_creating_directory; + // 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); } } }); } + /// + /// 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. + /// + private List ReadHistoryFileIncrementally() + { + var fileInfo = new FileInfo(Options.HistorySavePath); + if (fileInfo.Exists && fileInfo.Length != _historyFileLastSavedSize) + { + var historyLines = new List(); + using (var fs = new FileStream(Options.HistorySavePath, FileMode.Open)) + using (var sr = new StreamReader(fs)) + { + fs.Seek(_historyFileLastSavedSize, SeekOrigin.Begin); + + while (!sr.EndOfStream) + { + historyLines.Add(sr.ReadLine()); + } + } + + _historyFileLastSavedSize = fileInfo.Length; + return historyLines.Count > 0 ? historyLines : null; + } + + return null; + } + private bool MaybeReadHistoryFile() { if (Options.HistorySaveStyle == HistorySaveStyle.SaveIncrementally) { return WithHistoryFileMutexDo(1000, () => { - var fileInfo = new FileInfo(Options.HistorySavePath); - if (fileInfo.Exists && fileInfo.Length != _historyFileLastSavedSize) + List historyLines = ReadHistoryFileIncrementally(); + if (historyLines != null) { - var historyLines = new List(); - using (var fs = new FileStream(Options.HistorySavePath, FileMode.Open)) - using (var sr = new StreamReader(fs)) - { - fs.Seek(_historyFileLastSavedSize, SeekOrigin.Begin); - - while (!sr.EndOfStream) - { - historyLines.Add(sr.ReadLine()); - } - } UpdateHistoryFromFile(historyLines, fromDifferentSession: true, fromInitialRead: false); - - _historyFileLastSavedSize = fileInfo.Length; } }); } diff --git a/test/HistoryTest.cs b/test/HistoryTest.cs index 03947cf5..8b071ea1 100644 --- a/test/HistoryTest.cs +++ b/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() {