[Linux] correctly raise ZombieProcess on exe(), cmdline() and memory_…

…maps() (#2288)

HISTORY.rst

@@ -29,6 +29,8 @@ XXXX-XX-XX
 - 2284_, [Linux]: `memory_full_info`_ may incorrectly raise `ZombieProcess`_
  if it's determined via ``/proc/pid/smaps_rollup``. Instead we now fallback on
  reading ``/proc/pid/smaps``.
+- 2288_, [Linux]: correctly raise `ZombieProcess`_ on `exe`_, `cmdline`_ and
+  `memory_maps`_ instead of returning a "null" value.
 
 5.9.5
 =====

Makefile

@@ -180,7 +180,7 @@ test-memleaks:  ## Memory leak tests.
 	${MAKE} build
 	$(TEST_PREFIX) $(PYTHON) $(TSCRIPT) $(ARGS) psutil/tests/test_memleaks.py
 
-test-failed:  ## Re-run tests which failed on last run
+test-last-failed:  ## Re-run tests which failed on last run
 	${MAKE} build
 	$(TEST_PREFIX) $(PYTHON) $(TSCRIPT) $(ARGS) --last-failed
 

psutil/_psbsd.py

@@ -553,7 +553,7 @@ def is_zombie(pid):
     try:
         st = cext.proc_oneshot_info(pid)[kinfo_proc_map['status']]
         return PROC_STATUSES.get(st) == _common.STATUS_ZOMBIE
-    except Exception:
+    except OSError:
         return False
 
 

psutil/_pslinux.py

@@ -1654,12 +1654,12 @@ def wrapper(self, *args, **kwargs):
         except PermissionError:
             raise AccessDenied(self.pid, self._name)
         except ProcessLookupError:
+            self._raise_if_zombie()
             raise NoSuchProcess(self.pid, self._name)
         except FileNotFoundError:
+            self._raise_if_zombie()
             if not os.path.exists("%s/%s" % (self._procfs_path, self.pid)):
                 raise NoSuchProcess(self.pid, self._name)
-            # Note: zombies will keep existing under /proc until they're
-            # gone so there's no way to distinguish them in here.
             raise
     return wrapper
 
@@ -1675,7 +1675,27 @@ def __init__(self, pid):
         self._ppid = None
         self._procfs_path = get_procfs_path()
 
-    def _assert_alive(self):
+    def _is_zombie(self):
+        # Note: most of the times Linux is able to return info about the
+        # process even if it's a zombie, and /proc/{pid} will exist.
+        # There are some exceptions though, like exe(), cmdline() and
+        # memory_maps(). In these cases /proc/{pid}/{file} exists but
+        # it's empty. Instead of returning a "null" value we'll raise an
+        # exception.
+        try:
+            data = bcat("%s/%s/stat" % (self._procfs_path, self.pid))
+        except (IOError, OSError):
+            return False
+        else:
+            rpar = data.rfind(b')')
+            status = data[rpar + 2:rpar + 3]
+            return status == b"Z"
+
+    def _raise_if_zombie(self):
+        if self._is_zombie():
+            raise ZombieProcess(self.pid, self._name, self._ppid)
+
+    def _raise_if_not_alive(self):
         """Raise NSP if the process disappeared on us."""
         # For those C function who do not raise NSP, possibly returning
         # incorrect or incomplete result.
@@ -1749,29 +1769,26 @@ def name(self):
         # XXX - gets changed later and probably needs refactoring
         return name
 
+    @wrap_exceptions
     def exe(self):
         try:
             return readlink("%s/%s/exe" % (self._procfs_path, self.pid))
         except (FileNotFoundError, ProcessLookupError):
+            self._raise_if_zombie()
             # no such file error; might be raised also if the
             # path actually exists for system processes with
             # low pids (about 0-20)
             if os.path.lexists("%s/%s" % (self._procfs_path, self.pid)):
                 return ""
-            else:
-                if not pid_exists(self.pid):
-                    raise NoSuchProcess(self.pid, self._name)
-                else:
-                    raise ZombieProcess(self.pid, self._name, self._ppid)
-        except PermissionError:
-            raise AccessDenied(self.pid, self._name)
+            raise
 
     @wrap_exceptions
     def cmdline(self):
         with open_text("%s/%s/cmdline" % (self._procfs_path, self.pid)) as f:
             data = f.read()
         if not data:
             # may happen in case of zombie process
+            self._raise_if_zombie()
             return []
         # 'man proc' states that args are separated by null bytes '\0'
         # and last char is supposed to be a null byte. Nevertheless
@@ -1990,8 +2007,10 @@ def get_blocks(lines, current_block):
                 yield (current_block.pop(), data)
 
             data = self._read_smaps_file()
-            # Note: smaps file can be empty for certain processes.
+            # Note: smaps file can be empty for certain processes or for
+            # zombies.
             if not data:
+                self._raise_if_zombie()
                 return []
             lines = data.split(b'\n')
             ls = []
@@ -2030,14 +2049,7 @@ def get_blocks(lines, current_block):
 
     @wrap_exceptions
     def cwd(self):
-        try:
-            return readlink("%s/%s/cwd" % (self._procfs_path, self.pid))
-        except (FileNotFoundError, ProcessLookupError):
-            # https://github.com/giampaolo/psutil/issues/986
-            if not pid_exists(self.pid):
-                raise NoSuchProcess(self.pid, self._name)
-            else:
-                raise ZombieProcess(self.pid, self._name, self._ppid)
+        return readlink("%s/%s/cwd" % (self._procfs_path, self.pid))
 
     @wrap_exceptions
     def num_ctx_switches(self,
@@ -2086,7 +2098,7 @@ def threads(self):
             ntuple = _common.pthread(int(thread_id), utime, stime)
             retlist.append(ntuple)
         if hit_enoent:
-            self._assert_alive()
+            self._raise_if_not_alive()
         return retlist
 
     @wrap_exceptions
@@ -2179,12 +2191,11 @@ def rlimit(self, resource_, limits=None):
                             "got %s" % repr(limits))
                     prlimit(self.pid, resource_, limits)
             except OSError as err:
-                if err.errno == errno.ENOSYS and pid_exists(self.pid):
+                if err.errno == errno.ENOSYS:
                     # I saw this happening on Travis:
                     # https://travis-ci.org/giampaolo/psutil/jobs/51368273
-                    raise ZombieProcess(self.pid, self._name, self._ppid)
-                else:
-                    raise
+                    self._raise_if_zombie()
+                raise
 
     @wrap_exceptions
     def status(self):
@@ -2239,13 +2250,13 @@ def open_files(self):
                             path, int(fd), int(pos), mode, flags)
                         retlist.append(ntuple)
         if hit_enoent:
