diff options
author | Giampaolo Rodola <g.rodola@gmail.com> | 2021-10-03 15:55:16 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-10-03 15:55:16 +0200 |
commit | 5d39cc9c8d8dc04862786abba0307ed9350144ce (patch) | |
tree | e143b54d389d63ba6e21a7946345ed67babed5cb | |
parent | d01233263f046f07d5139a8611671525f74e3dd0 (diff) | |
download | psutil-5d39cc9c8d8dc04862786abba0307ed9350144ce.tar.gz |
Improve custom error tracebacks and messages (#1992)
Removal of duplicated `psutil.NoSuchProcess` text. Before:
```
psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists (pid=4651, name="python")
psutil.ZombieProcess: psutil.ZombieProcess process no longer exists and it's a zombie (pid=4651, name="python")
psutil.AccessDenied: psutil.AccessDenied (pid=4651, name="python")
psutil.TimeoutExpired: psutil.TimeoutExpired timeout after 5 seconds (pid=4651, name="python")
```
Now:
```
psutil.NoSuchProcess: process no longer exists (pid=4651, name="python")
psutil.ZombieProcess: process no longer exists and it's a zombie (pid=4651, name="python")
psutil.AccessDenied: (pid=4651, name="python")
psutil.TimeoutExpired: timeout after 5 seconds (pid=4651, name="python")
```
---
More info if process PID has been reused: Before:
```
psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists (pid=465148)
```
Now:
```
psutil.NoSuchProcess: process no longer exists and its PID has been reused (pid=465148)
```
---
Before:
```
psutil.NoSuchProcess: psutil.NoSuchProcess no process found with pid 666
```
Now:
```
psutil.NoSuchProcess: process PID not found (pid=666)
```
---
Before:
```
>>> psutil.NoSuchProcess(212, name="python")
psutil.NoSuchProcess process no longer exists (pid=212, name='python')
```
Now:
```
>>> psutil.NoSuchProcess(212, name="python")
psutil.NoSuchProcess(pid=212, name='python', msg='process no longer exists')
```
-rw-r--r-- | HISTORY.rst | 3 | ||||
-rw-r--r-- | psutil/__init__.py | 15 | ||||
-rw-r--r-- | psutil/_common.py | 79 | ||||
-rwxr-xr-x | psutil/tests/test_contracts.py | 1 | ||||
-rwxr-xr-x | psutil/tests/test_misc.py | 78 | ||||
-rwxr-xr-x | psutil/tests/test_process.py | 14 |
6 files changed, 108 insertions, 82 deletions
diff --git a/HISTORY.rst b/HISTORY.rst index ee7803d1..db0e4261 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -10,6 +10,9 @@ XXXX-XX-XX - 1851_: [Linux] cpu_freq() is slow on systems with many CPUs. Read current frequency values for all CPUs from /proc/cpuinfo instead of opening many files in /sys fs. (patch by marxin) +- 1992_: NoSuchProcess message now specifies if the PID has been reused. +- 1992_: error classes (NoSuchProcess, AccessDenied, etc.) now have a better + formatted and separated `__repr__` and `__str__` implementations. **Bug fixes** diff --git a/psutil/__init__.py b/psutil/__init__.py index 41f9bf5a..e52fdcda 100644 --- a/psutil/__init__.py +++ b/psutil/__init__.py @@ -269,7 +269,11 @@ def _assert_pid_not_reused(fun): @functools.wraps(fun) def wrapper(self, *args, **kwargs): if not self.is_running(): - raise NoSuchProcess(self.pid, self._name) + if self._pid_reused: + msg = "process no longer exists and its PID has been reused" + else: + msg = None + raise NoSuchProcess(self.pid, self._name, msg=msg) return fun(self, *args, **kwargs) return wrapper @@ -340,6 +344,7 @@ class Process(object): self._exe = None self._create_time = None self._gone = False + self._pid_reused = False self._hash = None self._lock = threading.RLock() # used for caching on Windows only (on POSIX ppid may change) @@ -364,8 +369,7 @@ class Process(object): pass except NoSuchProcess: if not _ignore_nsp: - msg = 'no process found with pid %s' % pid - raise NoSuchProcess(pid, None, msg) + raise NoSuchProcess(pid, msg='process PID not found') else: self._gone = True # This pair is supposed to indentify a Process instance @@ -571,7 +575,7 @@ class Process(object): It also checks if PID has been reused by another process in which case return False. """ - if self._gone: + if self._gone or self._pid_reused: return False try: # Checking if PID is alive is not enough as the PID might @@ -579,7 +583,8 @@ class Process(object): # verify process identity. # Process identity / uniqueness over time is guaranteed by # (PID + creation time) and that is verified in __eq__. - return self == Process(self.pid) + self._pid_reused = self != Process(self.pid) + return not self._pid_reused except ZombieProcess: # We should never get here as it's already handled in # Process.__init__; here just for extra safety. diff --git a/psutil/_common.py b/psutil/_common.py index 2acab626..306301ed 100644 --- a/psutil/_common.py +++ b/psutil/_common.py @@ -9,6 +9,7 @@ from __future__ import division, print_function +import collections import contextlib import errno import functools @@ -18,7 +19,6 @@ import stat import sys import threading import warnings -from collections import defaultdict from collections import namedtuple from socket import AF_INET from socket import SOCK_DGRAM @@ -275,15 +275,32 @@ class Error(Exception): """ __module__ = 'psutil' - def __init__(self, msg=""): - Exception.__init__(self, msg) - self.msg = msg + def _infodict(self, attrs): + try: + info = collections.OrderedDict() + except AttributeError: # pragma: no cover + info = {} # Python 2.6 + for name in attrs: + value = getattr(self, name, None) + if value: + info[name] = value + return info + + def __str__(self): + # invoked on `raise Error` + info = self._infodict(("pid", "ppid", "name")) + if info: + details = "(%s)" % ", ".join( + ["%s=%r" % (k, v) for k, v in info.items()]) + else: + details = None + return " ".join([x for x in (self.msg, details) if x]) def __repr__(self): - ret = "psutil.%s %s" % (self.__class__.__name__, self.msg) - return ret.strip() - - __str__ = __repr__ + # invoked on `repr(Error)` + info = self._infodict(("pid", "ppid", "name", "seconds", "msg")) + details = ", ".join(["%s=%r" % (k, v) for k, v in info.items()]) + return "psutil.%s(%s)" % (self.__class__.__name__, details) class NoSuchProcess(Error): @@ -293,16 +310,10 @@ class NoSuchProcess(Error): __module__ = 'psutil' def __init__(self, pid, name=None, msg=None): - Error.__init__(self, msg) + Error.__init__(self) self.pid = pid self.name = name - self.msg = msg - if msg is None: - if name: - details = "(pid=%s, name=%s)" % (self.pid, repr(self.name)) - else: - details = "(pid=%s)" % self.pid - self.msg = "process no longer exists " + details + self.msg = msg or "process no longer exists" class ZombieProcess(NoSuchProcess): @@ -315,19 +326,9 @@ class ZombieProcess(NoSuchProcess): __module__ = 'psutil' def __init__(self, pid, name=None, ppid=None, msg=None): - NoSuchProcess.__init__(self, msg) - self.pid = pid + NoSuchProcess.__init__(self, pid, name, msg) self.ppid = ppid - self.name = name - self.msg = msg - if msg is None: - args = ["pid=%s" % pid] - if name: - args.append("name=%s" % repr(self.name)) - if ppid: - args.append("ppid=%s" % self.ppid) - details = "(%s)" % ", ".join(args) - self.msg = "process still exists but it's a zombie " + details + self.msg = msg or "PID still exists but it's a zombie" class AccessDenied(Error): @@ -335,17 +336,10 @@ class AccessDenied(Error): __module__ = 'psutil' def __init__(self, pid=None, name=None, msg=None): - Error.__init__(self, msg) + Error.__init__(self) self.pid = pid self.name = name - self.msg = msg - if msg is None: - if (pid is not None) and (name is not None): - self.msg = "(pid=%s, name=%s)" % (pid, repr(name)) - elif (pid is not None): - self.msg = "(pid=%s)" % self.pid - else: - self.msg = "" + self.msg = msg or "" class TimeoutExpired(Error): @@ -355,14 +349,11 @@ class TimeoutExpired(Error): __module__ = 'psutil' def __init__(self, seconds, pid=None, name=None): - Error.__init__(self, "timeout after %s seconds" % seconds) + Error.__init__(self) self.seconds = seconds self.pid = pid self.name = name - if (pid is not None) and (name is not None): - self.msg += " (pid=%s, name=%s)" % (pid, repr(name)) - elif (pid is not None): - self.msg += " (pid=%s)" % self.pid + self.msg = "timeout after %s seconds" % seconds # =================================================================== @@ -629,8 +620,8 @@ class _WrapNumbers: assert name not in self.reminders assert name not in self.reminder_keys self.cache[name] = input_dict - self.reminders[name] = defaultdict(int) - self.reminder_keys[name] = defaultdict(set) + self.reminders[name] = collections.defaultdict(int) + self.reminder_keys[name] = collections.defaultdict(set) def _remove_dead_reminders(self, input_dict, name): """In case the number of keys changed between calls (e.g. a diff --git a/psutil/tests/test_contracts.py b/psutil/tests/test_contracts.py index 32c75fd7..b03477d9 100755 --- a/psutil/tests/test_contracts.py +++ b/psutil/tests/test_contracts.py @@ -360,7 +360,6 @@ def proc_info(pid): elif isinstance(exc, psutil.NoSuchProcess): tcase.assertProcessGone(proc) str(exc) - assert exc.msg def do_wait(): if pid != 0: diff --git a/psutil/tests/test_misc.py b/psutil/tests/test_misc.py index 81fa8f39..fc9f5b13 100755 --- a/psutil/tests/test_misc.py +++ b/psutil/tests/test_misc.py @@ -97,57 +97,71 @@ class TestMisc(PsutilTestCase): def test_process__str__(self): self.test_process__repr__(func=str) - def test_no_such_process__repr__(self, func=repr): + def test_no_such_process__repr__(self): self.assertEqual( repr(psutil.NoSuchProcess(321)), - "psutil.NoSuchProcess process no longer exists (pid=321)") + "psutil.NoSuchProcess(pid=321, msg='process no longer exists')") self.assertEqual( - repr(psutil.NoSuchProcess(321, name='foo')), - "psutil.NoSuchProcess process no longer exists (pid=321, " - "name='foo')") + repr(psutil.NoSuchProcess(321, name="name", msg="msg")), + "psutil.NoSuchProcess(pid=321, name='name', msg='msg')") + + def test_no_such_process__str__(self): + self.assertEqual( + str(psutil.NoSuchProcess(321)), + "process no longer exists (pid=321)") self.assertEqual( - repr(psutil.NoSuchProcess(321, msg='foo')), - "psutil.NoSuchProcess foo") + str(psutil.NoSuchProcess(321, name="name", msg="msg")), + "msg (pid=321, name='name')") - def test_zombie_process__repr__(self, func=repr): + def test_zombie_process__repr__(self): self.assertEqual( repr(psutil.ZombieProcess(321)), - "psutil.ZombieProcess process still exists but it's a zombie " - "(pid=321)") + 'psutil.ZombieProcess(pid=321, msg="PID still ' + 'exists but it\'s a zombie")') self.assertEqual( - repr(psutil.ZombieProcess(321, name='foo')), - "psutil.ZombieProcess process still exists but it's a zombie " - "(pid=321, name='foo')") + repr(psutil.ZombieProcess(321, name="name", ppid=320, msg="foo")), + "psutil.ZombieProcess(pid=321, ppid=320, name='name', msg='foo')") + + def test_zombie_process__str__(self): self.assertEqual( - repr(psutil.ZombieProcess(321, name='foo', ppid=1)), - "psutil.ZombieProcess process still exists but it's a zombie " - "(pid=321, name='foo', ppid=1)") + str(psutil.ZombieProcess(321)), + "PID still exists but it's a zombie (pid=321)") self.assertEqual( - repr(psutil.ZombieProcess(321, msg='foo')), - "psutil.ZombieProcess foo") + str(psutil.ZombieProcess(321, name="name", ppid=320, msg="foo")), + "foo (pid=321, ppid=320, name='name')") - def test_access_denied__repr__(self, func=repr): + def test_access_denied__repr__(self): self.assertEqual( repr(psutil.AccessDenied(321)), - "psutil.AccessDenied (pid=321)") + "psutil.AccessDenied(pid=321)") self.assertEqual( - repr(psutil.AccessDenied(321, name='foo')), - "psutil.AccessDenied (pid=321, name='foo')") + repr(psutil.AccessDenied(321, name="name", msg="msg")), + "psutil.AccessDenied(pid=321, name='name', msg='msg')") + + def test_access_denied__str__(self): self.assertEqual( - repr(psutil.AccessDenied(321, msg='foo')), - "psutil.AccessDenied foo") + str(psutil.AccessDenied(321)), + "(pid=321)") + self.assertEqual( + str(psutil.AccessDenied(321, name="name", msg="msg")), + "msg (pid=321, name='name')") - def test_timeout_expired__repr__(self, func=repr): + def test_timeout_expired__repr__(self): self.assertEqual( - repr(psutil.TimeoutExpired(321)), - "psutil.TimeoutExpired timeout after 321 seconds") + repr(psutil.TimeoutExpired(5)), + "psutil.TimeoutExpired(seconds=5, msg='timeout after 5 seconds')") + self.assertEqual( + repr(psutil.TimeoutExpired(5, pid=321, name="name")), + "psutil.TimeoutExpired(pid=321, name='name', seconds=5, " + "msg='timeout after 5 seconds')") + + def test_timeout_expired__str__(self): self.assertEqual( - repr(psutil.TimeoutExpired(321, pid=111)), - "psutil.TimeoutExpired timeout after 321 seconds (pid=111)") + str(psutil.TimeoutExpired(5)), + "timeout after 5 seconds") self.assertEqual( - repr(psutil.TimeoutExpired(321, pid=111, name='foo')), - "psutil.TimeoutExpired timeout after 321 seconds " - "(pid=111, name='foo')") + str(psutil.TimeoutExpired(5, pid=321, name="name")), + "timeout after 5 seconds (pid=321, name='name')") def test_process__eq__(self): p1 = psutil.Process() diff --git a/psutil/tests/test_process.py b/psutil/tests/test_process.py index dfe88547..002c58de 100755 --- a/psutil/tests/test_process.py +++ b/psutil/tests/test_process.py @@ -1332,6 +1332,20 @@ class TestProcess(PsutilTestCase): self.assertEqual(p.status(), psutil.STATUS_ZOMBIE) assert m.called + def test_reused_pid(self): + # Emulate a case where PID has been reused by another process. + subp = self.spawn_testproc() + p = psutil.Process(subp.pid) + p._ident = (p.pid, p.create_time() + 100) + assert not p.is_running() + assert p != psutil.Process(subp.pid) + msg = "process no longer exists and its PID has been reused" + self.assertRaisesRegex(psutil.NoSuchProcess, msg, p.suspend) + self.assertRaisesRegex(psutil.NoSuchProcess, msg, p.resume) + self.assertRaisesRegex(psutil.NoSuchProcess, msg, p.terminate) + self.assertRaisesRegex(psutil.NoSuchProcess, msg, p.kill) + self.assertRaisesRegex(psutil.NoSuchProcess, msg, p.children) + def test_pid_0(self): # Process(0) is supposed to work on all platforms except Linux if 0 not in psutil.pids(): |