psutil.Popen: inherit from subprocess + support wait(timeout=...) parameter (#1736)

8 files changed, 99 insertions, 60 deletions

diff --git a/HISTORY.rst b/HISTORY.rst
index d68e68fc..7b992a8c 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -8,6 +8,8 @@ XXXX-XX-XX
 **Enhancements**
 
 - 1729_: parallel tests on UNIX (make test-parallel).
+- 1736_: psutil.Popen now inherits from subprocess.Popen instead of
+  psutil.Process. Also, wait(timeout=...) parameter is backported to Python 2.7.
 
 **Bug fixes**
 
diff --git a/Makefile b/Makefile
index 76420bc7..1fad22f5 100644
--- a/Makefile
+++ b/Makefile
@@ -159,7 +159,7 @@ test-memleaks:  ## Memory leak tests.
 
 test-by-name:  ## e.g. make test-by-name ARGS=psutil.tests.test_system.TestSystemAPIs
 	${MAKE} install
-	@$(TEST_PREFIX) $(PYTHON) $(TSCRIPT) $(ARGS)
+	$(TEST_PREFIX) $(PYTHON) $(TSCRIPT) $(ARGS)
 
 test-failed:  ## Re-run tests which failed on last run
 	${MAKE} install
diff --git a/docs/index.rst b/docs/index.rst
index 55e1586d..08a69555 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -1966,15 +1966,8 @@ Popen class
 
   A more convenient interface to stdlib `subprocess.Popen`_.
   It starts a sub process and you deal with it exactly as when using
-  `subprocess.Popen`_.
-  but in addition it also provides all the methods of :class:`psutil.Process`
-  class.
-  For method names common to both classes such as
-  :meth:`send_signal() <psutil.Process.send_signal()>`,
-  :meth:`terminate() <psutil.Process.terminate()>` and
-  :meth:`kill() <psutil.Process.kill()>`
-  :class:`psutil.Process` implementation takes precedence.
-  For a complete documentation refer to subprocess module documentation.
+  `subprocess.Popen`_, but in addition it also provides all the methods of
+  :class:`psutil.Process` class as a unified interface.
 
   .. note::
 
@@ -1999,16 +1992,25 @@ Popen class
   0
   >>>
 
+  *timeout* parameter of `subprocess.Popen.wait`_  is backported for Python < 3.3.
   :class:`psutil.Popen` objects are supported as context managers via the with
-  statement: on exit, standard file descriptors are closed, and the process
-  is waited for. This is supported on all Python versions.
+  statement (added to Python 3.2). On exit, standard file descriptors are
+  closed, and the process is waited for. This is supported on all Python
+  versions.
 
   >>> import psutil, subprocess
   >>> with psutil.Popen(["ifconfig"], stdout=subprocess.PIPE) as proc:
   >>>     log.write(proc.stdout.read())
 
 
-  .. versionchanged:: 4.4.0 added context manager support
+  .. versionchanged:: 4.4.0 added context manager support.
+
+  .. versionchanged:: 5.7.1 inherit from `subprocess.Popen`_ instead of
+    :class:`psutil.Process`.
+
+  .. versionchanged:: 5.7.1 backporint `subprocess.Popen.wait`_ **timeout**
+    parameter on old Python versions.
+
 
 Windows services
 ================
@@ -2882,6 +2884,7 @@ Timeline
 .. _`SOCK_STREAM`: https://docs.python.org/3/library/socket.html#socket.SOCK_STREAM
 .. _`socket.fromfd`: https://docs.python.org/3/library/socket.html#socket.fromfd
 .. _`subprocess.Popen`: https://docs.python.org/3/library/subprocess.html#subprocess.Popen
+.. _`subprocess.Popen.wait`: https://docs.python.org/3/library/subprocess.html#subprocess.Popen.wait
 .. _`temperatures.py`: https://github.com/giampaolo/psutil/blob/master/scripts/temperatures.py
 .. _`TerminateProcess`: https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-terminateprocess
 .. _Tidelift security contact: https://tidelift.com/security
diff --git a/psutil/__init__.py b/psutil/__init__.py
index e6a2da8d..650fcf0f 100644
--- a/psutil/__init__.py
+++ b/psutil/__init__.py
@@ -1274,17 +1274,24 @@ class Process(object):
         return self._proc.wait(timeout)
 
 
+# The valid attr names which can be processed by Process.as_dict().
+_as_dict_attrnames = set(
+    [x for x in dir(Process) if not x.startswith('_') and x not in
+     ['send_signal', 'suspend', 'resume', 'terminate', 'kill', 'wait',
+      'is_running', 'as_dict', 'parent', 'parents', 'children', 'rlimit',
+      'memory_info_ex', 'oneshot']])
+
+
 # =====================================================================
 # --- Popen class
 # =====================================================================
 
 
-class Popen(Process):
+class Popen(subprocess.Popen):
     """A more convenient interface to stdlib subprocess.Popen class.
     It starts a sub process and deals with it exactly as when using
-    subprocess.Popen class but in addition also provides all the
-    properties and methods of psutil.Process class as a unified
-    interface:
+    subprocess.Popen class, but in addition it also provides all the
+    methods of psutil.Process class as a unified interface:
 
       >>> import psutil
       >>> from subprocess import PIPE
@@ -1302,11 +1309,12 @@ class Popen(Process):
       0
       >>>
 
-    For method names common to both classes such as kill(), terminate()
-    and wait(), psutil.Process implementation takes precedence.
+    In addition, it backports the following functionality:
+    * "with" statement (Python 3.2)
+    * wait(timeout=...) parameter (Python 3.3)
 
     Unlike subprocess.Popen this class pre-emptively checks whether PID
-    has been reused on send_signal(), terminate() and kill() so that
+    has been reused on send_signal(), terminate() and kill(), so that
     you don't accidentally terminate another process, fixing
     http://bugs.python.org/issue6973.
 
@@ -1318,21 +1326,21 @@ class Popen(Process):
         # Explicitly avoid to raise NoSuchProcess in case the process
         # spawned by subprocess.Popen terminates too quickly, see:
         # https://github.com/giampaolo/psutil/issues/193
-        self.__subproc = subprocess.Popen(*args, **kwargs)
-        self._init(self.__subproc.pid, _ignore_nsp=True)
+        self.__psproc = None
+        subprocess.Popen.__init__(self, *args, **kwargs)
+        self.__psproc = Process(self.pid)
+        self.__psproc._init(self.pid, _ignore_nsp=True)
 
     def __dir__(self):
-        return sorted(set(dir(Popen) + dir(subprocess.Popen)))
+        return sorted(set(dir(subprocess.Popen) + dir(Process)))
 
-    def __enter__(self):
-        if hasattr(self.__subproc, '__enter__'):
-            self.__subproc.__enter__()
-        return self
+    # Introduced in Python 3.2.
+    if not hasattr(subprocess.Popen, '__enter__'):
 
-    def __exit__(self, *args, **kwargs):
-        if hasattr(self.__subproc, '__exit__'):
-            return self.__subproc.__exit__(*args, **kwargs)
-        else:
+        def __enter__(self):
+            return self
+
+        def __exit__(self, *args, **kwargs):
             if self.stdout:
                 self.stdout.close()
             if self.stderr:
@@ -1350,25 +1358,30 @@ class Popen(Process):
             return object.__getattribute__(self, name)
         except AttributeError:
             try:
-                return object.__getattribute__(self.__subproc, name)
+                return object.__getattribute__(self.__psproc, name)
             except AttributeError:
                 raise AttributeError("%s instance has no attribute '%s'"
                                      % (self.__class__.__name__, name))
 
-    def wait(self, timeout=None):
-        if self.__subproc.returncode is not None:
-            return self.__subproc.returncode
-        ret = super(Popen, self).wait(timeout)
-        self.__subproc.returncode = ret
-        return ret
+    def send_signal(self, sig):
+        return self.__psproc.send_signal(sig)
 
+    def terminate(self):
+        return self.__psproc.terminate()
 
-# The valid attr names which can be processed by Process.as_dict().
-_as_dict_attrnames = set(
-    [x for x in dir(Process) if not x.startswith('_') and x not in
-     ['send_signal', 'suspend', 'resume', 'terminate', 'kill', 'wait',
-      'is_running', 'as_dict', 'parent', 'parents', 'children', 'rlimit',
-      'memory_info_ex', 'oneshot']])
+    def kill(self):
+        return self.__psproc.kill()
+
+    def wait(self, timeout=None):
+        if sys.version_info < (3, 3):
+            # backport of timeout parameter
+            if self.returncode is not None:
+                return self.returncode
+            ret = self.__psproc.wait(timeout)
+            self.returncode = ret
+            return ret
+        else:
+            return super(Popen, self).wait(timeout)
 
 
 # =====================================================================
diff --git a/psutil/tests/__init__.py b/psutil/tests/__init__.py
index bf3d973f..b31b845d 100644
--- a/psutil/tests/__init__.py
+++ b/psutil/tests/__init__.py
@@ -459,6 +459,20 @@ def terminate(proc_or_pid, sig=signal.SIGTERM, wait_w_timeout=GLOBAL_TIMEOUT):
     """Terminate and flush a psutil.Process, psutil.Popen or
     subprocess.Popen instance.
     """
+    def wait(proc, timeout=None):
+        if sys.version_info < (3, 3) and not \
+                isinstance(proc, (psutil.Process, psutil.Popen)):
+            # subprocess.Popen instance + no timeout arg.
+            try:
+                ret = psutil.Process(proc.pid).wait(timeout)
+            except psutil.NoSuchProcess:
+                pass
+            else:
+                proc.returncode = ret
+                return ret
+        else:
+            return proc.wait(timeout)
+
     if isinstance(proc_or_pid, int):
         try:
             proc = psutil.Process(proc_or_pid)
@@ -467,7 +481,18 @@ def terminate(proc_or_pid, sig=signal.SIGTERM, wait_w_timeout=GLOBAL_TIMEOUT):
     else:
         proc = proc_or_pid
 
-    if isinstance(proc, subprocess.Popen):
+    if isinstance(proc, (psutil.Process, psutil.Popen)):
+        try:
+            proc.send_signal(sig)
+        except psutil.NoSuchProcess:
+            _assert_no_pid(proc.pid)
+        else:
+            if wait_w_timeout:
+                ret = wait(proc, wait_w_timeout)
+                _assert_no_pid(proc.pid)
+                return ret
+    else:
+        # subprocess.Popen instance
         try:
             proc.send_signal(sig)
         except OSError as err:
@@ -475,6 +500,8 @@ def terminate(proc_or_pid, sig=signal.SIGTERM, wait_w_timeout=GLOBAL_TIMEOUT):
                 pass
             elif err.errno != errno.ESRCH:
                 raise
+        except psutil.NoSuchProcess:  # psutil.Popen
+            pass
         if proc.stdout:
             proc.stdout.close()
         if proc.stderr:
@@ -486,17 +513,9 @@ def terminate(proc_or_pid, sig=signal.SIGTERM, wait_w_timeout=GLOBAL_TIMEOUT):
         finally:
             if wait_w_timeout:
                 try:
-                    proc.wait(wait_w_timeout)
+                    return wait(proc, wait_w_timeout)
                 except ChildProcessError:
                     pass
-    else:
-        try:
-            proc.send_signal(sig)
-        except psutil.NoSuchProcess:
-            _assert_no_pid(proc.pid)
-        else:
-            if wait_w_timeout:
-                proc.wait(wait_w_timeout)
                 _assert_no_pid(proc.pid)
 
 
diff --git a/psutil/tests/runner.py b/psutil/tests/runner.py
index bfadc069..d90feabd 100755
--- a/psutil/tests/runner.py
+++ b/psutil/tests/runner.py
@@ -155,7 +155,8 @@ class SuiteLoader:
 
     def from_name(self, name):
         suite = unittest.TestSuite()
-        name = os.path.splitext(os.path.basename(name))[0]
+        if name.endswith('.py'):
+            name = os.path.splitext(os.path.basename(name))[0]
         suite.addTest(unittest.defaultTestLoader.loadTestsFromName(name))
         return suite
 
diff --git a/psutil/tests/test_linux.py b/psutil/tests/test_linux.py
index ea957c17..6d4a934a 100755
--- a/psutil/tests/test_linux.py
+++ b/psutil/tests/test_linux.py
@@ -33,16 +33,16 @@ from psutil.tests import HAS_BATTERY
 from psutil.tests import HAS_CPU_FREQ
 from psutil.tests import HAS_GETLOADAVG
 from psutil.tests import HAS_RLIMIT
-from psutil.tests import SYSMEM_TOLERANCE
 from psutil.tests import mock
 from psutil.tests import PYPY
 from psutil.tests import pyrun
-from psutil.tests import reap_children
 from psutil.tests import reload_module
 from psutil.tests import retry_on_failure
 from psutil.tests import safe_rmpath
 from psutil.tests import sh
 from psutil.tests import skip_on_not_implemented
+from psutil.tests import SYSMEM_TOLERANCE
+from psutil.tests import terminate
 from psutil.tests import ThreadTask
 from psutil.tests import TRAVIS
 from psutil.tests import unittest
@@ -1652,7 +1652,7 @@ class TestProcess(unittest.TestCase):
                 time.sleep(10)
             """ % testfn)
         sproc = pyrun(src)
