[Windows] psutil_handle_from_pid() refactoring (#1668)

1 files changed, 52 insertions, 160 deletions

diff --git a/psutil/arch/windows/process_utils.c b/psutil/arch/windows/process_utils.c
index f6867ca1..dbdebd48 100644
--- a/psutil/arch/windows/process_utils.c
+++ b/psutil/arch/windows/process_utils.c
@@ -52,9 +52,8 @@ psutil_get_pids(DWORD *numberOfReturnedPIDs) {
     return procArray;
 }
 
-/*
- * Return 1 if PID exists, 0 if not, -1 on error.
- */
+
+// Return 1 if PID exists, 0 if not, -1 on error.
 int
 psutil_pid_in_pids(DWORD pid) {
     DWORD *proclist = NULL;
@@ -75,94 +74,53 @@ psutil_pid_in_pids(DWORD pid) {
 }
 
 
-/*
- * Given a process HANDLE checks whether it's actually running.
- * Returns:
- * - 1: running
- * - 0: not running
- * - -1: WindowsError
- * - -2: AssertionError
- */
-int
-psutil_is_phandle_running(HANDLE hProcess, DWORD pid) {
-    DWORD processExitCode = 0;
+// Given a process handle checks whether it's actually running. If it
+// does return the handle, else return NULL with Python exception set.
+// This is needed because OpenProcess API sucks.
+HANDLE
+psutil_check_phandle(HANDLE hProcess, DWORD pid) {
+    DWORD exitCode;
 
     if (hProcess == NULL) {
         if (GetLastError() == ERROR_INVALID_PARAMETER) {
             // Yeah, this is the actual error code in case of
             // "no such process".
-            if (! psutil_assert_pid_not_exists(
-                    pid, "iphr: OpenProcess() -> ERROR_INVALID_PARAMETER")) {
-                return -2;
-            }
-            return 0;
+            NoSuchProcess("OpenProcess");
+            return NULL;
         }
-        return -1;
+        PyErr_SetFromOSErrnoWithSyscall("OpenProcess");
+        return NULL;
     }
 
-    if (GetExitCodeProcess(hProcess, &processExitCode)) {
+    if (GetExitCodeProcess(hProcess, &exitCode)) {
         // XXX - maybe STILL_ACTIVE is not fully reliable as per:
         // http://stackoverflow.com/questions/1591342/#comment47830782_1591379
-        if (processExitCode == STILL_ACTIVE) {
-            if (! psutil_assert_pid_exists(
-                    pid, "iphr: GetExitCodeProcess() -> STILL_ACTIVE")) {
-                return -2;
-            }
-            return 1;
+        if (exitCode == STILL_ACTIVE) {
+            return hProcess;
         }
-        else {
-            // We can't be sure so we look into pids.
-            if (psutil_pid_in_pids(pid) == 1) {
-                return 1;
-            }
-            else {
-                CloseHandle(hProcess);
-                return 0;
-            }
+        if (psutil_pid_in_pids(pid) == 1) {
+            return hProcess;
         }
+        CloseHandle(hProcess);
+        NoSuchProcess("GetExitCodeProcess != STILL_ACTIVE");
+        return NULL;
     }
 
-    CloseHandle(hProcess);
-    if (! psutil_assert_pid_not_exists( pid, "iphr: exit fun")) {
-        return -2;
-    }
-    return -1;
-}
-
-
-/*
- * Given a process HANDLE checks whether it's actually running and if
- * it does return it, else return NULL with the proper Python exception
- * set.
- */
-HANDLE
-psutil_check_phandle(HANDLE hProcess, DWORD pid) {
-    int ret = psutil_is_phandle_running(hProcess, pid);
-    if (ret == 1) {
+    if (GetLastError() == ERROR_ACCESS_DENIED) {
+        psutil_debug("GetExitCodeProcess -> ERROR_ACCESS_DENIED (ignored)");
+        SetLastError(0);
         return hProcess;
     }
-    else if (ret == 0) {
-        return NoSuchProcess("psutil_is_phandle_running");
-    }
-    else if (ret == -1) {
-        if (GetLastError() == ERROR_ACCESS_DENIED)
-            return PyErr_SetFromWindowsErr(0);
-        else
-            return PyErr_SetFromOSErrnoWithSyscall("OpenProcess");
-    }
-    else {
-        return NULL;
-    }
+    PyErr_SetFromOSErrnoWithSyscall("GetExitCodeProcess");
+    CloseHandle(hProcess);
+    return NULL;
 }
 
 
-/*
- * A wrapper around OpenProcess setting NSP exception if process
- * no longer exists.
- * "pid" is the process pid, "dwDesiredAccess" is the first argument
- * exptected by OpenProcess.
- * Return a process handle or NULL.
- */
+// A wrapper around OpenProcess setting NSP exception if process no
+// longer exists. *pid* is the process PID, *access* is the first
+// argument to OpenProcess.
+// Return a process handle or NULL with exception set.
 HANDLE
 psutil_handle_from_pid(DWORD pid, DWORD access) {
     HANDLE hProcess;
@@ -171,113 +129,47 @@ psutil_handle_from_pid(DWORD pid, DWORD access) {
         // otherwise we'd get NoSuchProcess
         return AccessDenied("automatically set for PID 0");
     }
-    // needed for GetExitCodeProcess
-    access |= PROCESS_QUERY_LIMITED_INFORMATION;
-    hProcess = OpenProcess(access, FALSE, pid);
-    return psutil_check_phandle(hProcess, pid);
-}
 
+    hProcess = OpenProcess(access, FALSE, pid);
 
-int
-psutil_assert_pid_exists(DWORD pid, char *err) {
-    if (PSUTIL_TESTING) {
-        if (psutil_pid_in_pids(pid) == 0) {
-            PyErr_SetString(PyExc_AssertionError, err);
-            return 0;
-        }
+    if ((hProcess == NULL) && (GetLastError() == ERROR_ACCESS_DENIED)) {
+        PyErr_SetFromOSErrnoWithSyscall("OpenProcess");
+        return NULL;
     }
-    return 1;
-}
-
 
-int
-psutil_assert_pid_not_exists(DWORD pid, char *err) {
-    if (PSUTIL_TESTING) {
-        if (psutil_pid_in_pids(pid) == 1) {
-            PyErr_SetString(PyExc_AssertionError, err);
-            return 0;
-        }
-    }
-    return 1;
+    hProcess = psutil_check_phandle(hProcess, pid);
+    return hProcess;
 }
 
 
-/*
-/* Check for PID existance by using OpenProcess() + GetExitCodeProcess.
-/* Returns:
- * 1: pid exists
- * 0: it doesn't
- * -1: error
- */
+// Check for PID existance. Return 1 if pid exists, 0 if not, -1 on error.
 int
 psutil_pid_is_running(DWORD pid) {
     HANDLE hProcess;
-    DWORD exitCode;
-    DWORD err;
 
     // Special case for PID 0 System Idle Process
     if (pid == 0)
         return 1;
     if (pid < 0)
         return 0;
+    return psutil_pid_in_pids(pid);
+
     hProcess = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid);
-    if (NULL == hProcess) {
-        err = GetLastError();
-        // Yeah, this is the actual error code in case of "no such process".
-        if (err == ERROR_INVALID_PARAMETER) {
-            if (! psutil_assert_pid_not_exists(
-                    pid, "pir: OpenProcess() -> INVALID_PARAMETER")) {
-                return -1;
-            }
-            return 0;
-        }
-        // Access denied obviously means there's a process to deny access to.
-        else if (err == ERROR_ACCESS_DENIED) {
-            if (! psutil_assert_pid_exists(
-                    pid, "pir: OpenProcess() ACCESS_DENIED")) {
-                return -1;
-            }
-            return 1;
-        }
-        // Be strict and raise an exception; the caller is supposed
-        // to take -1 into account.
-        else {
-            PyErr_SetFromOSErrnoWithSyscall("OpenProcess(PROCESS_VM_READ)");
-            return -1;
-        }
-    }
 
-    if (GetExitCodeProcess(hProcess, &exitCode)) {
+    // Access denied means there's a process to deny access to.
+    if ((hProcess == NULL) && (GetLastError() == ERROR_ACCESS_DENIED))
+        return 1;
+
+    hProcess = psutil_check_phandle(hProcess, pid);
+    if (hProcess != NULL) {
         CloseHandle(hProcess);
-        // XXX - maybe STILL_ACTIVE is not fully reliable as per:
-        // http://stackoverflow.com/questions/1591342/#comment47830782_1591379
-        if (exitCode == STILL_ACTIVE) {
-            if (! psutil_assert_pid_exists(
-                    pid, "pir: GetExitCodeProcess() -> STILL_ACTIVE")) {
-                return -1;
-            }
-            return 1;
-        }
-        // We can't be sure so we look into pids.
-        else {
-            return psutil_pid_in_pids(pid);
-        }
+        return 1;
     }
-    else {
-        err = GetLastError();
-        CloseHandle(hProcess);
-        // Same as for OpenProcess, assume access denied means there's
-        // a process to deny access to.
-        if (err == ERROR_ACCESS_DENIED) {
-            if (! psutil_assert_pid_exists(
-                    pid, "pir: GetExitCodeProcess() -> ERROR_ACCESS_DENIED")) {
-                return -1;
-            }
-            return 1;
-        }
-        else {
-            PyErr_SetFromOSErrnoWithSyscall("GetExitCodeProcess");
-            return -1;
-        }
+
+    CloseHandle(hProcess);
+    if ((PSUTIL_TESTING) && (psutil_pid_in_pids(pid) == 1)) {
+        PyErr_SetString(PyExc_AssertionError, "NULL handle but PID exists");
+        return -1;
     }
+    return 0;
 }