Merge pull request #52 from giampaolo/master

[pull] master from giampaolo:master

HISTORY.rst

@@ -24,8 +24,15 @@ XXXX-XX-XX
   (patch by student_2333)
 - 2268_: ``bytes2human()`` utility function was unable to properly represent
   negative values.
-- 2252_: [Windows]: `psutil.disk_usage`_ fails on Python 3.12+.  (patch by
+- 2252_, [Windows]: `psutil.disk_usage`_ fails on Python 3.12+.  (patch by
   Matthieu Darbois)
+- 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``.
+- 2287_, [OpenBSD], [NetBSD]: `Process.is_running()`_ erroneously return
+  ``False`` for zombie processes, because creation time cannot be determined.
+- 2288_, [Linux]: correctly raise `ZombieProcess`_ on `exe`_, `cmdline`_ and
+  `memory_maps`_ instead of returning a "null" value.
 
 5.9.5
 =====

Makefile

@@ -125,8 +125,8 @@ install-pip:  ## Install pip (no-op if already installed).
 setup-dev-env:  ## Install GIT hooks, pip, test deps (also upgrades them).
 	${MAKE} install-git-hooks
 	${MAKE} install-pip
-	$(PYTHON) -m pip install $(INSTALL_OPTS) --upgrade --trusted-host files.pythonhosted.org pip
-	$(PYTHON) -m pip install $(INSTALL_OPTS) --upgrade --trusted-host files.pythonhosted.org $(PY_DEPS)
+	$(PYTHON) -m pip install $(INSTALL_OPTS) --trusted-host files.pythonhosted.org --trusted-host pypi.org --upgrade pip
+	$(PYTHON) -m pip install $(INSTALL_OPTS) --trusted-host files.pythonhosted.org --trusted-host pypi.org --upgrade $(PY_DEPS)
 
 # ===================================================================
 # Tests
@@ -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/__init__.py

@@ -404,7 +404,7 @@ def __str__(self):
                 pass
             if self._exitcode not in (_SENTINEL, None):
                 info["exitcode"] = self._exitcode
-            if self._create_time:
+            if self._create_time is not None:
                 info['started'] = _pprint_secs(self._create_time)
             return "%s.%s(%s)" % (
                 self.__class__.__module__,
@@ -418,6 +418,20 @@ def __eq__(self, other):
         # on PID and creation time.
         if not isinstance(other, Process):
             return NotImplemented
+        if OPENBSD or NETBSD:  # pragma: no cover
+            # Zombie processes on Open/NetBSD have a creation time of
+            # 0.0. This covers the case when a process started normally
+            # (so it has a ctime), then it turned into a zombie. It's
+            # important to do this because is_running() depends on
+            # __eq__.
+            pid1, ctime1 = self._ident
+            pid2, ctime2 = other._ident
+            if pid1 == pid2:
+                if ctime1 and not ctime2:
+                    try:
+                        return self.status() == STATUS_ZOMBIE
+                    except Error:
+                        pass
         return self._ident == other._ident
 
     def __ne__(self, other):

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
@@ -1889,28 +1906,26 @@ def memory_info(self):
 
     if HAS_PROC_SMAPS_ROLLUP or HAS_PROC_SMAPS:
 
-        @wrap_exceptions
         def _parse_smaps_rollup(self):
             # /proc/pid/smaps_rollup was added to Linux in 2017. Faster
             # than /proc/pid/smaps. It reports higher PSS than */smaps
             # (from 1k up to 200k higher; tested against all processes).
+            # IMPORTANT: /proc/pid/smaps_rollup is weird, because it
+            # raises ESRCH / ENOENT for many PIDs, even if they're alive
+            # (also as root). In that case we'll use /proc/pid/smaps as
+            # fallback, which is slower but has a +50% success rate
+            # compared to /proc/pid/smaps_rollup.
             uss = pss = swap = 0
-            try:
-                with open_binary("{}/{}/smaps_rollup".format(
-                        self._procfs_path, self.pid)) as f:
-                    for line in f:
-                        if line.startswith(b"Private_"):
-                            # Private_Clean, Private_Dirty, Private_Hugetlb
-                            uss += int(line.split()[1]) * 1024
-                        elif line.startswith(b"Pss:"):
-                            pss = int(line.split()[1]) * 1024
-                        elif line.startswith(b"Swap:"):
-                            swap = int(line.split()[1]) * 1024
-            except ProcessLookupError:  # happens on readline()
-                if not pid_exists(self.pid):
-                    raise NoSuchProcess(self.pid, self._name)
-                else:
-                    raise ZombieProcess(self.pid, self._name, self._ppid)
+            with open_binary("{}/{}/smaps_rollup".format(
+                    self._procfs_path, self.pid)) as f:
+                for line in f:
+                    if line.startswith(b"Private_"):
+                        # Private_Clean, Private_Dirty, Private_Hugetlb
+                        uss += int(line.split()[1]) * 1024
+                    elif line.startswith(b"Pss:"):
+                        pss = int(line.split()[1]) * 1024
+                    elif line.startswith(b"Swap:"):
+                        swap = int(line.split()[1]) * 1024
             return (uss, pss, swap)
 
         @wrap_exceptions
@@ -1943,9 +1958,15 @@ def _parse_smaps(
             swap = sum(map(int, _swap_re.findall(smaps_data))) * 1024
             return (uss, pss, swap)
 
+        @wrap_exceptions
         def memory_full_info(self):
             if HAS_PROC_SMAPS_ROLLUP:  # faster
-                uss, pss, swap = self._parse_smaps_rollup()
+                try:
+                    uss, pss, swap = self._parse_smaps_rollup()
+                except (ProcessLookupError, FileNotFoundError) as err:
+                    debug("ignore %r for pid %s and retry using "
+                          "/proc/pid/smaps" % (err, self.pid))
+                    uss, pss, swap = self._parse_smaps()
             else:
                 uss, pss, swap = self._parse_smaps()
             basic_mem = self.memory_info()
@@ -1986,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 = []
@@ -2026,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,
@@ -2082,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
@@ -2175,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):
@@ -2235,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

@@ -43,6 +43,8 @@
 from psutil import AIX
 from psutil import LINUX
 from psutil import MACOS
+from psutil import NETBSD
+from psutil import OPENBSD
 from psutil import POSIX
 from psutil import SUNOS
 from psutil import WINDOWS
@@ -963,6 +965,65 @@ def assertProcessGone(self, proc):
         assert not psutil.pid_exists(proc.pid), proc.pid
         self.assertNotIn(proc.pid, psutil.pids())
 
+    def assertProcessZombie(self, proc):
+        # A zombie process should always be instantiable.
+        clone = psutil.Process(proc.pid)
+        # Cloned zombie on Open/NetBSD has null creation time, see:
+        # https://github.com/giampaolo/psutil/issues/2287
+        self.assertEqual(proc, clone)
+        if not (OPENBSD or NETBSD):
+            self.assertEqual(hash(proc), hash(clone))
+        # Its status always be querable.
+        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()])
+        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(
+        #                recursive=True)]
+        # self.assertIn(proc.pid, descendants)
+
+        # __eq__ can't be relied upon because creation time may not be
+        # querable.
+        # self.assertEqual(proc, psutil.Process(proc.pid))
+
+        # XXX should we also assume ppid() to be usable? Note: this
+        # would be an important use case as the only way to get
+        # rid of a zombie is to kill its parent.
+        # self.assertEqual(proc.ppid(), os.getpid())
+
 
 @unittest.skipIf(PYPY, "unreliable on PYPY")
 class TestMemoryLeak(PsutilTestCase):