-        self.addCleanup(reap_children)
+        self.addCleanup(terminate, sproc)
         call_until(lambda: os.listdir('.'), "'%s' not in ret" % testfn)
         p = psutil.Process(sproc.pid)
         time.sleep(.1)
diff --git a/psutil/tests/test_process.py b/psutil/tests/test_process.py
index 8095fc04..ac841eee 100755
--- a/psutil/tests/test_process.py
+++ b/psutil/tests/test_process.py
@@ -1338,7 +1338,7 @@ class TestProcess(unittest.TestCase):
                 pass
 
         zpid = create_zombie_proc()
-        self.addCleanup(reap_children, recursive=True)
+        self.addCleanup(reap_children)
         # A zombie process should always be instantiable
         zproc = psutil.Process(zpid)
         # ...and at least its status always be querable
@@ -1348,7 +1348,7 @@ class TestProcess(unittest.TestCase):
         # ...and as_dict() shouldn't crash
         zproc.as_dict()
         # if cmdline succeeds it should be an empty list
-        ret = succeed_or_zombie_p_exc(zproc.suspend)
+        ret = succeed_or_zombie_p_exc(zproc.cmdline)
         if ret is not None:
             self.assertEqual(ret, [])
 
@@ -1592,6 +1592,7 @@ class TestPopen(unittest.TestCase):
             self.assertTrue(dir(proc))
             self.assertRaises(AttributeError, getattr, proc, 'foo')
             proc.terminate()
+        proc.wait(timeout=3)
 
     def test_ctx_manager(self):
         with psutil.Popen([PYTHON_EXE, "-V"],