Snapshot existing Word PIDs before creating the COM instance (#10)

Author: SimonCropp

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");
+    }
+}

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<int> GetWordProcessIds()
+    {
+        var pids = new HashSet<int>();
+        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<int> 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")]