From 22dc5d3edbe71147f8ddcde1c48b7c2db2090913 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 21 Mar 2026 09:09:05 +1100 Subject: [PATCH] Snapshot existing Word PIDs before creating the COM instance --- src/MsWordDiff.Tests/ProcessCleanupTests.cs | 105 +++++++++++++++ src/MsWordDiff/Word.cs | 138 ++++++++++++++++---- 2 files changed, 221 insertions(+), 22 deletions(-) create mode 100644 src/MsWordDiff.Tests/ProcessCleanupTests.cs diff --git a/src/MsWordDiff.Tests/ProcessCleanupTests.cs b/src/MsWordDiff.Tests/ProcessCleanupTests.cs new file mode 100644 index 0000000..9af1e73 --- /dev/null +++ b/src/MsWordDiff.Tests/ProcessCleanupTests.cs @@ -0,0 +1,105 @@ +public class ProcessCleanupTests +{ + [Test] + public async Task GetWordProcessIds_ReturnsHashSet() + { + var pids = Word.GetWordProcessIds(); + await Assert.That(pids).IsNotNull(); + } + + [Test] + public async Task FindNewWordProcess_WhenNoNewProcesses_ReturnsNull() + { + var existingPids = Word.GetWordProcessIds(); + var result = Word.FindNewWordProcess(existingPids); + await Assert.That(result).IsNull(); + } + + [Test] + public void QuitAndKill_WithNullProcess_DoesNotThrow() => + Word.QuitAndKill((dynamic)new object(), null); + + [Test] + public void QuitAndKill_WithExitedProcess_DoesNotThrow() + { + var process = Process.Start(new ProcessStartInfo + { + FileName = "cmd.exe", + Arguments = "/c exit 0", + CreateNoWindow = true, + UseShellExecute = false + })!; + process.WaitForExit(); + + Word.QuitAndKill((dynamic)new object(), process); + process.Dispose(); + } + + [Test] + public async Task QuitAndKill_WithRunningProcess_KillsProcess() + { + var process = Process.Start(new ProcessStartInfo + { + FileName = "ping", + Arguments = "-n 60 127.0.0.1", + CreateNoWindow = true, + UseShellExecute = false + })!; + + await Assert.That(process.HasExited).IsFalse(); + + Word.QuitAndKill((dynamic)new object(), process); + + process.WaitForExit(5000); + await Assert.That(process.HasExited).IsTrue() + .Because("QuitAndKill should kill running processes"); + process.Dispose(); + } + + [Test] + [Explicit] + public async Task Launch_WithInvalidPath_DoesNotLeaveZombieProcess() + { + var wordType = Type.GetTypeFromProgID("Word.Application"); + if (wordType == null) + { + Skip.Test("Microsoft Word is not installed"); + } + + var beforePids = Word.GetWordProcessIds(); + + try + { + await Word.Launch( + @"C:\nonexistent\file1.docx", + @"C:\nonexistent\file2.docx"); + } + catch + { + // Expected - invalid file paths + } + + // Give Word a moment to fully shut down + await Task.Delay(3000); + + var afterPids = Word.GetWordProcessIds(); + afterPids.ExceptWith(beforePids); + + // Clean up any zombie processes (safety net) + foreach (var pid in afterPids) + { + try + { + using var p = Process.GetProcessById(pid); + p.Kill(); + } + catch + { + // Process may have already exited + } + } + + await Assert.That(afterPids.Count).IsEqualTo(0) + .Because("No zombie Word processes should remain after a failed Launch"); + } +} diff --git a/src/MsWordDiff/Word.cs b/src/MsWordDiff/Word.cs index 662e116..c47d07a 100644 --- a/src/MsWordDiff/Word.cs +++ b/src/MsWordDiff/Word.cs @@ -10,42 +10,70 @@ public static async Task Launch(string path1, string path2, bool quiet = false) var job = JobObject.Create(); + // Snapshot existing Word PIDs before creating the COM instance so we can + // identify the new WINWORD.EXE process immediately after creation and assign + // it to the Job Object before any document operations that could throw. + // Previously, assignment happened after opening the first document, leaving + // a window where exceptions would orphan the Word process. + var existingPids = GetWordProcessIds(); dynamic word = Activator.CreateInstance(wordType)!; + var process = FindNewWordProcess(existingPids); + if (process != null) + { + JobObject.AssignProcess(job, process.Handle); + } - // WdAlertLevel.wdAlertsNone = 0 - word.DisplayAlerts = 0; + try + { + // WdAlertLevel.wdAlertsNone = 0 + word.DisplayAlerts = 0; - // Disable AutoRecover to prevent "serious error" recovery dialogs - word.Options.SaveInterval = 0; + // Disable AutoRecover to prevent "serious error" recovery dialogs + word.Options.SaveInterval = 0; - var doc1 = Open(word, path1); + var doc1 = Open(word, path1); - // Get process from Word's window handle and assign to job - var hwnd = (IntPtr) word.ActiveWindow.Hwnd; - GetWindowThreadProcessId(hwnd, out var processId); - using var process = Process.GetProcessById(processId); - JobObject.AssignProcess(job, process.Handle); + // Fallback: if process snapshot didn't find the new process, get it via window handle + if (process == null) + { + var hwnd = (IntPtr)word.ActiveWindow.Hwnd; + GetWindowThreadProcessId(hwnd, out var processId); + process = Process.GetProcessById(processId); + JobObject.AssignProcess(job, process.Handle); + } - var doc2 = Open(word, path2); + var doc2 = Open(word, path2); - var compare = LaunchCompare(word, doc1, doc2); + var compare = LaunchCompare(word, doc1, doc2); - word.Visible = true; + word.Visible = true; - ApplyQuiet(quiet, word); + ApplyQuiet(quiet, word); - HideNavigationPane(word); + HideNavigationPane(word); - MinimizeRibbon(word); + MinimizeRibbon(word); - // Bring Word to the foreground - SetForegroundWindow(hwnd); + // Bring Word to the foreground + SetForegroundWindow((IntPtr)word.ActiveWindow.Hwnd); - await process.WaitForExitAsync(); + await process.WaitForExitAsync(); - Marshal.ReleaseComObject(compare); - Marshal.ReleaseComObject(word); - JobObject.Close(job); + Marshal.ReleaseComObject(compare); + } + catch + { + // If setup fails (e.g. invalid file path), gracefully quit Word + // then force-kill as a fallback to prevent zombie processes. + QuitAndKill(word, process); + throw; + } + finally + { + Marshal.ReleaseComObject(word); + process?.Dispose(); + JobObject.Close(job); + } RestoreRibbon(wordType); } @@ -126,9 +154,21 @@ static void MinimizeRibbon(dynamic word) } } + // RestoreRibbon creates a temporary Word instance solely to un-minimize the + // ribbon so the user's next normal Word session isn't affected. This instance + // is assigned to its own Job Object and has a kill fallback to prevent zombies + // (previously it had neither, making it the primary source of leaked processes). static void RestoreRibbon(Type wordType) { + var job = JobObject.Create(); + var existingPids = GetWordProcessIds(); dynamic word = Activator.CreateInstance(wordType)!; + var process = FindNewWordProcess(existingPids); + if (process != null) + { + JobObject.AssignProcess(job, process.Handle); + } + try { word.DisplayAlerts = 0; @@ -145,10 +185,64 @@ static void RestoreRibbon(Type wordType) word.Quit(); } + catch + { + QuitAndKill(word, process); + } finally { Marshal.ReleaseComObject(word); + process?.Dispose(); + JobObject.Close(job); + } + } + + // Attempts a graceful COM Quit, then force-kills the process as a fallback. + // All exceptions are swallowed because this runs in error/cleanup paths where + // COM may already be disconnected or the process may have exited. + internal static void QuitAndKill(dynamic word, Process? process) + { + try { word.Quit(SaveChanges: false); } + catch { /* COM may already be disconnected */ } + + if (process is { HasExited: false }) + { + try { process.Kill(); } + catch { /* Process may have exited between check and kill */ } + } + } + + // Snapshots current WINWORD PIDs. Used with FindNewWordProcess to identify + // the process created by Activator.CreateInstance without needing a window handle. + internal static HashSet GetWordProcessIds() + { + var pids = new HashSet(); + foreach (var p in Process.GetProcessesByName("WINWORD")) + { + pids.Add(p.Id); + p.Dispose(); + } + return pids; + } + + // Finds the WINWORD process that appeared after the snapshot was taken. + // If multiple new processes appear (rare race condition), keeps the last one found. + internal static Process? FindNewWordProcess(HashSet existingPids) + { + Process? found = null; + foreach (var p in Process.GetProcessesByName("WINWORD")) + { + if (!existingPids.Contains(p.Id)) + { + found?.Dispose(); + found = p; + } + else + { + p.Dispose(); + } } + return found; } [LibraryImport("user32.dll")]