ProcessWatchdog: no longer fail on exit code 259 on Windows

Our previous implementation of ProcessWatchdog was susceptible to a
well-known issue of GetProcessExitCode: if the process' real exit code
is STILL_ALIVE, then there's no way to determine that it has exited,
because this number is equal to a special value that function returns
for a living process.

This new implementation fixes that, and also introduces a new, better
API to pass more arguments (mostly for the purpose of testing).

rd-net/Lifetimes/Diagnostics/ProcessWatchdog.cs

@@ -15,8 +15,23 @@ namespace JetBrains.Diagnostics
   /// </summary>
   [PublicAPI] public static class ProcessWatchdog
   {
+    public class Options
+    {
+      public int Pid { get; }
+      public Lifetime Lifetime { get; }
+      public TimeSpan? GracefulShutdownPeriod { get; set; }
+      public Action? BeforeProcessKill { get; set; }
+      public Action? KillCurrentProcess { get; set; }
+
+      public Options(int pid, Lifetime lifetime)
+      {
+        Pid = pid;
+        Lifetime = lifetime;
+      }
+    }
+
     private static readonly ILog ourLogger = Log.GetLog(nameof(ProcessWatchdog));
-    private const int DELAY_BEFORE_RETRY = 1000;
+    internal const int DELAY_BEFORE_RETRY = 1000;
     private const int ERROR_INVALID_PARAMETER = 87;
 
     public static void StartWatchdogForPidEnvironmentVariable(string envVarName, Action? beforeProcessKill = null)
@@ -45,10 +60,27 @@ public static void StartWatchdogForPid(int pid, Action? beforeProcessKill = null
       StartWatchdogForPid(pid, Lifetime.Eternal, beforeProcessKill: beforeProcessKill);
     }
 
-    public static void StartWatchdogForPid(int pid, Lifetime lifetime, TimeSpan? gracefulShutdownPeriod = null, Action? beforeProcessKill = null)
+    public static void StartWatchdogForPid(
+      int pid,
+      Lifetime lifetime,
+      TimeSpan? gracefulShutdownPeriod = null,
+      Action? beforeProcessKill = null) =>
+      StartWatchdogForPid(new Options(pid, lifetime)
+      {
+        GracefulShutdownPeriod = gracefulShutdownPeriod,
+        BeforeProcessKill = beforeProcessKill
+      });
+
+    public static void StartWatchdogForPid(Options options)
     {
+      var pid = options.Pid;
       var watchThread = new Thread(() =>
       {
+        var lifetime = options.Lifetime;
+        var beforeProcessKill = options.BeforeProcessKill;
+        var gracefulShutdownPeriod = options.GracefulShutdownPeriod;
+        var killCurrentProcess = options.KillCurrentProcess;
+
         ourLogger.Info($"Monitoring parent process PID:{pid}");
 
         var useWinApi = true;
@@ -84,7 +116,10 @@ public static void StartWatchdogForPid(int pid, Lifetime lifetime, TimeSpan? gra
               // ignored
             }
 
-            Process.GetCurrentProcess().Kill();
+            if (killCurrentProcess != null)
+              killCurrentProcess();
+            else
+              Process.GetCurrentProcess().Kill();
             return;
           }
 
@@ -136,16 +171,18 @@ private static bool ProcessExists_Windows(int pid)
       var handle = IntPtr.Zero;
       try
       {
-        handle = Kernel32.OpenProcess(ProcessAccessRights.PROCESS_QUERY_LIMITED_INFORMATION, false, pid);
+        handle = Kernel32.OpenProcess(
+          ProcessAccessRights.PROCESS_QUERY_LIMITED_INFORMATION | ProcessAccessRights.SYNCHRONIZE,
+          false,
+          pid);
         if (handle == IntPtr.Zero)
         {
           var errorCode = Marshal.GetLastWin32Error();
           return errorCode == ERROR_INVALID_PARAMETER ? false : throw new Win32Exception(errorCode); // ERROR_INVALID_PARAMETER means that process doesn't exist
         }
 
-        return Kernel32.GetExitCodeProcess(handle, out var exitCode)
-          ? exitCode == ProcessExitCode.STILL_ALIVE
-          : throw new Win32Exception();
+        var isTerminated = Kernel32.WaitForSingleObject(handle, 0u) == 0u;
+        return !isTerminated;
       }
       finally
       {

rd-net/Lifetimes/Interop/Windows.cs

@@ -12,13 +12,10 @@ public static extern IntPtr OpenProcess(
             [In] ProcessAccessRights dwDesiredAccess,
             [In] bool bInheritHandle,
             [In] int dwProcessId);
-    
+
         [DllImport(DllName, SetLastError = true)]
-        public static extern bool GetExitCodeProcess(
-            [In] IntPtr hProcess,
-            [Out] out ProcessExitCode lpExitCode
-        );
-
+        public static extern uint WaitForSingleObject(IntPtr hHandle, uint dwMilliseconds);
+
         [DllImport(DllName, SetLastError = true)]
         public static extern bool CloseHandle(
             [In] IntPtr handle

rd-net/Test.Lifetimes/Diagnostics/ProcessWatchdogTest.cs

@@ -0,0 +1,83 @@
+#if !NET35
+using System;
+using System.Diagnostics;
+using System.Globalization;
+using System.Threading.Tasks;
+using JetBrains.Core;
+using JetBrains.Diagnostics;
+using JetBrains.Lifetimes;
+using JetBrains.Util;
+using NUnit.Framework;
+
+namespace Test.Lifetimes.Diagnostics;
+
+public class ProcessWatchdogTest
+{
+  [Test]
+  public Task TestWithSleepingProcess() => DoTest(StartSleepingProcess, true);
+
+  [Test]
+  public Task TestWithProcessReturning259() => DoTest(() => GetTerminatedProcess(259), false);
+
+  private static Task DoTest(Func<Process> processCreator, bool assertAlive) => Lifetime.UsingAsync(async lt =>
+  {
+    var process = lt.Bracket(
+      processCreator,
+      p =>
+      {
+        if (!p.HasExited) p.Kill();
+        p.Dispose();
+      });
+
+    var tcs = new TaskCompletionSource<Unit>();
+    var options = new ProcessWatchdog.Options(process.Id, lt)
+    {
+      BeforeProcessKill = () => tcs.SetResult(Unit.Instance),
+      KillCurrentProcess = () => { }
+    };
+
+    ProcessWatchdog.StartWatchdogForPid(options);
+
+    var timeForReliableDetection = ProcessWatchdog.DELAY_BEFORE_RETRY * 2;
+    var task = tcs.Task;
+    if (assertAlive)
+    {
+      await Task.Delay(timeForReliableDetection, lt);
+      Assert.IsFalse(task.IsCompleted);
+    }
+
+    if (!process.HasExited) process.Kill();
+
+    if (await Task.WhenAny(task, Task.Delay(timeForReliableDetection, lt)) != task)
+    {
+      Assert.Fail($"Termination of process {process.Id} wasn't detected during the timeout.");
+    }
+  });
+
+  private Process StartSleepingProcess()
+  {
+    if (RuntimeInfo.IsRunningUnderWindows)
+    {
+      return Process.Start(new ProcessStartInfo("cmd.exe", "/c timeout 30")
+      {
+        WindowStyle = ProcessWindowStyle.Hidden
+      });
+    }
+
+    return Process.Start("sleep", "30");
+  }
+
+  private Process GetTerminatedProcess(int exitCode)
+  {
+    var process = RuntimeInfo.IsRunningUnderWindows
+      ? Process.Start(new ProcessStartInfo("cmd.exe", $"/c exit {exitCode.ToString(CultureInfo.InvariantCulture)}")
+      {
+        WindowStyle = ProcessWindowStyle.Hidden
+      })
+      : Process.Start("/usr/bin/sh", $"-c \"exit {exitCode.ToString(CultureInfo.InvariantCulture)}\"");
+    process!.WaitForExit();
+    Assert.AreEqual(exitCode, process.ExitCode);
+    return process;
+  }
+}
+#endif