-            self._assert_alive()
+            self._raise_if_not_alive()
         return retlist
 
     @wrap_exceptions
     def connections(self, kind='inet'):
         ret = _connections.retrieve(kind, self.pid)
-        self._assert_alive()
+        self._raise_if_not_alive()
         return ret
 
     @wrap_exceptions

psutil/_psosx.py

@@ -332,7 +332,7 @@ def is_zombie(pid):
     try:
         st = cext.proc_kinfo_oneshot(pid)[kinfo_proc_map['status']]
         return st == cext.SZOMB
-    except Exception:
+    except OSError:
         return False
 
 

psutil/tests/__init__.py

@@ -964,17 +964,38 @@ def assertProcessZombie(self, proc):
         self.assertEqual(proc.status(), psutil.STATUS_ZOMBIE)
         # It should be considered 'running'.
         assert proc.is_running()
+        assert psutil.pid_exists(proc.pid)
         # as_dict() shouldn't crash.
         proc.as_dict()
         # It should show up in pids() and process_iter().
         self.assertIn(proc.pid, psutil.pids())
         self.assertIn(proc.pid, [x.pid for x in psutil.process_iter()])
-        # It cannot be signaled or terminated.
+        psutil._pmap = {}
+        self.assertIn(proc.pid, [x.pid for x in psutil.process_iter()])
+        # Call all methods.
+        ns = process_namespace(proc)
+        for fun, name in ns.iter(ns.all):
+            with self.subTest(name):
+                try:
+                    fun()
+                except (psutil.ZombieProcess, psutil.AccessDenied):
+                    pass
+        if LINUX:
+            # https://github.com/giampaolo/psutil/pull/2288
+            self.assertRaises(psutil.ZombieProcess, proc.cmdline)
+            self.assertRaises(psutil.ZombieProcess, proc.exe)
+            self.assertRaises(psutil.ZombieProcess, proc.memory_maps)
+        # Zombie cannot be signaled or terminated.
         proc.suspend()
         proc.resume()
         proc.terminate()
         proc.kill()
         assert proc.is_running()
