Merge branch 'master' of github.com:giampaolo/psutil

7 files changed, 221 insertions, 58 deletions

diff --git a/HISTORY.rst b/HISTORY.rst
index 4a277c08..a97a802a 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -1,5 +1,17 @@
 Bug tracker at https://github.com/giampaolo/psutil/issues
 
+3.0.2 - XXXX-XX-XX
+==================
+
+**Bug fixes**
+
+- #636: [Linux] *connections functions may swallow errors and return an
+  incomplete list of connnections.
+- #637: [UNIX] raise exception if trying to send signal to Process PID 0 as it
+  will affect os.getpid()'s process group instead of PID 0.
+- #639: [Linux] Process.cmdline() can be truncated.
+
+
 3.0.1 - 2015-06-18
 ==================
 
diff --git a/docs/index.rst b/docs/index.rst
index 9d527ebb..51cc6c7b 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -598,9 +598,8 @@ Exceptions
    method called the OS may be able to succeed in retrieving the process
    information or not.
    Note: this is a subclass of :class:`NoSuchProcess` so if you're not
-   interested in retrieving zombies while iterating over all processes (e.g.
-   via :func:`process_iter()`) you can ignore this exception and just catch
-   :class:`NoSuchProcess`.
+   interested in retrieving zombies (e.g. when using :func:`process_iter()`)
+   you can ignore this exception and just catch :class:`NoSuchProcess`.
 
    *New in 3.0.0*
 
