Hold a handle on a tracked process

src/DiffEngineTray.Tests/ProcessExHandleTests.cs

@@ -0,0 +1,53 @@
+/// <summary>
+/// TryGet holds an OS handle on the process it hands back.
+/// <para>
+/// Process.GetProcessById holds none of its own, so Kill, HasExited and MainWindowHandle each
+/// re-open the id at the moment they are called. A tracked move outlives its diff tool by design -
+/// HandleScanMove keeps it while the temp file is still there - so hours later that id may belong
+/// to something else, and "Accept all" or "Open diff tool" would act on whatever it is.
+/// </para>
+/// <para>
+/// Asserted through the exit code, which is only readable if a handle was open before the process
+/// ended. That is the same thing as the id being held, since Windows will not hand out an id while
+/// a handle to it exists.
+/// </para>
+/// </summary>
+[NotInParallel]
+public class ProcessExHandleTests
+{
+    [Test]
+    public async Task TheProcessIsStillReadableAfterItExits()
+    {
+        // Any long lived process will do; this project has no fake tool of its own
+        var started = FileLockUtils.StartFileLockProcess(Path.GetTempFileName());
+
+        try
+        {
+            await Assert.That(ProcessEx.TryGet(started.Id, out var tracked)).IsTrue();
+
+            started.Kill();
+            await Assert.That(started.WaitForExit(30000)).IsTrue();
+
+            // Readable only because TryGet opened a handle while it was still running
+            await Assert.That(tracked!.HasExited).IsTrue();
+            await Assert.That(tracked.ExitCode).IsNotEqualTo(int.MinValue);
+            tracked.Dispose();
+        }
+        finally
+        {
+            try
+            {
+                if (!started.HasExited)
+                {
+                    started.Kill();
+                }
+            }
+            catch
+            {
+                // Already gone
+            }
+
+            started.Dispose();
+        }
+    }
+}

src/DiffEngineTray/ProcessEx.cs

@@ -16,9 +16,19 @@ public static bool TryGet(int id, [NotNullWhen(true)] out Process? process)
             }
         }
 
+        Process? opened = null;
         try
         {
-            process = Process.GetProcessById(id);
+            opened = Process.GetProcessById(id);
+            process = opened;
+            // Forces the OS handle open, and keeps it. GetProcessById holds none of its own, so
+            // Kill, HasExited and MainWindowHandle each re-open the id at the moment they are
+            // called - and a tracked move outlives its diff tool by design, since HandleScanMove
+            // keeps it while the temp file is still there. Hours later that id may belong to
+            // something else, and "Accept all" or "Open diff tool" would kill whatever it is.
+            // An open handle also stops Windows handing the id out again while this move is
+            // tracked, so there is nothing to confuse it with
+            _ = process.Handle;
             return true;
         }
         catch (ArgumentException)
@@ -27,6 +37,17 @@ public static bool TryGet(int id, [NotNullWhen(true)] out Process? process)
             process = null;
             return false;
         }
+        catch (Exception exception)
+            when (exception is Win32Exception or InvalidOperationException)
+        {
+            // The handle could not be held - it exited between the probe above and here, or this
+            // account cannot open it. Without one there is no way to tell the process apart from a
+            // later holder of the same id, so it is better tracked as no process at all: the tool
+            // is then not killed, rather than something else being killed in its place
+            opened?.Dispose();
+            process = null;
+            return false;
+        }
     }
 
     public static void KillAndDispose(this Process process)