+        assert psutil.pid_exists(proc.pid)
+        self.assertIn(proc.pid, psutil.pids())
+        self.assertIn(proc.pid, [x.pid for x in psutil.process_iter()])
+        psutil._pmap = {}
+        self.assertIn(proc.pid, [x.pid for x in psutil.process_iter()])
 
         # Its parent should 'see' it (edit: not true on BSD and MACOS).
         # descendants = [x.pid for x in psutil.Process().children(

psutil/tests/test_linux.py

@@ -2060,24 +2060,10 @@ def open_mock_2(name, *args, **kwargs):
 
     def test_exe_mocked(self):
         with mock.patch('psutil._pslinux.readlink',
-                        side_effect=OSError(errno.ENOENT, "")) as m1:
-            with mock.patch('psutil.Process.cmdline',
-                            side_effect=psutil.AccessDenied(0, "")) as m2:
-                # No such file error; might be raised also if /proc/pid/exe
-                # path actually exists for system processes with low pids
-                # (about 0-20). In this case psutil is supposed to return
-                # an empty string.
-                ret = psutil.Process().exe()
-                assert m1.called
-                assert m2.called
-                self.assertEqual(ret, "")
-
-                # ...but if /proc/pid no longer exist we're supposed to treat
-                # it as an alias for zombie process
-                with mock.patch('psutil._pslinux.os.path.lexists',
-                                return_value=False):
-                    self.assertRaises(
-                        psutil.ZombieProcess, psutil.Process().exe)
+                        side_effect=OSError(errno.ENOENT, "")) as m:
+            ret = psutil.Process().exe()
+            assert m.called
+            self.assertEqual(ret, "")
 
     def test_issue_1014(self):
         # Emulates a case where smaps file does not exist. In this case
@@ -2096,23 +2082,15 @@ def test_rlimit_zombie(self):
         # happen in case of zombie process:
         # https://travis-ci.org/giampaolo/psutil/jobs/51368273
         with mock.patch("psutil._pslinux.prlimit",
-                        side_effect=OSError(errno.ENOSYS, "")) as m:
-            p = psutil.Process()
-            p.name()
-            with self.assertRaises(psutil.ZombieProcess) as exc:
-                p.rlimit(psutil.RLIMIT_NOFILE)
-            assert m.called
-        self.assertEqual(exc.exception.pid, p.pid)
-        self.assertEqual(exc.exception.name, p.name())
-
-    def test_cwd_zombie(self):
-        with mock.patch("psutil._pslinux.os.readlink",
-                        side_effect=OSError(errno.ENOENT, "")) as m:
-            p = psutil.Process()
-            p.name()
-            with self.assertRaises(psutil.ZombieProcess) as exc:
-                p.cwd()
-            assert m.called
+                        side_effect=OSError(errno.ENOSYS, "")) as m1:
+            with mock.patch("psutil._pslinux.Process._is_zombie",
+                            return_value=True) as m2:
+                p = psutil.Process()
+                p.name()
+                with self.assertRaises(psutil.ZombieProcess) as exc:
+                    p.rlimit(psutil.RLIMIT_NOFILE)
+        assert m1.called
+        assert m2.called
         self.assertEqual(exc.exception.pid, p.pid)
         self.assertEqual(exc.exception.name, p.name())
 

psutil/tests/test_process.py

@@ -1320,23 +1320,8 @@ def assert_raises_nsp(fun, fun_name):
 
     @unittest.skipIf(not POSIX, 'POSIX only')
     def test_zombie_process(self):
-        def succeed_or_zombie_p_exc(fun):
-            try:
-                return fun()
-            except (psutil.ZombieProcess, psutil.AccessDenied):
-                pass
-
         parent, zombie = self.spawn_zombie()
         self.assertProcessZombie(zombie)
-        ns = process_namespace(zombie)
-        for fun, name in ns.iter(ns.all):
-            succeed_or_zombie_p_exc(fun)
-
-        assert psutil.pid_exists(zombie.pid)
-        self.assertIn(zombie.pid, psutil.pids())
-        self.assertIn(zombie.pid, [x.pid for x in psutil.process_iter()])
-        psutil._pmap = {}
-        self.assertIn(zombie.pid, [x.pid for x in psutil.process_iter()])
 
     @unittest.skipIf(not POSIX, 'POSIX only')
     def test_zombie_process_is_running_w_exc(self):