diff --git a/HISTORY.rst b/HISTORY.rst index 52e909f00..72ad966c2 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -24,8 +24,11 @@ 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) +- 2283_, [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``. 5.9.5 ===== diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py index e0acb0e29..7ca6abcfc 100644 --- a/psutil/_pslinux.py +++ b/psutil/_pslinux.py @@ -1889,28 +1889,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 +1941,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() diff --git a/psutil/tests/__init__.py b/psutil/tests/__init__.py index 0781f0686..f5fdfb11d 100644 --- a/psutil/tests/__init__.py +++ b/psutil/tests/__init__.py @@ -957,6 +957,29 @@ 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) + self.assertEqual(proc, clone) + # Its status always be querable. + self.assertEqual(proc.status(), psutil.STATUS_ZOMBIE) + # It should be considered 'running'. + assert proc.is_running() + # as_dict() shouldn't crash. + proc.as_dict() + # Terminate and kill should not be possible. + proc.terminate() + proc.kill() + assert proc.is_running() + # 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()) + @unittest.skipIf(PYPY, "unreliable on PYPY") class TestMemoryLeak(PsutilTestCase): diff --git a/psutil/tests/test_contracts.py b/psutil/tests/test_contracts.py index e0bb92ce5..9ff6d6ada 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 @@ -359,6 +359,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) @@ -401,14 +402,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() @@ -417,7 +420,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_process.py b/psutil/tests/test_process.py index f08f034b2..46daa9a53 100755 --- a/psutil/tests/test_process.py +++ b/psutil/tests/test_process.py @@ -1327,33 +1327,16 @@ def succeed_or_zombie_p_exc(fun): 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) + 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(zproc.pid) - self.assertIn(zproc.pid, psutil.pids()) - self.assertIn(zproc.pid, [x.pid for x in psutil.process_iter()]) + 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(zproc.pid, [x.pid for x in psutil.process_iter()]) + 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): diff --git a/psutil/tests/test_system.py b/psutil/tests/test_system.py index 3f9676294..9e81e2f5d 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