1044 osx zombies (#1100)

* small refactoring

* #1044: define a separate ctx manager which handles zombie processes

* add create_zombie_proc utility function

* disable test on windows

* #1044: cmdline() was incorrectly raising AD instead of ZombieProcess

* #1044: environ() was incorrectly raising AD instead of ZombieProcess

* #1044: memory_maps() was incorrectly raising AD instead of ZombieProcess

* #1044: threads() was incorrectly raising AD instead of ZombieProcess

* enhance test

* fix threads()

psutil/_pslinux.py

@@ -1292,14 +1292,15 @@ def users():
 def boot_time():
     """Return the system boot time expressed in seconds since the epoch."""
     global BOOT_TIME
-    with open_binary('%s/stat' % get_procfs_path()) as f:
+    path = '%s/stat' % get_procfs_path()
+    with open_binary(path) as f:
         for line in f:
             if line.startswith(b'btime'):
                 ret = float(line.strip().split()[1])
                 BOOT_TIME = ret
                 return ret
         raise RuntimeError(
-            "line 'btime' not found in %s/stat" % get_procfs_path())
+            "line 'btime' not found in %s" % path)
 
 
 # =====================================================================
@@ -1331,14 +1332,15 @@ def pid_exists(pid):
             # Note: already checked that this is faster than using a
             # regular expr. Also (a lot) faster than doing
             # 'return pid in pids()'
-            with open_binary("%s/%s/status" % (get_procfs_path(), pid)) as f:
+            path = "%s/%s/status" % (get_procfs_path(), pid)
+            with open_binary(path) as f:
                 for line in f:
                     if line.startswith(b"Tgid:"):
                         tgid = int(line.split()[1])
                         # If tgid and pid are the same then we're
                         # dealing with a process PID.
                         return tgid == pid
-                raise ValueError("'Tgid' line not found")
+                raise ValueError("'Tgid' line not found in %s" % path)
         except (EnvironmentError, ValueError):
             return pid in pids()
 

psutil/_psosx.py

@@ -4,6 +4,7 @@
 
 """OSX platform implementation."""
 
+import contextlib
 import errno
 import functools
 import os
@@ -291,22 +292,40 @@ def wrapper(self, *args, **kwargs):
         try:
             return fun(self, *args, **kwargs)
         except OSError as err:
-            if self.pid == 0:
-                if 0 in pids():
-                    raise AccessDenied(self.pid, self._name)
-                else:
-                    raise
             if err.errno == errno.ESRCH:
-                if not pid_exists(self.pid):
-                    raise NoSuchProcess(self.pid, self._name)
-                else:
-                    raise ZombieProcess(self.pid, self._name, self._ppid)
+                raise NoSuchProcess(self.pid, self._name)
             if err.errno in (errno.EPERM, errno.EACCES):
                 raise AccessDenied(self.pid, self._name)
             raise
     return wrapper
 
 
+@contextlib.contextmanager
+def catch_zombie(proc):
+    """There are some poor C APIs which incorrectly raise ESRCH when
+    the process is still alive or it's a zombie, or even RuntimeError
+    (those who don't set errno). This is here in order to solve:
+    https://github.com/giampaolo/psutil/issues/1044
+    """
+    try:
+        yield
+    except (OSError, RuntimeError) as err:
+        if isinstance(err, RuntimeError) or err.errno == errno.ESRCH:
+            try:
+                # status() is not supposed to lie and correctly detect
+                # zombies so if it raises ESRCH it's true.
+                status = proc.status()
+            except NoSuchProcess:
+                raise err
+            else:
+                if status == _common.STATUS_ZOMBIE:
+                    raise ZombieProcess(proc.pid, proc._name, proc._ppid)
+                else:
+                    raise AccessDenied(proc.pid, proc._name)
+        else:
+            raise
+
+
 class Process(object):
     """Wrapper class around underlying C implementation."""
 