diff --git a/psutil/__init__.py b/psutil/__init__.py
index 64a8d691..7a3fb066 100644
--- a/psutil/__init__.py
+++ b/psutil/__init__.py
@@ -681,7 +681,7 @@ class Process(object):
             """
             if ioclass is None:
                 if value is not None:
-                    raise ValueError("'ioclass' must be specified")
+                    raise ValueError("'ioclass' argument must be specified")
                 return self._proc.ionice_get()
             else:
                 return self._proc.ionice_set(ioclass, value)
@@ -1007,10 +1007,12 @@ class Process(object):
 
     if _POSIX:
         def _send_signal(self, sig):
-            # XXX: according to "man 2 kill" PID 0 has a special
-            # meaning as it refers to <<every process in the process
-            # group of the calling process>>, so should we prevent
-            # it here?
+            if self.pid == 0:
+                # see "man 2 kill"
+                raise ValueError(
+                    "preventing sending signal to process with PID 0 as it "
+                    "would affect every process in the process group of the "
+                    "calling process (os.getpid()) instead of PID 0")
             try:
                 os.kill(self.pid, sig)
             except OSError as err:
@@ -1853,14 +1855,6 @@ def test():
                                     time.localtime(sum(pinfo['cpu_times'])))
             try:
                 user = p.username()
-            except KeyError:
-                if _POSIX:
-                    if pinfo['uids']:
-                        user = str(pinfo['uids'].real)
-                    else:
-                        user = ''
-                else:
-                    raise
             except Error:
                 user = ''
             if _WINDOWS and '\\' in user:
diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py
index a0a4d350..be443eff 100644
--- a/psutil/_pslinux.py
+++ b/psutil/_pslinux.py
@@ -25,7 +25,7 @@ from . import _psutil_linux as cext
 from . import _psutil_posix as cext_posix
 from ._common import isfile_strict, usage_percent
 from ._common import NIC_DUPLEX_FULL, NIC_DUPLEX_HALF, NIC_DUPLEX_UNKNOWN
-from ._compat import PY3
+from ._compat import PY3, long
 
 if sys.version_info >= (3, 4):
     import enum
@@ -329,7 +329,7 @@ def boot_time():
                 ret = float(line.strip().split()[1])
                 BOOT_TIME = ret
                 return ret
-        raise RuntimeError("line 'btime' not found")
+        raise RuntimeError("line 'btime' not found in /proc/stat")
 
 
 # --- processes
@@ -383,9 +383,17 @@ class Connections:
         for fd in os.listdir("/proc/%s/fd" % pid):
             try:
                 inode = os.readlink("/proc/%s/fd/%s" % (pid, fd))
-            except OSError:
-                # TODO: need comment here
-                continue
+            except OSError as err:
+                # ENOENT == file which is gone in the meantime;
+                # os.stat('/proc/%s' % self.pid) will be done later
+                # to force NSP (if it's the case)
+                if err.errno in (errno.ENOENT, errno.ESRCH):
+                    continue
+                elif err.errno == errno.EINVAL:
+                    # not a link
+                    continue
+                else:
+                    raise
             else:
                 if inode.startswith('socket:['):
                     # the process is using a socket
@@ -744,7 +752,7 @@ class Process(object):
     def exe(self):
         try:
             exe = os.readlink("/proc/%s/exe" % self.pid)
-        except (OSError, IOError) as err:
+        except OSError as err:
             if err.errno in (errno.ENOENT, errno.ESRCH):
                 # no such file error; might be raised also if the
                 # path actually exists for system processes with
@@ -775,7 +783,10 @@ class Process(object):
         fname = "/proc/%s/cmdline" % self.pid
         kw = dict(encoding=DEFAULT_ENCODING) if PY3 else dict()
         with open(fname, "rt", **kw) as f:
-            return [x for x in f.read()[:-1].split('\x00')]
+            data = f.read()
+        if data.endswith('\x00'):
+            data = data[:-1]
+        return [x for x in data.split('\x00')]
 
     @wrap_exceptions
     def terminal(self):
@@ -983,7 +994,7 @@ class Process(object):
             try:
                 with open(fname, 'rb') as f:
                     st = f.read().strip()
-            except EnvironmentError as err:
+            except IOError as err:
                 if err.errno == errno.ENOENT:
                     # no such file or directory; it means thread
                     # disappeared on us
@@ -1044,32 +1055,43 @@ class Process(object):
 
         @wrap_exceptions
         def ionice_set(self, ioclass, value):
+            if value is not None:
+                if not PY3 and not isinstance(value, (int, long)):
+                    msg = "value argument is not an integer (gor %r)" % value
+                    raise TypeError(msg)
+                if not 0 <= value <= 8:
+                    raise ValueError(
+                        "value argument range expected is between 0 and 8")
+
             if ioclass in (IOPRIO_CLASS_NONE, None):
                 if value:
-                    msg = "can't specify value with IOPRIO_CLASS_NONE"
+                    msg = "can't specify value with IOPRIO_CLASS_NONE " \
+                          "(got %r)" % value
                     raise ValueError(msg)
                 ioclass = IOPRIO_CLASS_NONE
                 value = 0
-            if ioclass in (IOPRIO_CLASS_RT, IOPRIO_CLASS_BE):
-                if value is None:
-                    value = 4
             elif ioclass == IOPRIO_CLASS_IDLE:
                 if value:
-                    msg = "can't specify value with IOPRIO_CLASS_IDLE"
+                    msg = "can't specify value with IOPRIO_CLASS_IDLE " \
+                          "(got %r)" % value
                     raise ValueError(msg)
                 value = 0
+            elif ioclass in (IOPRIO_CLASS_RT, IOPRIO_CLASS_BE):
+                if value is None:
+                    # TODO: add comment explaining why this is 4 (?)
+                    value = 4
             else:
-                value = 0
-            if not 0 <= value <= 8:
-                raise ValueError(
-                    "value argument range expected is between 0 and 8")
+                # otherwise we would get OSError(EVINAL)
+                raise ValueError("invalid ioclass argument %r" % ioclass)
+
             return cext.proc_ioprio_set(self.pid, ioclass, value)
 
     if HAS_PRLIMIT:
         @wrap_exceptions
         def rlimit(self, resource, limits=None):
-            # if pid is 0 prlimit() applies to the calling process and
-            # we don't want that
+            # If pid is 0 prlimit() applies to the calling process and
+            # we don't want that. We should never get here though as
+            # PID 0 is not supported on Linux.
             if self.pid == 0:
                 raise ValueError("can't use prlimit() against PID 0 process")
             try:
@@ -1080,7 +1102,8 @@ class Process(object):
                     # set
                     if len(limits) != 2:
                         raise ValueError(
-                            "second argument must be a (soft, hard) tuple")
+                            "second argument must be a (soft, hard) tuple, "
+                            "got %s" % repr(limits))
                     soft, hard = limits
                     cext.linux_prlimit(self.pid, resource, soft, hard)
             except OSError as err:
@@ -1148,27 +1171,30 @@ class Process(object):
 
     @wrap_exceptions
     def ppid(self):
-        with open("/proc/%s/status" % self.pid, 'rb') as f:
+        fpath = "/proc/%s/status" % self.pid
+        with open(fpath, 'rb') as f:
             for line in f:
                 if line.startswith(b"PPid:"):
                     # PPid: nnnn
                     return int(line.split()[1])
-            raise NotImplementedError("line not found")
+            raise NotImplementedError("line 'PPid' not found in %s" % fpath)
 
     @wrap_exceptions
     def uids(self):
-        with open("/proc/%s/status" % self.pid, 'rb') as f:
+        fpath = "/proc/%s/status" % self.pid
+        with open(fpath, 'rb') as f:
             for line in f:
                 if line.startswith(b'Uid:'):
                     _, real, effective, saved, fs = line.split()
                     return _common.puids(int(real), int(effective), int(saved))
-            raise NotImplementedError("line not found")
+            raise NotImplementedError("line 'Uid' not found in %s" % fpath)
 
     @wrap_exceptions
     def gids(self):
-        with open("/proc/%s/status" % self.pid, 'rb') as f:
+        fpath = "/proc/%s/status" % self.pid
+        with open(fpath, 'rb') as f:
             for line in f:
                 if line.startswith(b'Gid:'):
                     _, real, effective, saved, fs = line.split()
                     return _common.pgids(int(real), int(effective), int(saved))
-            raise NotImplementedError("line not found")
+            raise NotImplementedError("line 'Gid' not found in %s" % fpath)
diff --git a/test/_linux.py b/test/_linux.py
index 5d7f0521..493c1491 100644
--- a/test/_linux.py
+++ b/test/_linux.py
@@ -8,6 +8,7 @@
 
 from __future__ import division
 import contextlib
+import errno
 import fcntl
 import os
 import pprint
@@ -15,6 +16,7 @@ import re
 import socket
 import struct
 import sys
+import tempfile
 import time
 import warnings
 
@@ -26,7 +28,7 @@ except ImportError:
 from test_psutil import POSIX, TOLERANCE, TRAVIS, LINUX
 from test_psutil import (skip_on_not_implemented, sh, get_test_subprocess,
                          retry_before_failing, get_kernel_version, unittest,
-                         which)
+                         which, call_until)
 
 import psutil
 import psutil._pslinux
@@ -280,6 +282,26 @@ class LinuxSpecificTestCase(unittest.TestCase):
             self.assertIsNone(psutil._pslinux.cpu_count_physical())
             assert m.called
 
+    def test_proc_open_files_file_gone(self):
+        # simulates a file which gets deleted during open_files()
+        # execution
+        p = psutil.Process()
+        files = p.open_files()
+        with tempfile.NamedTemporaryFile():
+            # give the kernel some time to see the new file
+            call_until(p.open_files, "len(ret) != %i" % len(files))
+            with mock.patch('psutil._pslinux.os.readlink',
+                            side_effect=OSError(errno.ENOENT, "")) as m:
+                files = p.open_files()
+                assert not files
+                assert m.called
+            # also simulate the case where os.readlink() returns EINVAL
+            # in which case psutil is supposed to 'continue'
+            with mock.patch('psutil._pslinux.os.readlink',
+                            side_effect=OSError(errno.EINVAL, "")) as m:
+                self.assertEqual(p.open_files(), [])
+                assert m.called
+
     def test_proc_terminal_mocked(self):
         with mock.patch('psutil._pslinux._psposix._get_terminal_map',
                         return_value={}) as m:
@@ -321,6 +343,20 @@ class LinuxSpecificTestCase(unittest.TestCase):
                 psutil._pslinux.Process(os.getpid()).gids)
             assert m.called
 
+    def test_proc_io_counters_mocked(self):
+        with mock.patch('psutil._pslinux.open', create=True) as m:
+            self.assertRaises(
+                NotImplementedError,
+                psutil._pslinux.Process(os.getpid()).io_counters)
+            assert m.called
+
+    def test_boot_time_mocked(self):
+        with mock.patch('psutil._pslinux.open', create=True) as m:
+            self.assertRaises(
+                RuntimeError,
+                psutil._pslinux.boot_time)
+            assert m.called
+
     # --- tests for specific kernel versions
 
     @unittest.skipUnless(
diff --git a/test/test_memory_leaks.py b/test/test_memory_leaks.py
index 802e20ab..6f02dc0a 100644
--- a/test/test_memory_leaks.py
+++ b/test/test_memory_leaks.py
@@ -10,6 +10,7 @@ functions many times and compare process memory usage before and
 after the calls.  It might produce false positives.
 """
 
