win: almost fix race detecting ESRCH in uv_kill (libuv#4341)

It might happen that only using `WaitForSingleObject()` with timeout 0
could return WAIT_TIMEOUT as the process might not have been signaled
yet. To improve things, first use `GetExitCodeProcess()` and check
that `status` is not `STILL_ACTIVE`. Then, to cover for the case that the exit
code was actually `STILL_ACTIVE` use `WaitForSingleObject()`. This could
still be prone to the race condition but only for that case.

Author: santigimeno

src/win/process.c

@@ -1364,23 +1364,49 @@ static int uv__kill(HANDLE process_handle, int signum) {
       /* Unconditionally terminate the process. On Windows, killed processes
        * normally return 1. */
       int err;
+      DWORD status;
 
       if (TerminateProcess(process_handle, 1))
         return 0;
 
-      /* If the process already exited before TerminateProcess was called,.
+      /* If the process already exited before TerminateProcess was called,
        * TerminateProcess will fail with ERROR_ACCESS_DENIED. */
       err = GetLastError();
-      if (err == ERROR_ACCESS_DENIED &&
-          WaitForSingleObject(process_handle, 0) == WAIT_OBJECT_0) {
-        return UV_ESRCH;
+      if (err == ERROR_ACCESS_DENIED) {
+        /* First check using GetExitCodeProcess() with status different from
+         * STILL_ACTIVE (259). This check can be set incorrectly by the process,
+         * though that is uncommon. */
+        if (GetExitCodeProcess(process_handle, &status) &&
+            status != STILL_ACTIVE) {
+          return UV_ESRCH;
+        }
+
+        /* But the process could have exited with code == STILL_ACTIVE, use then
+         * WaitForSingleObject with timeout zero. This is prone to a race
+         * condition as it could return WAIT_TIMEOUT because the handle might
+         * not have been signaled yet.That would result in returning the wrong
+         * error code here (UV_EACCES instead of UV_ESRCH), but we cannot fix
+         * the kernel synchronization issue that TerminateProcess is
+         * inconsistent with WaitForSingleObject with just the APIs available to
+         * us in user space. */
+        if (WaitForSingleObject(process_handle, 0) == WAIT_OBJECT_0) {
+          return UV_ESRCH;
+        }
       }
 
       return uv_translate_sys_error(err);
     }
 
     case 0: {
       /* Health check: is the process still alive? */
+      DWORD status;
+
+      if (!GetExitCodeProcess(process_handle, &status))
+        return uv_translate_sys_error(GetLastError());
+
+      if (status != STILL_ACTIVE)
+        return UV_ESRCH;
+
       switch (WaitForSingleObject(process_handle, 0)) {
         case WAIT_OBJECT_0:
           return UV_ESRCH;