Skip to content

Commit

Permalink
[Linux] memory_full_info() incorrectly raise ZombieProcess (#2284)
Browse files Browse the repository at this point in the history
...this happens, for example, with PID 2 (kthreadd):

```
>>> import psutil
>>> psutil.Process(2).memory_info()
pmem(rss=0, vms=0, shared=0, text=0, lib=0, data=0, dirty=0)
>>> psutil.Process(2).memory_full_info()
Traceback (most recent call last):
  File "/home/giampaolo/svn/psutil/psutil/_pslinux.py", line 1901, in _parse_smaps_rollup
    for line in f:
ProcessLookupError: [Errno 3] No such process

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/giampaolo/svn/psutil/psutil/__init__.py", line 1091, in memory_full_info
    return self._proc.memory_full_info()
  File "/home/giampaolo/svn/psutil/psutil/_pslinux.py", line 1948, in memory_full_info
    uss, pss, swap = self._parse_smaps_rollup()
  File "/home/giampaolo/svn/psutil/psutil/_pslinux.py", line 1653, in wrapper
    return fun(self, *args, **kwargs)
  File "/home/giampaolo/svn/psutil/psutil/_pslinux.py", line 1913, in _parse_smaps_rollup
    raise ZombieProcess(self.pid, self._name, self._ppid)
psutil.ZombieProcess: PID still exists but it's a zombie (pid=2)
```

The error originates from `/proc/pid/smaps_rollup`, which acts weirdly: it raises ESRCH / ENOENT for many PIDs, even if they're alive (also as root). In that case we'll have to use `/proc/pid/smaps` as fallback, which is slower but has a +50% success rate compared to ``/proc/pid/smaps_rollup``.
  • Loading branch information
giampaolo authored Aug 1, 2023
1 parent 179efa1 commit 1b7a3e1
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 47 deletions.
5 changes: 4 additions & 1 deletion HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
=====
Expand Down
40 changes: 22 additions & 18 deletions psutil/_pslinux.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
33 changes: 33 additions & 0 deletions psutil/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,39 @@ 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.
psutil.Process(proc.pid)
# 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()
# 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.
proc.suspend()
proc.resume()
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(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):
Expand Down
11 changes: 7 additions & 4 deletions psutil/tests/test_contracts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()

Expand All @@ -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 = []
Expand Down
29 changes: 6 additions & 23 deletions psutil/tests/test_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion psutil/tests/test_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1b7a3e1

Please sign in to comment.