@@ -327,7 +346,8 @@ def _get_kinfo_proc(self):
     @memoize_when_activated
     def _get_pidtaskinfo(self):
         # Note: should work for PIDs owned by user only.
-        ret = cext.proc_pidtaskinfo_oneshot(self.pid)
+        with catch_zombie(self):
+            ret = cext.proc_pidtaskinfo_oneshot(self.pid)
         assert len(ret) == len(pidtaskinfo_map)
         return ret
 
@@ -346,19 +366,18 @@ def name(self):
 
     @wrap_exceptions
     def exe(self):
-        return cext.proc_exe(self.pid)
+        with catch_zombie(self):
+            return cext.proc_exe(self.pid)
 
     @wrap_exceptions
     def cmdline(self):
-        if not pid_exists(self.pid):
-            raise NoSuchProcess(self.pid, self._name)
-        return cext.proc_cmdline(self.pid)
+        with catch_zombie(self):
+            return cext.proc_cmdline(self.pid)
 
     @wrap_exceptions
     def environ(self):
-        if not pid_exists(self.pid):
-            raise NoSuchProcess(self.pid, self._name)
-        return parse_environ_block(cext.proc_environ(self.pid))
+        with catch_zombie(self):
+            return parse_environ_block(cext.proc_environ(self.pid))
 
     @wrap_exceptions
     def ppid(self):
@@ -367,7 +386,8 @@ def ppid(self):
 
     @wrap_exceptions
     def cwd(self):
-        return cext.proc_cwd(self.pid)
+        with catch_zombie(self):
+            return cext.proc_cwd(self.pid)
 
     @wrap_exceptions
     def uids(self):
@@ -440,7 +460,8 @@ def open_files(self):
         if self.pid == 0:
             return []
         files = []
-        rawlist = cext.proc_open_files(self.pid)
+        with catch_zombie(self):
+            rawlist = cext.proc_open_files(self.pid)
         for path, fd in rawlist:
             if isfile_strict(path):
                 ntuple = _common.popenfile(path, fd)
@@ -453,7 +474,8 @@ def connections(self, kind='inet'):
             raise ValueError("invalid %r kind argument; choose between %s"
                              % (kind, ', '.join([repr(x) for x in conn_tmap])))
         families, types = conn_tmap[kind]
-        rawlist = cext.proc_connections(self.pid, families, types)
+        with catch_zombie(self):
+            rawlist = cext.proc_connections(self.pid, families, types)
         ret = []
         for item in rawlist:
             fd, fam, type, laddr, raddr, status = item
@@ -468,7 +490,8 @@ def connections(self, kind='inet'):
     def num_fds(self):
         if self.pid == 0:
             return 0
-        return cext.proc_num_fds(self.pid)
+        with catch_zombie(self):
+            return cext.proc_num_fds(self.pid)
 
     @wrap_exceptions
     def wait(self, timeout=None):
@@ -479,11 +502,13 @@ def wait(self, timeout=None):
 
     @wrap_exceptions
     def nice_get(self):
-        return cext_posix.getpriority(self.pid)
+        with catch_zombie(self):
+            return cext_posix.getpriority(self.pid)
 
     @wrap_exceptions
     def nice_set(self, value):
-        return cext_posix.setpriority(self.pid, value)
+        with catch_zombie(self):
+            return cext_posix.setpriority(self.pid, value)
 
     @wrap_exceptions
     def status(self):
@@ -493,7 +518,8 @@ def status(self):
 
     @wrap_exceptions
     def threads(self):
-        rawlist = cext.proc_threads(self.pid)
+        with catch_zombie(self):
+            rawlist = cext.proc_threads(self.pid)
         retlist = []
         for thread_id, utime, stime in rawlist:
             ntuple = _common.pthread(thread_id, utime, stime)
@@ -502,4 +528,5 @@ def threads(self):
 
     @wrap_exceptions
     def memory_maps(self):
-        return cext.proc_memory_maps(self.pid)
+        with catch_zombie(self):
+            return cext.proc_memory_maps(self.pid)

