diff --git a/docs/mdsource/tray.source.md b/docs/mdsource/tray.source.md index 67de9ae0..de2ec701 100644 --- a/docs/mdsource/tray.source.md +++ b/docs/mdsource/tray.source.md @@ -49,6 +49,18 @@ Clicking "file1" or "file2" will delete file1 or file2 respectively. The drop do "Accept all" will accept all pending moves and all pending deletes. +### Locked files + +If accepting a move fails because the files are locked by another process (for example the snapshot is open in Microsoft Word), a prompt is shown listing the locked files and the locking processes: + + + + * "Ignore" leaves the move pending so it can be accepted later. + * "Kill [process] and accept" kills the locking processes and accepts the move. + * "Kill and accept all pending" kills the locking processes and accepts all pending moves, killing any other locking processes without further prompts. + * "Always kill" kills the locking processes and accepts the move. The choice is stored in settings, so future locked files are killed without prompting. It can be toggled in the Options dialog. + + ### Discard Discard will clear all currently tracked items. @@ -79,6 +91,11 @@ By default, when a diff is opened, the temp file will be on the left and the tar Control the [max instances to launch setting](docs/diff-tool.md#maxinstancestolaunch). +#### Always kill locking processes + +When accepting a move with [locked files](#locked-files), kill the locking processes without prompting. + + #### Discard all HotKey Registers a system wide HotKey to discard pending: diff --git a/docs/tray.md b/docs/tray.md index cfa30d75..27bd7ce5 100644 --- a/docs/tray.md +++ b/docs/tray.md @@ -56,6 +56,18 @@ Clicking "file1" or "file2" will delete file1 or file2 respectively. The drop do "Accept all" will accept all pending moves and all pending deletes. +### Locked files + +If accepting a move fails because the files are locked by another process (for example the snapshot is open in Microsoft Word), a prompt is shown listing the locked files and the locking processes: + + + + * "Ignore" leaves the move pending so it can be accepted later. + * "Kill [process] and accept" kills the locking processes and accepts the move. + * "Kill and accept all pending" kills the locking processes and accepts all pending moves, killing any other locking processes without further prompts. + * "Always kill" kills the locking processes and accepts the move. The choice is stored in settings, so future locked files are killed without prompting. It can be toggled in the Options dialog. + + ### Discard Discard will clear all currently tracked items. @@ -86,6 +98,11 @@ By default, when a diff is opened, the temp file will be on the left and the tar Control the [max instances to launch setting](docs/diff-tool.md#maxinstancestolaunch). +#### Always kill locking processes + +When accepting a move with [locked files](#locked-files), kill the locking processes without prompting. + + #### Discard all HotKey Registers a system wide HotKey to discard pending: diff --git a/src/DiffEngineTray.Tests/FileLockKillerTest.cs b/src/DiffEngineTray.Tests/FileLockKillerTest.cs index 92a7bc9a..bdca6311 100644 --- a/src/DiffEngineTray.Tests/FileLockKillerTest.cs +++ b/src/DiffEngineTray.Tests/FileLockKillerTest.cs @@ -1,5 +1,44 @@ public class FileLockKillerTest { + [Test] + public async Task GetLockingProcesses_WhenFileNotLocked_ReturnsEmpty() + { + var file = Path.Combine(Path.GetTempPath(), $"FileLockKillerTest_{Guid.NewGuid()}.txt"); + try + { + File.WriteAllText(file, "content"); + var result = FileLockKiller.GetLockingProcesses(file); + await Assert.That(result).IsEmpty(); + } + finally + { + File.Delete(file); + } + } + + [Test] + public async Task GetLockingProcesses_WhenFileLocked_ReturnsProcess() + { + var file = Path.Combine(Path.GetTempPath(), $"FileLockKillerTest_{Guid.NewGuid()}.txt"); + File.WriteAllText(file, "content"); + + var lockProcess = FileLockUtils.StartFileLockProcess(file); + + try + { + await Assert.That(FileLockUtils.IsFileLocked(file)).IsTrue(); + + var result = FileLockKiller.GetLockingProcesses(file); + + await Assert.That(result.Select(_ => _.ProcessId)).Contains(lockProcess.Id); + } + finally + { + FileLockUtils.Cleanup(lockProcess); + File.Delete(file); + } + } + [Test] public async Task KillLockingProcesses_WhenFileNotLocked_ReturnsFalse() { @@ -30,11 +69,11 @@ public async Task KillLockingProcesses_WhenFileLocked_KillsProcess() var file = Path.Combine(Path.GetTempPath(), $"FileLockKillerTest_{Guid.NewGuid()}.txt"); File.WriteAllText(file, "content"); - var lockProcess = StartFileLockProcess(file); + var lockProcess = FileLockUtils.StartFileLockProcess(file); try { - await Assert.That(IsFileLocked(file)).IsTrue(); + await Assert.That(FileLockUtils.IsFileLocked(file)).IsTrue(); var result = FileLockKiller.KillLockingProcesses(file); @@ -45,12 +84,7 @@ public async Task KillLockingProcesses_WhenFileLocked_KillsProcess() } finally { - if (!lockProcess.HasExited) - { - lockProcess.Kill(); - } - - lockProcess.Dispose(); + FileLockUtils.Cleanup(lockProcess); File.Delete(file); } } @@ -63,11 +97,11 @@ public async Task MoveSucceedsAfterKillingLockingProcess() File.WriteAllText(file, "content"); File.WriteAllText(tempFile, "new content"); - var lockProcess = StartFileLockProcess(file); + var lockProcess = FileLockUtils.StartFileLockProcess(file); try { - await Assert.That(IsFileLocked(file)).IsTrue(); + await Assert.That(FileLockUtils.IsFileLocked(file)).IsTrue(); await Assert.That(FileEx.SafeMove(tempFile, file)).IsFalse(); FileLockKiller.KillLockingProcesses(file); @@ -77,53 +111,9 @@ public async Task MoveSucceedsAfterKillingLockingProcess() } finally { - if (!lockProcess.HasExited) - { - lockProcess.Kill(); - } - - lockProcess.Dispose(); + FileLockUtils.Cleanup(lockProcess); File.Delete(file); File.Delete(tempFile); } } - - static Process StartFileLockProcess(string path) - { - var script = $"$f = [System.IO.File]::Open('{path.Replace("'", "''")}', 'Open', 'ReadWrite', 'None'); [Console]::WriteLine('locked'); Start-Sleep -Seconds 60"; - var process = new Process - { - StartInfo = new() - { - FileName = "powershell.exe", - Arguments = $"-NoProfile -Command \"{script}\"", - UseShellExecute = false, - CreateNoWindow = true, - RedirectStandardOutput = true - } - }; - process.Start(); - - // Wait for the process to signal that it has acquired the lock - var line = process.StandardOutput.ReadLine(); - if (line != "locked") - { - throw new InvalidOperationException($"Expected 'locked' but got '{line}'"); - } - - return process; - } - - static bool IsFileLocked(string path) - { - try - { - using var stream = File.Open(path, FileMode.Open, FileAccess.ReadWrite, FileShare.None); - return false; - } - catch (IOException) - { - return true; - } - } -} +} \ No newline at end of file diff --git a/src/DiffEngineTray.Tests/FileLockUtils.cs b/src/DiffEngineTray.Tests/FileLockUtils.cs new file mode 100644 index 00000000..1f396c06 --- /dev/null +++ b/src/DiffEngineTray.Tests/FileLockUtils.cs @@ -0,0 +1,52 @@ +static class FileLockUtils +{ + public static Process StartFileLockProcess(string path) + { + var script = $"$f = [System.IO.File]::Open('{path.Replace("'", "''")}', 'Open', 'ReadWrite', 'None'); [Console]::WriteLine('locked'); Start-Sleep -Seconds 60"; + var process = new Process + { + StartInfo = new() + { + FileName = "powershell.exe", + Arguments = $"-NoProfile -Command \"{script}\"", + UseShellExecute = false, + CreateNoWindow = true, + RedirectStandardOutput = true + } + }; + process.Start(); + + // Wait for the process to signal that it has acquired the lock + var line = process.StandardOutput.ReadLine(); + if (line != "locked") + { + throw new InvalidOperationException($"Expected 'locked' but got '{line}'"); + } + + return process; + } + + public static bool IsFileLocked(string path) + { + try + { + using var stream = File.Open(path, FileMode.Open, FileAccess.ReadWrite, FileShare.None); + return false; + } + catch (IOException) + { + return true; + } + } + + public static void Cleanup(Process process) + { + if (!process.HasExited) + { + process.Kill(); + process.WaitForExit(5000); + } + + process.Dispose(); + } +} \ No newline at end of file diff --git a/src/DiffEngineTray.Tests/LockedFilesFormTests.Default.verified.png b/src/DiffEngineTray.Tests/LockedFilesFormTests.Default.verified.png new file mode 100644 index 00000000..15aaa41c Binary files /dev/null and b/src/DiffEngineTray.Tests/LockedFilesFormTests.Default.verified.png differ diff --git a/src/DiffEngineTray.Tests/LockedFilesFormTests.cs b/src/DiffEngineTray.Tests/LockedFilesFormTests.cs new file mode 100644 index 00000000..e3a86878 --- /dev/null +++ b/src/DiffEngineTray.Tests/LockedFilesFormTests.cs @@ -0,0 +1,38 @@ +#if DEBUG +public class LockedFilesFormTests +{ + //[Test] + //[Explicit] + //public void Launch() + //{ + // using var form = new LockedFilesForm(BuildMove(), BuildLocked()); + // form.ShowDialog(); + //} + + [Test] + public async Task Default() + { + using var form = new LockedFilesForm(BuildMove(), BuildLocked()); + await Verify(form); + } + + static TrackedMove BuildMove() => + new( + @"C:\tests\AdviceSummaryTests.BuildWord.received.docx", + @"C:\tests\AdviceSummaryTests.BuildWord.verified.docx", + null, + null, + false, + null, + null, + "docx"); + + static LockedFiles BuildLocked() => + new( + [ + @"C:\tests\AdviceSummaryTests.BuildWord.received.docx", + @"C:\tests\AdviceSummaryTests.BuildWord.verified.docx" + ], + [new(1234, "Microsoft Word")]); +} +#endif \ No newline at end of file diff --git a/src/DiffEngineTray.Tests/OptionsFormTests.Default.verified.png b/src/DiffEngineTray.Tests/OptionsFormTests.Default.verified.png index 0344c533..fcc67c35 100644 Binary files a/src/DiffEngineTray.Tests/OptionsFormTests.Default.verified.png and b/src/DiffEngineTray.Tests/OptionsFormTests.Default.verified.png differ diff --git a/src/DiffEngineTray.Tests/OptionsFormTests.WithKeys.verified.png b/src/DiffEngineTray.Tests/OptionsFormTests.WithKeys.verified.png index f359fcd3..605f5968 100644 Binary files a/src/DiffEngineTray.Tests/OptionsFormTests.WithKeys.verified.png and b/src/DiffEngineTray.Tests/OptionsFormTests.WithKeys.verified.png differ diff --git a/src/DiffEngineTray.Tests/RecordingTracker.cs b/src/DiffEngineTray.Tests/RecordingTracker.cs index 64155ec7..3fac9d23 100644 --- a/src/DiffEngineTray.Tests/RecordingTracker.cs +++ b/src/DiffEngineTray.Tests/RecordingTracker.cs @@ -1,11 +1,12 @@ -class RecordingTracker() : +class RecordingTracker(LockedFilesResolver? lockedFilesResolver = null) : Tracker( () => { }, () => { - }) + }, + lockedFilesResolver) { public async Task AssertEmpty() { @@ -13,4 +14,4 @@ public async Task AssertEmpty() await Assert.That(Moves).IsEmpty(); await Assert.That(TrackingAny).IsFalse(); } -} +} \ No newline at end of file diff --git a/src/DiffEngineTray.Tests/SettingsHelperTests.ReadWrite.verified.txt b/src/DiffEngineTray.Tests/SettingsHelperTests.ReadWrite.verified.txt index 83d0fa94..02a3966a 100644 --- a/src/DiffEngineTray.Tests/SettingsHelperTests.ReadWrite.verified.txt +++ b/src/DiffEngineTray.Tests/SettingsHelperTests.ReadWrite.verified.txt @@ -6,6 +6,7 @@ Key: T }, RunAtStartup: false, + AlwaysKillLockingProcesses: true, TargetOnLeft: false, MaxInstancesToLaunch: 5 } \ No newline at end of file diff --git a/src/DiffEngineTray.Tests/SettingsHelperTests.cs b/src/DiffEngineTray.Tests/SettingsHelperTests.cs index 69c839c9..0d1ac165 100644 --- a/src/DiffEngineTray.Tests/SettingsHelperTests.cs +++ b/src/DiffEngineTray.Tests/SettingsHelperTests.cs @@ -20,7 +20,8 @@ await SettingsHelper.Write( Key = "T" }, MaxInstancesToLaunch = 5, - TargetOnLeft = false + TargetOnLeft = false, + AlwaysKillLockingProcesses = true }); var result = await SettingsHelper.Read(); diff --git a/src/DiffEngineTray.Tests/TrackerLockedMoveTest.cs b/src/DiffEngineTray.Tests/TrackerLockedMoveTest.cs new file mode 100644 index 00000000..911e5719 --- /dev/null +++ b/src/DiffEngineTray.Tests/TrackerLockedMoveTest.cs @@ -0,0 +1,124 @@ +public class TrackerLockedMoveTest : + IDisposable +{ + [Test] + public async Task Ignore_KeepsMovePending() + { + LockedFiles? observed = null; + await using var tracker = new RecordingTracker( + (_, locked) => + { + observed = locked; + return LockedFilesResponse.Ignore; + }); + var lockProcess = FileLockUtils.StartFileLockProcess(target1); + try + { + var tracked = tracker.AddMove(temp1, target1, "theExe", "theArguments", false, null); + tracker.Accept(tracked); + + await Assert.That(tracker.Moves).HasSingleItem(); + await Assert.That(observed).IsNotNull(); + await Assert.That(observed!.Files).Contains(target1); + await Assert.That(observed.Processes.Select(_ => _.ProcessId)).Contains(lockProcess.Id); + } + finally + { + FileLockUtils.Cleanup(lockProcess); + } + + await Assert.That(File.ReadAllText(target1)).IsEqualTo("old"); + } + + [Test] + public async Task Kill_AcceptsMove() + { + await using var tracker = new RecordingTracker( + (_, _) => LockedFilesResponse.Kill); + var lockProcess = FileLockUtils.StartFileLockProcess(target1); + try + { + var tracked = tracker.AddMove(temp1, target1, "theExe", "theArguments", false, null); + tracker.Accept(tracked); + + await tracker.AssertEmpty(); + await Assert.That(File.ReadAllText(target1)).IsEqualTo("new"); + await Assert.That(lockProcess.WaitForExit(5000)).IsTrue(); + } + finally + { + FileLockUtils.Cleanup(lockProcess); + } + } + + [Test] + public async Task KillAndAcceptAllPending_AcceptsOtherMoves() + { + var resolveCount = 0; + await using var tracker = new RecordingTracker( + (_, _) => + { + resolveCount++; + return LockedFilesResponse.KillAndAcceptAllPending; + }); + var lockProcess1 = FileLockUtils.StartFileLockProcess(target1); + var lockProcess2 = FileLockUtils.StartFileLockProcess(target2); + try + { + var tracked = tracker.AddMove(temp1, target1, "theExe", "theArguments", false, null); + tracker.AddMove(temp2, target2, "theExe", "theArguments", false, null); + + tracker.Accept(tracked); + + await tracker.AssertEmpty(); + await Assert.That(resolveCount).IsEqualTo(1); + await Assert.That(File.ReadAllText(target1)).IsEqualTo("new"); + await Assert.That(File.ReadAllText(target2)).IsEqualTo("new"); + } + finally + { + FileLockUtils.Cleanup(lockProcess1); + FileLockUtils.Cleanup(lockProcess2); + } + } + + [Test] + public async Task NoResolver_KeepsMovePending() + { + await using var tracker = new RecordingTracker(); + var lockProcess = FileLockUtils.StartFileLockProcess(target1); + try + { + var tracked = tracker.AddMove(temp1, target1, "theExe", "theArguments", false, null); + tracker.Accept(tracked); + + await Assert.That(tracker.Moves).HasSingleItem(); + } + finally + { + FileLockUtils.Cleanup(lockProcess); + } + + await Assert.That(File.ReadAllText(target1)).IsEqualTo("old"); + } + + static string CreateFile(string content) + { + var path = Path.Combine(Path.GetTempPath(), $"TrackerLockedMoveTest_{Guid.NewGuid()}.txt"); + File.WriteAllText(path, content); + return path; + } + + public void Dispose() + { + File.Delete(temp1); + File.Delete(temp2); + File.Delete(target1); + File.Delete(target2); + } + + string temp1 = CreateFile("new"); + string temp2 = CreateFile("new"); + string target1 = CreateFile("old"); + string target2 = CreateFile("old"); +} \ No newline at end of file diff --git a/src/DiffEngineTray/FileLockKiller.cs b/src/DiffEngineTray/FileLockKiller.cs index fdf3de6e..09e5cac6 100644 --- a/src/DiffEngineTray/FileLockKiller.cs +++ b/src/DiffEngineTray/FileLockKiller.cs @@ -51,13 +51,13 @@ struct RM_PROCESS_INFO public bool bRestartable; } - public static bool KillLockingProcesses(string filePath) + public static List GetLockingProcesses(string filePath) { - var killed = false; + var processes = new List(); if (RmStartSession(out var sessionHandle, 0, Guid.NewGuid().ToString()) != 0) { - return false; + return processes; } try @@ -65,7 +65,7 @@ public static bool KillLockingProcesses(string filePath) var resources = new[] { filePath }; if (RmRegisterResources(sessionHandle, (uint)resources.Length, resources, 0, [], 0, []) != 0) { - return false; + return processes; } var procInfo = 0u; @@ -74,7 +74,7 @@ public static bool KillLockingProcesses(string filePath) if (result != errorMoreData || procInfoNeeded == 0) { - return false; + return processes; } var processInfo = new RM_PROCESS_INFO[procInfoNeeded]; @@ -83,29 +83,25 @@ public static bool KillLockingProcesses(string filePath) if (result != 0) { - return false; + return processes; } + var currentProcessId = Environment.ProcessId; for (var i = 0; i < procInfo; i++) { - var processId = (int)processInfo[i].Process.dwProcessId; - if (!ProcessEx.TryGet(processId, out var process)) + var info = processInfo[i]; + var processId = (int)info.Process.dwProcessId; + if (processId == currentProcessId) { continue; } - Log.Information( - "Killing locking process '{ProcessName}' (PID: {ProcessId}) for file '{FilePath}'", - processInfo[i].strAppName, - processId, - filePath); - process.KillAndDispose(); - killed = true; + processes.Add(new(processId, info.strAppName)); } } catch (Exception exception) { - ExceptionHandler.Handle($"Failed to kill locking processes for '{filePath}'.", exception); + ExceptionHandler.Handle($"Failed to get locking processes for '{filePath}'.", exception); } finally { @@ -113,6 +109,31 @@ public static bool KillLockingProcesses(string filePath) _ = RmEndSession(sessionHandle); } + return processes; + } + + public static bool Kill(IEnumerable processes) + { + var killed = false; + + foreach (var locking in processes) + { + if (!ProcessEx.TryGet(locking.ProcessId, out var process)) + { + continue; + } + + Log.Information( + "Killing locking process '{ProcessName}' (PID: {ProcessId})", + locking.Name, + locking.ProcessId); + process.KillAndDispose(); + killed = true; + } + return killed; } -} + + public static bool KillLockingProcesses(string filePath) => + Kill(GetLockingProcesses(filePath)); +} \ No newline at end of file diff --git a/src/DiffEngineTray/LockedFiles.cs b/src/DiffEngineTray/LockedFiles.cs new file mode 100644 index 00000000..faacbd6e --- /dev/null +++ b/src/DiffEngineTray/LockedFiles.cs @@ -0,0 +1,9 @@ +record LockedFiles(List Files, List Processes) +{ + public string ProcessNames => + string.Join( + ", ", + Processes + .Select(_ => _.Name) + .Distinct()); +} \ No newline at end of file diff --git a/src/DiffEngineTray/LockedFilesForm.cs b/src/DiffEngineTray/LockedFilesForm.cs new file mode 100644 index 00000000..97185dec --- /dev/null +++ b/src/DiffEngineTray/LockedFilesForm.cs @@ -0,0 +1,116 @@ +class LockedFilesForm : + Form +{ + public LockedFilesResponse Response { get; private set; } = LockedFilesResponse.Ignore; + public bool AlwaysKill { get; private set; } + + public LockedFilesForm(TrackedMove move, LockedFiles locked) + { + var processNames = locked.ProcessNames; + + Text = $"Locked files: {move.Name}"; + Icon = Images.Active; + AutoSize = true; + FormBorderStyle = FormBorderStyle.FixedDialog; + MaximizeBox = false; + MinimizeBox = false; + ShowInTaskbar = false; + StartPosition = FormStartPosition.CenterScreen; + TopMost = true; + Padding = new(8); + // GrowAndShrink so the form hugs its content height; MinimumSize keeps + // the width stable. GrowOnly (the AutoSize default) plus a fixed + // ClientSize left extra padding below the content. + AutoSizeMode = AutoSizeMode.GrowAndShrink; + MinimumSize = new(576, 0); + + var buttons = new FlowLayoutPanel + { + FlowDirection = FlowDirection.LeftToRight, + AutoSize = true, + Dock = DockStyle.Top, + Padding = new(3) + }; + + Button AddButton(string text, Action clicked) + { + var button = new Button + { + Text = text, + AutoSize = true, + UseVisualStyleBackColor = true + }; + button.Click += (_, _) => + { + clicked(); + DialogResult = DialogResult.OK; + }; + buttons.Controls.Add(button); + return button; + } + + var ignore = AddButton( + "Ignore", + () => Response = LockedFilesResponse.Ignore); + AddButton( + $"Kill {processNames} and accept", + () => Response = LockedFilesResponse.Kill); + AddButton( + "Kill and accept all pending", + () => Response = LockedFilesResponse.KillAndAcceptAllPending); + AddButton( + "Always kill", + () => + { + AlwaysKill = true; + Response = LockedFilesResponse.Kill; + }); + + // One Label per file rather than a single multi-line Label: multi-line + // Labels round each line's leading whitespace independently, so bullets + // that share an identical prefix can end up misaligned. + var filesPanel = new FlowLayoutPanel + { + FlowDirection = FlowDirection.TopDown, + AutoSize = true, + WrapContents = false, + Dock = DockStyle.Top, + Padding = new(12, 0, 3, 3) + }; + foreach (var file in locked.Files) + { + filesPanel.Controls.Add( + new Label + { + Text = $"• {Path.GetFileName(file)}", + AutoSize = true, + // GDI rendering measures whitespace consistently; the GDI+ + // default does not, which is what skews the bullet spacing. + UseCompatibleTextRendering = false + }); + } + + // Dock = Top stacks in reverse add order, so add bottom-most first + Controls.Add( + new Label + { + Text = "\"Always kill\" is stored in settings and can be changed in Options.", + AutoSize = true, + Dock = DockStyle.Top, + Padding = new(3), + ForeColor = SystemColors.GrayText + }); + Controls.Add(buttons); + Controls.Add(filesPanel); + Controls.Add( + new Label + { + Text = $"The following files are locked by {processNames}:", + AutoSize = true, + Dock = DockStyle.Top, + Padding = new(3) + }); + + CancelButton = ignore; + } +} \ No newline at end of file diff --git a/src/DiffEngineTray/LockedFilesHandler.cs b/src/DiffEngineTray/LockedFilesHandler.cs new file mode 100644 index 00000000..4d40d995 --- /dev/null +++ b/src/DiffEngineTray/LockedFilesHandler.cs @@ -0,0 +1,38 @@ +static class LockedFilesHandler +{ + public static bool AlwaysKill { get; set; } + + public static LockedFilesResponse Resolve(TrackedMove move, LockedFiles locked) + { + if (AlwaysKill) + { + return LockedFilesResponse.Kill; + } + + using var form = new LockedFilesForm(move, locked); + form.ShowDialog(); + + if (form.AlwaysKill) + { + AlwaysKill = true; + Persist(); + } + + return form.Response; + } + + static void Persist() => + Task.Run(async () => + { + try + { + var settings = await SettingsHelper.Read(); + settings.AlwaysKillLockingProcesses = true; + await SettingsHelper.Write(settings); + } + catch (Exception exception) + { + ExceptionHandler.Handle("Failed to persist AlwaysKillLockingProcesses setting", exception); + } + }); +} \ No newline at end of file diff --git a/src/DiffEngineTray/LockedFilesResolver.cs b/src/DiffEngineTray/LockedFilesResolver.cs new file mode 100644 index 00000000..56e8bbd5 --- /dev/null +++ b/src/DiffEngineTray/LockedFilesResolver.cs @@ -0,0 +1 @@ +delegate LockedFilesResponse LockedFilesResolver(TrackedMove move, LockedFiles locked); \ No newline at end of file diff --git a/src/DiffEngineTray/LockedFilesResponse.cs b/src/DiffEngineTray/LockedFilesResponse.cs new file mode 100644 index 00000000..b3a0ee83 --- /dev/null +++ b/src/DiffEngineTray/LockedFilesResponse.cs @@ -0,0 +1,6 @@ +enum LockedFilesResponse +{ + Ignore, + Kill, + KillAndAcceptAllPending +} \ No newline at end of file diff --git a/src/DiffEngineTray/LockingProcess.cs b/src/DiffEngineTray/LockingProcess.cs new file mode 100644 index 00000000..ee7a28e1 --- /dev/null +++ b/src/DiffEngineTray/LockingProcess.cs @@ -0,0 +1 @@ +record LockingProcess(int ProcessId, string Name); \ No newline at end of file diff --git a/src/DiffEngineTray/Program.cs b/src/DiffEngineTray/Program.cs index 45f676bb..c3aac851 100644 --- a/src/DiffEngineTray/Program.cs +++ b/src/DiffEngineTray/Program.cs @@ -28,6 +28,8 @@ static async Task Inner() return; } + LockedFilesHandler.AlwaysKill = settings.AlwaysKillLockingProcesses; + Application.EnableVisualStyles(); Application.SetCompatibleTextRenderingDefault(false); var tokenSource = new CancelSource(); @@ -48,7 +50,8 @@ static async Task Inner() await using var tracker = new Tracker( active: () => icon.Icon = Images.Active, - inactive: () => icon.Icon = Images.Default); + inactive: () => icon.Icon = Images.Default, + lockedFilesResolver: LockedFilesHandler.Resolve); using var task = StartServer(tracker, cancel); diff --git a/src/DiffEngineTray/Settings/OptionsForm.Designer.cs b/src/DiffEngineTray/Settings/OptionsForm.Designer.cs index 8343bd6d..f84b425e 100644 --- a/src/DiffEngineTray/Settings/OptionsForm.Designer.cs +++ b/src/DiffEngineTray/Settings/OptionsForm.Designer.cs @@ -28,6 +28,7 @@ void InitializeComponent() this.flowLayoutPanel1 = new System.Windows.Forms.FlowLayoutPanel(); this.updateButton = new System.Windows.Forms.Button(); this.targetOnLeftCheckBox = new System.Windows.Forms.CheckBox(); + this.alwaysKillCheckBox = new System.Windows.Forms.CheckBox(); this.maxInstancesGroupBox = new System.Windows.Forms.GroupBox(); this.maxInstancesNumericUpDown = new System.Windows.Forms.NumericUpDown(); this.label1 = new System.Windows.Forms.Label(); @@ -224,6 +225,20 @@ void InitializeComponent() this.targetOnLeftCheckBox.Text = "Open target on left. The default is temp on left and target on right"; this.targetOnLeftCheckBox.UseVisualStyleBackColor = true; // + // alwaysKillCheckBox + // + this.alwaysKillCheckBox.AutoSize = true; + this.alwaysKillCheckBox.Dock = System.Windows.Forms.DockStyle.Top; + this.alwaysKillCheckBox.Location = new System.Drawing.Point(5, 58); + this.alwaysKillCheckBox.Margin = new System.Windows.Forms.Padding(2); + this.alwaysKillCheckBox.Name = "alwaysKillCheckBox"; + this.alwaysKillCheckBox.Padding = new System.Windows.Forms.Padding(5, 4, 5, 4); + this.alwaysKillCheckBox.Size = new System.Drawing.Size(520, 27); + this.alwaysKillCheckBox.TabIndex = 10; + this.alwaysKillCheckBox.Text = "Always kill processes locking files when accepting. When disabled, a prompt is s" + + "hown"; + this.alwaysKillCheckBox.UseVisualStyleBackColor = true; + // // maxInstancesGroupBox // this.maxInstancesGroupBox.Controls.Add(this.maxInstancesNumericUpDown); @@ -287,6 +302,7 @@ void InitializeComponent() this.Controls.Add(this.acceptAllHotKey); this.Controls.Add(this.discardAllHotKey); this.Controls.Add(this.maxInstancesGroupBox); + this.Controls.Add(this.alwaysKillCheckBox); this.Controls.Add(this.targetOnLeftCheckBox); this.Controls.Add(this.startupCheckBox); this.FormBorderStyle = System.Windows.Forms.FormBorderStyle.FixedDialog; @@ -325,6 +341,7 @@ void InitializeComponent() private System.Windows.Forms.FlowLayoutPanel flowLayoutPanel1; private System.Windows.Forms.Button updateButton; private System.Windows.Forms.CheckBox targetOnLeftCheckBox; + private System.Windows.Forms.CheckBox alwaysKillCheckBox; private System.Windows.Forms.GroupBox maxInstancesGroupBox; private System.Windows.Forms.NumericUpDown maxInstancesNumericUpDown; private System.Windows.Forms.Label label1; diff --git a/src/DiffEngineTray/Settings/OptionsForm.cs b/src/DiffEngineTray/Settings/OptionsForm.cs index 7f1d10c4..4b99adcc 100644 --- a/src/DiffEngineTray/Settings/OptionsForm.cs +++ b/src/DiffEngineTray/Settings/OptionsForm.cs @@ -19,6 +19,7 @@ public OptionsForm(Settings settings, Func> Save(KeyRegister keyRegister, Tra } await SettingsHelper.Write(settings); + LockedFilesHandler.AlwaysKill = settings.AlwaysKillLockingProcesses; return []; } diff --git a/src/DiffEngineTray/Settings/Settings.cs b/src/DiffEngineTray/Settings/Settings.cs index c441f457..840de049 100644 --- a/src/DiffEngineTray/Settings/Settings.cs +++ b/src/DiffEngineTray/Settings/Settings.cs @@ -4,6 +4,7 @@ public class Settings public HotKey? DiscardAllHotKey { get; set; } public HotKey? AcceptOpenHotKey { get; set; } public bool RunAtStartup { get; set; } + public bool AlwaysKillLockingProcesses { get; set; } [JsonIgnore] public bool TargetOnLeft { get; set; } = TargetPosition.TargetOnLeft; [JsonIgnore] diff --git a/src/DiffEngineTray/Tracker.cs b/src/DiffEngineTray/Tracker.cs index 28c139e3..a12584b6 100644 --- a/src/DiffEngineTray/Tracker.cs +++ b/src/DiffEngineTray/Tracker.cs @@ -3,15 +3,17 @@ class Tracker : { Action active; Action inactive; + LockedFilesResolver? lockedFilesResolver; ConcurrentDictionary moves = new(StringComparer.OrdinalIgnoreCase); ConcurrentDictionary deletes = new(StringComparer.OrdinalIgnoreCase); AsyncTimer timer; int lastScanCount; - public Tracker(Action active, Action inactive) + public Tracker(Action active, Action inactive, LockedFilesResolver? lockedFilesResolver = null) { this.active = active; this.inactive = inactive; + this.lockedFilesResolver = lockedFilesResolver; timer = new( ScanFiles, TimeSpan.FromSeconds(2), @@ -207,7 +209,7 @@ public TrackedDelete AddDelete(string file) => public void Accept(TrackedDelete delete) { - if (deletes.Remove(delete.File, out var removed)) + if (deletes.TryRemove(delete.File, out var removed)) { File.Delete(removed.File); } @@ -217,60 +219,167 @@ public void Accept(IEnumerable toAccept) { foreach (var delete in toAccept) { - if (deletes.Remove(delete.File, out var removed)) + if (deletes.TryRemove(delete.File, out var removed)) { File.Delete(removed.File); } } } - public void Accept(IEnumerable toAccept) + public void Accept(IEnumerable toAccept) => + AcceptMoves(toAccept); + + public void Accept(TrackedMove move) => + AcceptMoves([move]); + + class AcceptBatch + { + public bool KillWithoutPrompt; + public bool AcceptAllPending; + } + + void AcceptMoves(IEnumerable toAccept) { + var batch = new AcceptBatch(); foreach (var move in toAccept) { - if (moves.Remove(move.Temp, out var removed)) + AcceptMove(move, batch); + } + + if (batch.AcceptAllPending) + { + foreach (var move in moves.Values) { - InnerMove(removed); + AcceptMove(move, batch); } } } - public void Accept(TrackedMove move) + void AcceptMove(TrackedMove move, AcceptBatch batch) { - if (moves.Remove(move.Temp, out var removed)) + if (!moves.TryRemove(move.Temp, out var removed)) { - InnerMove(removed); + return; + } + + if (!InnerMove(removed, batch)) + { + // Keep the move pending so accepting can be retried + moves.TryAdd(removed.Temp, removed); } } public void Discard(TrackedMove move) { - if (moves.Remove(move.Temp, out var removed)) + if (moves.TryRemove(move.Temp, out var removed)) { InnerDiscard(removed); } } - static void InnerMove(TrackedMove move) + // Returns false when the move should be kept pending + bool InnerMove(TrackedMove move, AcceptBatch batch) { KillProcesses(move); - if (!FileEx.SafeMove(move.Temp, move.Target)) + if (FileEx.SafeMove(move.Temp, move.Target)) + { + DeleteTempDirectory(move); + return true; + } + + var locked = FindLockedFiles(move); + if (locked == null) + { + // Not caused by a file lock. Drop the move since it is likely a + // running test is reading or writing to the files, and the result + // will re-add the tracked item + return true; + } + + Log.Information( + "Files for `{Name}` are locked by {Processes}", + move.Name, + locked.ProcessNames); + + if (!ShouldKill(move, locked, batch)) { - if (move.KillLockingProcess && - FileLockKiller.KillLockingProcesses(move.Target)) + return false; + } + + FileLockKiller.Kill(locked.Processes); + + if (FileEx.SafeMove(move.Temp, move.Target)) + { + DeleteTempDirectory(move); + return true; + } + + return false; + } + + bool ShouldKill(TrackedMove move, LockedFiles locked, AcceptBatch batch) + { + if (move.KillLockingProcess || + batch.KillWithoutPrompt) + { + return true; + } + + if (lockedFilesResolver == null) + { + return false; + } + + switch (lockedFilesResolver(move, locked)) + { + case LockedFilesResponse.Kill: + return true; + case LockedFilesResponse.KillAndAcceptAllPending: + batch.KillWithoutPrompt = true; + batch.AcceptAllPending = true; + return true; + default: + return false; + } + } + + static LockedFiles? FindLockedFiles(TrackedMove move) + { + var files = new List(); + var processes = new List(); + + void AddLockers(string file) + { + var lockers = FileLockKiller.GetLockingProcesses(file); + if (lockers.Count == 0) { - if (!FileEx.SafeMove(move.Temp, move.Target)) - { - return; - } + return; } - else + + files.Add(file); + foreach (var locker in lockers) { - return; + if (processes.TrueForAll(_ => _.ProcessId != locker.ProcessId)) + { + processes.Add(locker); + } } } + AddLockers(move.Temp); + AddLockers(move.Target); + + if (files.Count == 0) + { + return null; + } + + return new(files, processes); + } + + static void DeleteTempDirectory(TrackedMove move) + { var directory = Path.GetDirectoryName(move.Temp)!; FileEx.SafeDeleteDirectory(directory); } @@ -321,28 +430,17 @@ public void AcceptOpen() { AcceptAllDeletes(); - foreach (var move in moves.Values) - { - if (move.Process == null) - { - continue; - } - - if (move.Process.HasExited) - { - continue; - } - - InnerMove(move); - moves.Remove(move.Temp, out _); - } + AcceptMoves( + moves.Values + .Where(_ => _.Process is { HasExited: false }) + .ToList()); } public void AcceptAll() { AcceptAllDeletes(); - AcceptAllMoves(); + AcceptMoves(moves.Values); } void AcceptAllDeletes() @@ -355,16 +453,6 @@ void AcceptAllDeletes() deletes.Clear(); } - void AcceptAllMoves() - { - foreach (var move in moves.Values) - { - InnerMove(move); - } - - moves.Clear(); - } - public ICollection Deletes => deletes.Values; public ICollection Moves => moves.Values;