+import functools
 import gc
 import os
 import socket
@@ -20,7 +21,7 @@ import time
 import psutil
 import psutil._common
 
-from psutil._compat import xrange
+from psutil._compat import xrange, callable
 from test_psutil import (WINDOWS, POSIX, OSX, LINUX, SUNOS, BSD, TESTFN,
                          RLIMIT_SUPPORT, TRAVIS)
 from test_psutil import (reap_children, supports_ipv6, safe_remove,
@@ -92,7 +93,7 @@ class Base(unittest.TestCase):
     def get_mem(self):
         return psutil.Process().memory_info()[0]
 
-    def call(self, *args, **kwargs):
+    def call(self, function, *args, **kwargs):
         raise NotImplementedError("must be implemented in subclass")
 
 
@@ -106,15 +107,25 @@ class TestProcessObjectLeaks(Base):
         reap_children()
 
     def call(self, function, *args, **kwargs):
-        meth = getattr(self.proc, function)
-        if '_exc' in kwargs:
-            exc = kwargs.pop('_exc')
-            self.assertRaises(exc, meth, *args, **kwargs)
+        if callable(function):
+            if '_exc' in kwargs:
+                exc = kwargs.pop('_exc')
+                self.assertRaises(exc, function, *args, **kwargs)
+            else:
+                try:
+                    function(*args, **kwargs)
+                except psutil.Error:
+                    pass
         else:
-            try:
-                meth(*args, **kwargs)
-            except psutil.Error:
-                pass
+            meth = getattr(self.proc, function)
+            if '_exc' in kwargs:
+                exc = kwargs.pop('_exc')
+                self.assertRaises(exc, meth, *args, **kwargs)
+            else:
+                try:
+                    meth(*args, **kwargs)
+                except psutil.Error:
+                    pass
 
     @skip_if_linux()
     def test_name(self):
@@ -165,8 +176,10 @@ class TestProcessObjectLeaks(Base):
             value = psutil.Process().ionice()
             self.execute('ionice', value)
         else:
+            from psutil._pslinux import cext
             self.execute('ionice', psutil.IOPRIO_CLASS_NONE)
-            self.execute_w_exc(OSError, 'ionice', -1)
+            fun = functools.partial(cext.proc_ioprio_set, os.getpid(), -1, 0)
+            self.execute_w_exc(OSError, fun)
 
     @unittest.skipIf(OSX or SUNOS, "feature not supported on this platform")
     @skip_if_linux()
diff --git a/test/test_psutil.py b/test/test_psutil.py
index b93cd863..91423841 100644
--- a/test/test_psutil.py
+++ b/test/test_psutil.py
@@ -46,6 +46,10 @@ try:
     import ipaddress  # python >= 3.3
 except ImportError:
     ipaddress = None
+try:
+    from unittest import mock  # py3
+except ImportError:
+    import mock  # requires "pip install mock"
 
 import psutil
 from psutil._compat import PY3, callable, long, unicode
@@ -580,6 +584,7 @@ class TestSystemAPIs(unittest.TestCase):
         sproc3 = get_test_subprocess()
         procs = [psutil.Process(x.pid) for x in (sproc1, sproc2, sproc3)]
         self.assertRaises(ValueError, psutil.wait_procs, procs, timeout=-1)
+        self.assertRaises(TypeError, psutil.wait_procs, procs, callback=1)
         t = time.time()
         gone, alive = psutil.wait_procs(procs, timeout=0.01, callback=callback)
 
@@ -675,6 +680,8 @@ class TestSystemAPIs(unittest.TestCase):
         self.assertFalse(psutil.pid_exists(sproc.pid))
         self.assertFalse(psutil.pid_exists(-1))
         self.assertEqual(psutil.pid_exists(0), 0 in psutil.pids())
+        # pid 0
+        psutil.pid_exists(0) == 0 in psutil.pids()
 
     def test_pid_exists_2(self):
         reap_children()
@@ -1129,13 +1136,30 @@ class TestProcess(unittest.TestCase):
     def test_send_signal(self):
         sig = signal.SIGKILL if POSIX else signal.SIGTERM
         sproc = get_test_subprocess()
-        test_pid = sproc.pid
-        p = psutil.Process(test_pid)
+        p = psutil.Process(sproc.pid)
         p.send_signal(sig)
         exit_sig = p.wait()
-        self.assertFalse(psutil.pid_exists(test_pid))
+        self.assertFalse(psutil.pid_exists(p.pid))
         if POSIX:
             self.assertEqual(exit_sig, sig)
+            #
+            sproc = get_test_subprocess()
+            p = psutil.Process(sproc.pid)
+            p.send_signal(sig)
+            with mock.patch('psutil.os.kill',
+                            side_effect=OSError(errno.ESRCH, "")) as fun:
+                with self.assertRaises(psutil.NoSuchProcess):
+                    p.send_signal(sig)
+                assert fun.called
+            #
+            sproc = get_test_subprocess()
+            p = psutil.Process(sproc.pid)
+            p.send_signal(sig)
+            with mock.patch('psutil.os.kill',
+                            side_effect=OSError(errno.EPERM, "")) as fun:
+                with self.assertRaises(psutil.AccessDenied):
+                    p.send_signal(sig)
+                assert fun.called
 
     def test_wait(self):
         # check exit code signal
@@ -1354,7 +1378,20 @@ class TestProcess(unittest.TestCase):
                 ioclass, value = p.ionice()
                 self.assertEqual(ioclass, 2)
                 self.assertEqual(value, 7)
+                #
                 self.assertRaises(ValueError, p.ionice, 2, 10)
+                self.assertRaises(ValueError, p.ionice, 2, -1)
+                self.assertRaises(ValueError, p.ionice, 4)
+                self.assertRaises(TypeError, p.ionice, 2, "foo")
+                self.assertRaisesRegexp(
+                    ValueError, "can't specify value with IOPRIO_CLASS_NONE",
+                    p.ionice, psutil.IOPRIO_CLASS_NONE, 1)
+                self.assertRaisesRegexp(
+                    ValueError, "can't specify value with IOPRIO_CLASS_IDLE",
+                    p.ionice, psutil.IOPRIO_CLASS_IDLE, 1)
+                self.assertRaisesRegexp(
+                    ValueError, "'ioclass' argument must be specified",
+                    p.ionice, value=1)
             finally:
                 p.ionice(IOPRIO_CLASS_NONE)
         else:
@@ -1399,6 +1436,12 @@ class TestProcess(unittest.TestCase):
         p = psutil.Process(sproc.pid)
         p.rlimit(psutil.RLIMIT_NOFILE, (5, 5))
         self.assertEqual(p.rlimit(psutil.RLIMIT_NOFILE), (5, 5))
+        # If pid is 0 prlimit() applies to the calling process and
+        # we don't want that.
+        with self.assertRaises(ValueError):
+            psutil._psplatform.Process(0).rlimit(0)
+        with self.assertRaises(ValueError):
+            p.rlimit(psutil.RLIMIT_NOFILE, (5, 5, 5))
 
     def test_num_threads(self):
         # on certain platforms such as Linux we might test for exact
@@ -1639,6 +1682,11 @@ class TestProcess(unittest.TestCase):
         if POSIX:
             import pwd
             self.assertEqual(p.username(), pwd.getpwuid(os.getuid()).pw_name)
+            with mock.patch("psutil.pwd.getpwuid",
+                            side_effect=KeyError) as fun:
+                p.username() == str(p.uids().real)
+                assert fun.called
+
         elif WINDOWS and 'USERNAME' in os.environ:
             expected_username = os.environ['USERNAME']
             expected_domain = os.environ['USERDOMAIN']
@@ -1703,6 +1751,7 @@ class TestProcess(unittest.TestCase):
         files = p.open_files()
         self.assertFalse(TESTFN in files)
         with open(TESTFN, 'w'):
+            # give the kernel some time to see the new file
             call_until(p.open_files, "len(ret) != %i" % len(files))
             filenames = [x.path for x in p.open_files()]
             self.assertIn(TESTFN, filenames)
@@ -2184,6 +2233,10 @@ class TestProcess(unittest.TestCase):
             except psutil.AccessDenied:
                 pass
 
+            self.assertRaisesRegexp(
+                ValueError, "preventing sending signal to process with PID 0",
+                p.send_signal(signal.SIGTERM))
+
         self.assertIn(p.ppid(), (0, 1))
         # self.assertEqual(p.exe(), "")
         p.cmdline()
@@ -2197,7 +2250,6 @@ class TestProcess(unittest.TestCase):
         except psutil.AccessDenied:
             pass
 
-        # username property
         try:
             if POSIX:
                 self.assertEqual(p.username(), 'root')
@@ -2224,6 +2276,7 @@ class TestProcess(unittest.TestCase):
             proc.stdin
             self.assertTrue(hasattr(proc, 'name'))
             self.assertTrue(hasattr(proc, 'stdin'))
+            self.assertTrue(dir(proc))
             self.assertRaises(AttributeError, getattr, proc, 'foo')
         finally:
             proc.kill()
@@ -2571,6 +2624,19 @@ class TestMisc(unittest.TestCase):
         p.wait()
         self.assertIn(str(sproc.pid), str(p))
         self.assertIn("terminated", str(p))
+        # test error conditions
+        with mock.patch.object(psutil.Process, 'name',
+                               side_effect=psutil.ZombieProcess(1)) as meth:
+            self.assertIn("zombie", str(p))
+            self.assertIn("pid", str(p))
+            assert meth.called
+        with mock.patch.object(psutil.Process, 'name',
+                               side_effect=psutil.AccessDenied) as meth:
+            self.assertIn("pid", str(p))
+            assert meth.called
+
+    def test__repr__(self):
+        repr(psutil.Process())
 
     def test__eq__(self):
         p1 = psutil.Process()
@@ -2667,6 +2733,23 @@ class TestMisc(unittest.TestCase):
         module = imp.load_source('setup', setup_py)
         self.assertRaises(SystemExit, module.setup)
 
+    def test_ad_on_process_creation(self):
+        # We are supposed to be able to instantiate Process also in case
+        # of zombie processes or access denied.
+        with mock.patch.object(psutil.Process, 'create_time',
+                               side_effect=psutil.AccessDenied) as meth:
+            psutil.Process()
+            assert meth.called
+        with mock.patch.object(psutil.Process, 'create_time',
+                               side_effect=psutil.ZombieProcess(1)) as meth:
+            psutil.Process()
+            assert meth.called
+        with mock.patch.object(psutil.Process, 'create_time',
+                               side_effect=ValueError) as meth:
+            with self.assertRaises(ValueError):
+                psutil.Process()
+            assert meth.called
+
 
 # ===================================================================
 # --- Example script tests