psutil/_psutil_osx.c

@@ -125,6 +125,8 @@ psutil_pids(PyObject *self, PyObject *args) {
  * using sysctl() and filling up a kinfo_proc struct.
  * It should be possible to do this for all processes without
  * incurring into permission (EPERM) errors.
+ * This will also succeed for zombie processes returning correct
+ * information.
  */
 static PyObject *
 psutil_proc_kinfo_oneshot(PyObject *self, PyObject *args) {
@@ -173,8 +175,9 @@ psutil_proc_kinfo_oneshot(PyObject *self, PyObject *args) {
  * Return multiple process info as a Python tuple in one shot by
  * using proc_pidinfo(PROC_PIDTASKINFO) and filling a proc_taskinfo
  * struct.
- * Contrarily from proc_kinfo above this function will return EACCES
- * for PIDs owned by another user.
+ * Contrarily from proc_kinfo above this function will fail with
+ * EACCES for PIDs owned by another user and with ESRCH for zombie
+ * processes.
  */
 static PyObject *
 psutil_proc_pidtaskinfo_oneshot(PyObject *self, PyObject *args) {
@@ -226,6 +229,7 @@ psutil_proc_name(PyObject *self, PyObject *args) {
 
 /*
  * Return process current working directory.
+ * Raises NSP in case of zombie process.
  */
 static PyObject *
 psutil_proc_cwd(PyObject *self, PyObject *args) {
@@ -332,10 +336,7 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
 
     err = task_for_pid(mach_task_self(), (pid_t)pid, &task);
     if (err != KERN_SUCCESS) {
-        if (psutil_pid_exists(pid) == 0)
-            NoSuchProcess();
-        else
-            AccessDenied();
+        psutil_raise_for_pid(pid, "task_for_pid() failed");
         goto error;
     }
 
@@ -1002,7 +1003,7 @@ psutil_proc_threads(PyObject *self, PyObject *args) {
     if (! PyArg_ParseTuple(args, "l", &pid))
         goto error;
 
-    // task_for_pid() requires special privileges
+    // task_for_pid() requires root privileges
     err = task_for_pid(mach_task_self(), (pid_t)pid, &task);
     if (err != KERN_SUCCESS) {
         if (psutil_pid_exists(pid) == 0)
@@ -1186,6 +1187,7 @@ psutil_proc_open_files(PyObject *self, PyObject *args) {
 
 /*
  * Return process TCP and UDP connections as a list of tuples.
+ * Raises NSP in case of zombie process.
  * References:
  * - lsof source code: http://goo.gl/SYW79 and http://goo.gl/wNrC0
  * - /usr/include/sys/proc_info.h
@@ -1389,6 +1391,7 @@ psutil_proc_connections(PyObject *self, PyObject *args) {
 
 /*
  * Return number of file descriptors opened by process.
+ * Raises NSP in case of zombie process.
  */
 static PyObject *
 psutil_proc_num_fds(PyObject *self, PyObject *args) {

psutil/_psutil_windows.c

@@ -65,8 +65,11 @@
 typedef BOOL (WINAPI *LPFN_GLPI)
     (PSYSTEM_LOGICAL_PROCESSOR_INFORMATION,  PDWORD);
 
-// fix for mingw32, see
+// Fix for mingw32, see:
 // https://github.com/giampaolo/psutil/issues/351#c2
+// This is actually a DISK_PERFORMANCE struct:
+// https://msdn.microsoft.com/en-us/library/windows/desktop/
+//     aa363991(v=vs.85).aspx
 typedef struct _DISK_PERFORMANCE_WIN_2008 {
     LARGE_INTEGER BytesRead;
     LARGE_INTEGER BytesWritten;

psutil/arch/osx/process_info.c

@@ -141,13 +141,12 @@ psutil_get_cmdline(long pid) {
     mib[1] = KERN_PROCARGS2;
     mib[2] = (pid_t)pid;
     if (sysctl(mib, 3, procargs, &argmax, NULL, 0) < 0) {
-        if (EINVAL == errno) {
-            // EINVAL == access denied OR nonexistent PID
-            if (psutil_pid_exists(pid))
-                AccessDenied();
-            else
-                NoSuchProcess();
-        }
+        // In case of zombie process we'll get EINVAL. We translate it
+        // to NSP and _psosx.py will translate it to ZP.
+        if ((errno == EINVAL) && (psutil_pid_exists(pid)))
+            NoSuchProcess();
+        else
+            PyErr_SetFromErrno(PyExc_OSError);
         goto error;
     }
 
@@ -236,13 +235,12 @@ psutil_get_environ(long pid) {
     mib[1] = KERN_PROCARGS2;
     mib[2] = (pid_t)pid;
     if (sysctl(mib, 3, procargs, &argmax, NULL, 0) < 0) {
-        if (EINVAL == errno) {
-            // EINVAL == access denied OR nonexistent PID
-            if (psutil_pid_exists(pid))
-                AccessDenied();
-            else
-                NoSuchProcess();
-        }
+        // In case of zombie process we'll get EINVAL. We translate it
+        // to NSP and _psosx.py will translate it to ZP.
+        if ((errno == EINVAL) && (psutil_pid_exists(pid)))
+            NoSuchProcess();
+        else
+            PyErr_SetFromErrno(PyExc_OSError);
         goto error;
     }
 

psutil/tests/__init__.py

@@ -18,6 +18,7 @@
 import os
 import random
 import re
+import select
 import shutil
 import socket
 import stat
@@ -35,6 +36,7 @@
 from socket import SOCK_STREAM
 
 import psutil
+from psutil import OSX
 from psutil import POSIX
 from psutil import SUNOS
 from psutil import WINDOWS
@@ -71,7 +73,7 @@
     "HAS_SENSORS_BATTERY", "HAS_BATTERY""HAS_SENSORS_FANS",
     "HAS_SENSORS_TEMPERATURES", "HAS_MEMORY_FULL_INFO",
     # subprocesses
-    'pyrun', 'reap_children', 'get_test_subprocess',
+    'pyrun', 'reap_children', 'get_test_subprocess', 'create_zombie_proc',
     'create_proc_children_pair',
     # threads
     'ThreadTask'
@@ -330,6 +332,43 @@ def create_proc_children_pair():
     return (child1, child2)
 
 
+def create_zombie_proc():
+    """Create a zombie process and return its PID."""
+    assert psutil.POSIX
+    unix_file = tempfile.mktemp(prefix=TESTFILE_PREFIX) if OSX else TESTFN
+    src = textwrap.dedent("""\
+        import os, sys, time, socket, contextlib
+        child_pid = os.fork()
+        if child_pid > 0:
+            time.sleep(3000)
+        else:
+            # this is the zombie process
+            s = socket.socket(socket.AF_UNIX)
+            with contextlib.closing(s):
+                s.connect('%s')
+                if sys.version_info < (3, ):
+                    pid = str(os.getpid())
+                else:
+                    pid = bytes(str(os.getpid()), 'ascii')
+                s.sendall(pid)
+        """ % unix_file)
+    with contextlib.closing(socket.socket(socket.AF_UNIX)) as sock:
+        sock.settimeout(GLOBAL_TIMEOUT)
+        sock.bind(unix_file)
+        sock.listen(1)
+        pyrun(src)
+        conn, _ = sock.accept()
+        try:
+            select.select([conn.fileno()], [], [], GLOBAL_TIMEOUT)
+            zpid = int(conn.recv(1024))
+            _pids_started.add(zpid)
+            zproc = psutil.Process(zpid)
+            call_until(lambda: zproc.status(), "ret == psutil.STATUS_ZOMBIE")
+            return zpid
+        finally:
+            conn.close()
+
+
 @_cleanup_on_err
 def pyrun(src, **kwds):
     """Run python 'src' code string in a separate interpreter.