diff --git a/HISTORY.rst b/HISTORY.rst index 52e909f00..f3bcba9d0 100644 --- a/HISTORY.rst +++ b/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 ===== diff --git a/Makefile b/Makefile index 0143abe08..c6b002a60 100644 --- a/Makefile +++ b/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 diff --git a/psutil/__init__.py b/psutil/__init__.py index 68f3c7ca9..49ab94d50 100644 --- a/psutil/__init__.py +++ b/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): diff --git a/psutil/_psbsd.py b/psutil/_psbsd.py index 37433c2db..fb4217efe 100644 --- a/psutil/_psbsd.py +++ b/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 diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py index e0acb0e29..0f102cbfa 100644 --- a/psutil/_pslinux.py +++ b/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,22 +1769,18 @@ 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): @@ -1772,6 +1788,7 @@ def cmdline(self): 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 diff --git a/psutil/_psosx.py b/psutil/_psosx.py index 893bdba18..8da2d9a32 100644 --- a/psutil/_psosx.py +++ b/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 diff --git a/psutil/tests/__init__.py b/psutil/tests/__init__.py index 0f9188397..34746de42 100644 --- a/psutil/tests/__init__.py +++ b/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): diff --git a/psutil/tests/test_contracts.py b/psutil/tests/test_contracts.py index 5878f505d..f44911af3 100755 --- a/psutil/tests/test_contracts.py +++ b/psutil/tests/test_contracts.py @@ -31,11 +31,11 @@ from psutil import POSIX from psutil import SUNOS from psutil import WINDOWS +from psutil._compat import PY3 from psutil._compat import FileNotFoundError from psutil._compat import long from psutil._compat import range from psutil._compat import unicode -from psutil._compat import PY3 from psutil.tests import APPVEYOR from psutil.tests import CI_TESTING from psutil.tests import GITHUB_ACTIONS @@ -361,6 +361,7 @@ def check_exception(exc, proc, name, ppid): tcase.assertEqual(exc.pid, pid) tcase.assertEqual(exc.name, name) if isinstance(exc, psutil.ZombieProcess): + tcase.assertProcessZombie(proc) if exc.ppid is not None: tcase.assertGreaterEqual(exc.ppid, 0) tcase.assertEqual(exc.ppid, ppid) @@ -403,14 +404,16 @@ class TestFetchAllProcesses(PsutilTestCase): Uses a process pool to get info about all processes. """ + use_proc_pool = not CI_TESTING + def setUp(self): # Using a pool in a CI env may result in deadlock, see: # https://github.com/giampaolo/psutil/issues/2104 - if not CI_TESTING: + if self.use_proc_pool: self.pool = multiprocessing.Pool() def tearDown(self): - if not CI_TESTING: + if self.use_proc_pool: self.pool.terminate() self.pool.join() @@ -419,7 +422,7 @@ def iter_proc_info(self): # same object as test_contracts.proc_info". from psutil.tests.test_contracts import proc_info - if not CI_TESTING: + if self.use_proc_pool: return self.pool.imap_unordered(proc_info, psutil.pids()) else: ls = [] diff --git a/psutil/tests/test_linux.py b/psutil/tests/test_linux.py index 912e44268..ed13c73c5 100755 --- a/psutil/tests/test_linux.py +++ b/psutil/tests/test_linux.py @@ -2062,24 +2062,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 @@ -2098,23 +2084,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()) diff --git a/psutil/tests/test_process.py b/psutil/tests/test_process.py index 7cb73f3e6..0f5e5ee6b 100755 --- a/psutil/tests/test_process.py +++ b/psutil/tests/test_process.py @@ -1335,40 +1335,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() - # A zombie process should always be instantiable - zproc = psutil.Process(zombie.pid) - # ...and at least its status always be querable - self.assertEqual(zproc.status(), psutil.STATUS_ZOMBIE) - # ...and it should be considered 'running' - assert zproc.is_running() - # ...and as_dict() shouldn't crash - zproc.as_dict() - # ...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(zpid, descendants) - # XXX should we also assume ppid 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(zpid.ppid(), os.getpid()) - # ...and all other APIs should be able to deal with it - - ns = process_namespace(zproc) - for fun, name in ns.iter(ns.all): - succeed_or_zombie_p_exc(fun) - - assert psutil.pid_exists(zproc.pid) - self.assertIn(zproc.pid, psutil.pids()) - self.assertIn(zproc.pid, [x.pid for x in psutil.process_iter()]) - psutil._pmap = {} - self.assertIn(zproc.pid, [x.pid for x in psutil.process_iter()]) + self.assertProcessZombie(zombie) @unittest.skipIf(not POSIX, 'POSIX only') def test_zombie_process_is_running_w_exc(self): diff --git a/psutil/tests/test_system.py b/psutil/tests/test_system.py index 95c77a5b1..0995b6970 100755 --- a/psutil/tests/test_system.py +++ b/psutil/tests/test_system.py @@ -30,9 +30,9 @@ from psutil import POSIX from psutil import SUNOS from psutil import WINDOWS +from psutil._compat import PY3 from psutil._compat import FileNotFoundError from psutil._compat import long -from psutil._compat import PY3 from psutil.tests import ASCII_FS from psutil.tests import CI_TESTING from psutil.tests import DEVNULL @@ -73,6 +73,11 @@ def test_process_iter(self): p.wait() self.assertNotIn(sproc.pid, [x.pid for x in psutil.process_iter()]) + # assert there are no duplicates + ls = [x for x in psutil.process_iter()] + self.assertEqual(sorted(ls, key=lambda x: x.pid), + sorted(set(ls), key=lambda x: x.pid)) + with mock.patch('psutil.Process', side_effect=psutil.NoSuchProcess(os.getpid())): self.assertEqual(list(psutil.process_iter()), [])