[Windows] #1877: turn OpenProcess -> ERROR_SUCCESS into AD or NSP

HISTORY.rst

@@ -20,6 +20,8 @@ XXXX-XX-XX
 - 1866_: [Windows] process exe(), cmdline(), environ() may raise "invalid
   access to memory location" on Python 3.9.
 - 1874_: [Solaris] wrong swap output given when encrypted column is present
+- 1877_: [Windows] OpenProcess may fail with ERROR_SUCCESS. Turn it into
+  AccessDenied or NoSuchProcess depending on whether the PID is alive.
 - 1886_: [macOS] EIO error may be raised on cmdline() and environment(). Now
   it gets translated into AccessDenied.
 

psutil/_psutil_bsd.c

@@ -477,10 +477,10 @@ psutil_proc_environ(PyObject *self, PyObject *args) {
                 kvm_close(kd);
                 return py_retdict;
             case EPERM:
-                AccessDenied("kvm_getenvv");
+                AccessDenied("kvm_getenvv -> EPERM");
                 break;
             case ESRCH:
-                NoSuchProcess("kvm_getenvv");
+                NoSuchProcess("kvm_getenvv -> ESRCH");
                 break;
 #if defined(PSUTIL_FREEBSD)
             case ENOMEM:
@@ -489,7 +489,7 @@ psutil_proc_environ(PyObject *self, PyObject *args) {
                 // "sudo procstat -e <pid of your XOrg server>".)
                 // Map the error condition to 'AccessDenied'.
                 sprintf(errbuf,
-                        "kvm_getenvv(pid=%ld, ki_uid=%d): errno=ENOMEM",
+                        "kvm_getenvv(pid=%ld, ki_uid=%d) -> ENOMEM",
                         pid, p->ki_uid);
                 AccessDenied(errbuf);
                 break;

psutil/_psutil_common.c

@@ -105,7 +105,7 @@ NoSuchProcess(const char *syscall) {
     PyObject *exc;
     char msg[1024];
 
-    sprintf(msg, "No such process (originated from %s)", syscall);
+    sprintf(msg, "assume no such process (originated from %s)", syscall);
     exc = PyObject_CallFunction(PyExc_OSError, "(is)", ESRCH, msg);
     PyErr_SetObject(PyExc_OSError, exc);
     Py_XDECREF(exc);
@@ -122,7 +122,7 @@ AccessDenied(const char *syscall) {
     PyObject *exc;
     char msg[1024];
 
-    sprintf(msg, "Access denied (originated from %s)", syscall);
+    sprintf(msg, "assume access denied (originated from %s)", syscall);
     exc = PyObject_CallFunction(PyExc_OSError, "(is)", EACCES, msg);
     PyErr_SetObject(PyExc_OSError, exc);
     Py_XDECREF(exc);

psutil/_psutil_osx.c

@@ -99,7 +99,8 @@ psutil_task_for_pid(pid_t pid, mach_port_t *task)
         if (psutil_pid_exists(pid) == 0)
             NoSuchProcess("task_for_pid");
         else if (psutil_is_zombie(pid) == 1)
-            PyErr_SetString(ZombieProcessError, "task_for_pid() failed");
+            PyErr_SetString(ZombieProcessError,
+                            "task_for_pid -> psutil_is_zombie -> 1");
         else {
             psutil_debug(
                 "task_for_pid() failed (pid=%ld, err=%i, errno=%i, msg='%s'); "

psutil/_psutil_windows.c

@@ -159,22 +159,15 @@ psutil_proc_kill(PyObject *self, PyObject *args) {
         return AccessDenied("automatically set for PID 0");
 
     hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
+    hProcess = psutil_check_phandle(hProcess, pid, 0);
     if (hProcess == NULL) {
-        if (GetLastError() == ERROR_INVALID_PARAMETER) {
-            // see https://github.com/giampaolo/psutil/issues/24
-            psutil_debug("OpenProcess -> ERROR_INVALID_PARAMETER turned "
-                         "into NoSuchProcess");
-            NoSuchProcess("OpenProcess");
-        }
-        else {
-            PyErr_SetFromWindowsErr(0);
-        }
         return NULL;
     }
 
     if (! TerminateProcess(hProcess, SIGTERM)) {
         // ERROR_ACCESS_DENIED may happen if the process already died. See:
         // https://github.com/giampaolo/psutil/issues/1099
+        // http://bugs.python.org/issue14252
         if (GetLastError() != ERROR_ACCESS_DENIED) {
             PyErr_SetFromOSErrnoWithSyscall("TerminateProcess");
             return NULL;
@@ -280,7 +273,7 @@ psutil_proc_times(PyObject *self, PyObject *args) {
         if (GetLastError() == ERROR_ACCESS_DENIED) {
             // usually means the process has died so we throw a NoSuchProcess
             // here
-            NoSuchProcess("GetProcessTimes");
+            NoSuchProcess("GetProcessTimes -> ERROR_ACCESS_DENIED");
         }
         else {
             PyErr_SetFromWindowsErr(0);
@@ -332,7 +325,7 @@ psutil_proc_cmdline(PyObject *self, PyObject *args, PyObject *kwdict) {
 
     pid_return = psutil_pid_is_running(pid);
     if (pid_return == 0)
-        return NoSuchProcess("psutil_pid_is_running");
+        return NoSuchProcess("psutil_pid_is_running -> 0");
     if (pid_return == -1)
         return NULL;
 
@@ -356,7 +349,7 @@ psutil_proc_environ(PyObject *self, PyObject *args) {
 
     pid_return = psutil_pid_is_running(pid);
     if (pid_return == 0)
-        return NoSuchProcess("psutil_pid_is_running");
+        return NoSuchProcess("psutil_pid_is_running -> 0");
     if (pid_return == -1)
         return NULL;
 
@@ -383,7 +376,7 @@ psutil_proc_exe(PyObject *self, PyObject *args) {
         return NULL;
 
     if (pid == 0)
-        return AccessDenied("forced for PID 0");
+        return AccessDenied("automatically set for PID 0");
 
     buffer = MALLOC_ZERO(bufferSize);
     if (! buffer)
@@ -417,7 +410,7 @@ psutil_proc_exe(PyObject *self, PyObject *args) {
     if (! NT_SUCCESS(status)) {
         FREE(buffer);
         if (psutil_pid_is_running(pid) == 0)
-            NoSuchProcess("NtQuerySystemInformation");
+            NoSuchProcess("psutil_pid_is_running -> 0");
         else
             psutil_SetFromNTStatusErr(status, "NtQuerySystemInformation");
         return NULL;
@@ -536,10 +529,10 @@ psutil_GetProcWsetInformation(
 
     if (!NT_SUCCESS(status)) {
         if (status == STATUS_ACCESS_DENIED) {
-            AccessDenied("NtQueryVirtualMemory");
+            AccessDenied("NtQueryVirtualMemory -> STATUS_ACCESS_DENIED");
         }
         else if (psutil_pid_is_running(pid) == 0) {
-            NoSuchProcess("psutil_pid_is_running");
+            NoSuchProcess("psutil_pid_is_running -> 0");
         }
         else {
             PyErr_Clear();
@@ -644,7 +637,7 @@ psutil_proc_cwd(PyObject *self, PyObject *args) {
 
     pid_return = psutil_pid_is_running(pid);
     if (pid_return == 0)
-        return NoSuchProcess("psutil_pid_is_running");
+        return NoSuchProcess("psutil_pid_is_running -> 0");
     if (pid_return == -1)
         return NULL;
 
@@ -703,13 +696,13 @@ psutil_proc_threads(PyObject *self, PyObject *args) {
     if (pid == 0) {
         // raise AD instead of returning 0 as procexp is able to
         // retrieve useful information somehow
-        AccessDenied("automatically set for PID 0");
+        AccessDenied("forced for PID 0");
         goto error;
     }
 
     pid_return = psutil_pid_is_running(pid);
     if (pid_return == 0) {
-        NoSuchProcess("psutil_pid_is_running");
+        NoSuchProcess("psutil_pid_is_running -> 0");
         goto error;
     }
     if (pid_return == -1)

psutil/arch/freebsd/specific.c

@@ -262,7 +262,7 @@ psutil_proc_exe(PyObject *self, PyObject *args) {
         if (ret == -1)
             return NULL;
         else if (ret == 0)
-            return NoSuchProcess("psutil_pid_exists");
+            return NoSuchProcess("psutil_pid_exists -> 0");
         else
             strcpy(pathname, "");
     }

psutil/arch/netbsd/specific.c

@@ -126,7 +126,7 @@ psutil_proc_cwd(PyObject *self, PyObject *args) {
     int name[] = { CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_CWD};
     if (sysctl(name, 4, path, &pathlen, NULL, 0) != 0) {
         if (errno == ENOENT)
-            NoSuchProcess("");
+            NoSuchProcess("sysctl -> ENOENT");
         else
             PyErr_SetFromErrno(PyExc_OSError);
         return NULL;
@@ -142,7 +142,7 @@ psutil_proc_cwd(PyObject *self, PyObject *args) {
     free(buf);
     if (len == -1) {
         if (errno == ENOENT)
-            NoSuchProcess("readlink (ENOENT)");
+            NoSuchProcess("readlink -> ENOENT");
         else
             PyErr_SetFromErrno(PyExc_OSError);
         return NULL;
@@ -198,7 +198,7 @@ psutil_proc_exe(PyObject *self, PyObject *args) {
         if (ret == -1)
             return NULL;
         else if (ret == 0)
-            return NoSuchProcess("psutil_pid_exists");
+            return NoSuchProcess("psutil_pid_exists -> 0");
         else
             strcpy(pathname, "");
     }

psutil/arch/osx/process_info.c

@@ -125,7 +125,7 @@ psutil_sysctl_procargs(pid_t pid, char *procargs, size_t argmax) {
 
     if (sysctl(mib, 3, procargs, &argmax, NULL, 0) < 0) {
         if (psutil_pid_exists(pid) == 0) {
-            NoSuchProcess("");
+            NoSuchProcess("psutil_pid_exists -> 0");
             return 1;
         }
         // In case of zombie process we'll get EINVAL. We translate it

psutil/arch/windows/process_info.c

@@ -55,10 +55,7 @@ psutil_convert_winerr(ULONG err, char* syscall) {
     char fullmsg[8192];
 
     if (err == ERROR_NOACCESS)  {
-        sprintf(
-            fullmsg,
-            "(originated from %s -> ERROR_NOACCESS; converted to AccessDenied)",
-            syscall);
+        sprintf(fullmsg, "%s -> ERROR_NOACCESS", syscall);
         psutil_debug(fullmsg);
         AccessDenied(fullmsg);
     }
@@ -434,7 +431,7 @@ psutil_cmdline_query_proc(DWORD pid, WCHAR **pdata, SIZE_T *psize) {
     // https://github.com/giampaolo/psutil/issues/1501
     if (status == STATUS_NOT_FOUND) {
         AccessDenied("NtQueryInformationProcess(ProcessBasicInformation) -> "
-                     "STATUS_NOT_FOUND translated into PermissionError");
+                     "STATUS_NOT_FOUND");
         goto error;
     }
 

psutil/arch/windows/process_utils.c

@@ -61,8 +61,10 @@ psutil_pid_in_pids(DWORD pid) {
     DWORD i;
 
     proclist = psutil_get_pids(&numberOfReturnedPIDs);
-    if (proclist == NULL)
+    if (proclist == NULL) {
+        psutil_debug("psutil_get_pids() failed");
         return -1;
+    }
     for (i = 0; i < numberOfReturnedPIDs; i++) {
         if (proclist[i] == pid) {
             free(proclist);
@@ -78,20 +80,36 @@ psutil_pid_in_pids(DWORD pid) {
 // 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) {
+psutil_check_phandle(HANDLE hProcess, DWORD pid, int check_exit_code) {
     DWORD exitCode;
 
     if (hProcess == NULL) {
         if (GetLastError() == ERROR_INVALID_PARAMETER) {
             // Yeah, this is the actual error code in case of
             // "no such process".
-            NoSuchProcess("OpenProcess");
+            NoSuchProcess("OpenProcess -> ERROR_INVALID_PARAMETER");
+            return NULL;
+        }
+        if (GetLastError() == ERROR_SUCCESS) {
+            // Yeah, it's this bad.
+            // https://github.com/giampaolo/psutil/issues/1877
+            if (psutil_pid_in_pids(pid) == 1) {
+                psutil_debug("OpenProcess -> ERROR_SUCCESS turned into AD");
+                AccessDenied("OpenProcess -> ERROR_SUCCESS");
+            }
+            else {
+                psutil_debug("OpenProcess -> ERROR_SUCCESS turned into NSP");
+                NoSuchProcess("OpenProcess -> ERROR_SUCCESS");
+            }
             return NULL;
         }
         PyErr_SetFromOSErrnoWithSyscall("OpenProcess");
         return NULL;
     }
 
+    if (check_exit_code == 0)
+        return hProcess;
+
     if (GetExitCodeProcess(hProcess, &exitCode)) {
         // XXX - maybe STILL_ACTIVE is not fully reliable as per:
         // http://stackoverflow.com/questions/1591342/#comment47830782_1591379
@@ -137,7 +155,7 @@ psutil_handle_from_pid(DWORD pid, DWORD access) {
         return NULL;
     }
 
-    hProcess = psutil_check_phandle(hProcess, pid);
+    hProcess = psutil_check_phandle(hProcess, pid, 1);
     return hProcess;
 }
 
@@ -159,7 +177,7 @@ psutil_pid_is_running(DWORD pid) {
     if ((hProcess == NULL) && (GetLastError() == ERROR_ACCESS_DENIED))
         return 1;
 
-    hProcess = psutil_check_phandle(hProcess, pid);
+    hProcess = psutil_check_phandle(hProcess, pid, 1);
     if (hProcess != NULL) {
         CloseHandle(hProcess);
         return 1;

psutil/arch/windows/process_utils.h

@@ -6,6 +6,7 @@
 
 DWORD* psutil_get_pids(DWORD *numberOfReturnedPIDs);
 HANDLE psutil_handle_from_pid(DWORD pid, DWORD dwDesiredAccess);
+HANDLE psutil_check_phandle(HANDLE hProcess, DWORD pid, int check_exit_code);
 int psutil_pid_is_running(DWORD pid);
 int psutil_assert_pid_exists(DWORD pid, char *err);
 int psutil_assert_pid_not_exists(DWORD pid, char *err);

psutil/tests/test_contracts.py

@@ -449,8 +449,12 @@ def exe(self, ret, info):
             # http://stackoverflow.com/questions/3112546/os-path-exists-lies
             if POSIX and os.path.isfile(ret):
                 if hasattr(os, 'access') and hasattr(os, "X_OK"):
-                    # XXX may fail on MACOS
-                    assert os.access(ret, os.X_OK)
+                    # XXX: may fail on MACOS
+                    try:
+                        assert os.access(ret, os.X_OK)
+                    except AssertionError:
+                        if os.path.exists(ret):
+                            raise
 
     def pid(self, ret, info):
         self.assertIsInstance(ret, int)

scripts/procsmem.py

@@ -80,18 +80,19 @@ def main():
                 procs.append(p)
 
     procs.sort(key=lambda p: p._uss)
-    templ = "%-7s %-7s %-30s %7s %7s %7s %7s"
-    print(templ % ("PID", "User", "Cmdline", "USS", "PSS", "Swap", "RSS"))
+    templ = "%-7s %-7s %7s %7s %7s %7s %7s"
+    print(templ % ("PID", "User", "USS", "PSS", "Swap", "RSS", "Cmdline"))
     print("=" * 78)
     for p in procs[:86]:
+        cmd = " ".join(p._info["cmdline"])[:50] if p._info["cmdline"] else ""
         line = templ % (
             p.pid,
             p._info["username"][:7] if p._info["username"] else "",
-            " ".join(p._info["cmdline"])[:30],
             convert_bytes(p._uss),
             convert_bytes(p._pss) if p._pss != "" else "",
             convert_bytes(p._swap) if p._swap != "" else "",
             convert_bytes(p._rss),
+            cmd,
         )
         print(line)
     if ad_pids: