diff options
author | Giampaolo Rodola <g.rodola@gmail.com> | 2020-05-01 02:20:52 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-05-01 11:20:52 +0200 |
commit | 8d8a7804d159e5b80378000b57bbfbaf63ce6e8f (patch) | |
tree | f8a0b45d618ef69faca670bdde8077bb0c20d427 | |
parent | 3480e1b05f3e98744a1b6aa6fe286caac86e6bbd (diff) | |
download | psutil-8d8a7804d159e5b80378000b57bbfbaf63ce6e8f.tar.gz |
Revert #1736: Popen inheriting from subprocess (#1744)
-rw-r--r-- | HISTORY.rst | 4 | ||||
-rw-r--r-- | docs/index.rst | 47 | ||||
-rw-r--r-- | psutil/__init__.py | 69 | ||||
-rw-r--r-- | psutil/tests/__init__.py | 21 | ||||
-rwxr-xr-x | psutil/tests/runner.py | 40 |
5 files changed, 69 insertions, 112 deletions
diff --git a/HISTORY.rst b/HISTORY.rst index c375934c..6f8087db 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -7,9 +7,7 @@ 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. +- 1729_: parallel tests on UNIX (make test-parallel). They're twice as fast! - 1741_: "make build/install" is now run in parallel and it's about 15% faster on UNIX. diff --git a/docs/index.rst b/docs/index.rst index 08a69555..133e69fe 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -1959,24 +1959,18 @@ Process class >>> p.terminate() >>> p.wait() -Popen class ------------ - .. class:: Popen(*args, **kwargs) - 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 as a unified interface. - - .. note:: - - Unlike `subprocess.Popen`_ this class preemptively checks whether PID has - been reused on - :meth:`send_signal() <psutil.Process.send_signal()>`, - :meth:`terminate() <psutil.Process.terminate()>` and - :meth:`kill() <psutil.Process.kill()>` - so that you can't accidentally terminate another process, fixing `BPO-6973`_. + Starts a sub-process via `subprocess.Popen`_, and in addition it provides + all the methods of :class:`psutil.Process` in a single class. + For method names common to both classes such as + :meth:`send_signal() <psutil.Process.send_signal()>`, + :meth:`terminate() <psutil.Process.terminate()>`, + :meth:`kill() <psutil.Process.kill()>` and + :meth:`wait() <psutil.Process.wait()>` + :class:`psutil.Process` implementation takes precedence. + This may have some advantages, like making sure PID has not been reused, + fixing `BPO-6973`_. >>> import psutil >>> from subprocess import PIPE @@ -1992,25 +1986,10 @@ 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 (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:: 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. + .. versionchanged:: 4.4.0 added context manager support + .. versionchanged:: 5.7.1 wait() invokes :meth:`wait() <psutil.Process.wait()>` + instead of `subprocess.Popen.wait`_. Windows services ================ diff --git a/psutil/__init__.py b/psutil/__init__.py index 028ab049..acf5ee79 100644 --- a/psutil/__init__.py +++ b/psutil/__init__.py @@ -1289,12 +1289,12 @@ _as_dict_attrnames = set( # ===================================================================== -class Popen(subprocess.Popen): +class Popen(Process): """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 it also provides all the - methods of psutil.Process class as a unified interface: - + subprocess.Popen class but in addition also provides all the + properties and methods of psutil.Process class as a unified + interface: >>> import psutil >>> from subprocess import PIPE >>> p = psutil.Popen(["python", "-c", "print 'hi'"], stdout=PIPE) @@ -1310,16 +1310,12 @@ class Popen(subprocess.Popen): >>> p.wait(timeout=2) 0 >>> - - In addition, it backports the following functionality: - * "with" statement (Python 3.2) - * wait(timeout=...) parameter (Python 3.3) - + For method names common to both classes such as kill(), terminate() + and wait(), psutil.Process implementation takes precedence. 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. - For a complete documentation refer to: http://docs.python.org/3/library/subprocess.html """ @@ -1328,21 +1324,21 @@ class Popen(subprocess.Popen): # 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.__psproc = None - subprocess.Popen.__init__(self, *args, **kwargs) - self.__psproc = Process(self.pid) - self.__psproc._init(self.pid, _ignore_nsp=True) + self.__subproc = subprocess.Popen(*args, **kwargs) + self._init(self.__subproc.pid, _ignore_nsp=True) def __dir__(self): - return sorted(set(dir(subprocess.Popen) + dir(Process))) + return sorted(set(dir(Popen) + dir(subprocess.Popen))) - # Introduced in Python 3.2. - if not hasattr(subprocess.Popen, '__enter__'): + def __enter__(self): + if hasattr(self.__subproc, '__enter__'): + self.__subproc.__enter__() + return self - def __enter__(self): - return self - - def __exit__(self, *args, **kwargs): + def __exit__(self, *args, **kwargs): + if hasattr(self.__subproc, '__exit__'): + return self.__subproc.__exit__(*args, **kwargs) + else: if self.stdout: self.stdout.close() if self.stderr: @@ -1360,30 +1356,21 @@ class Popen(subprocess.Popen): return object.__getattribute__(self, name) except AttributeError: try: - return object.__getattribute__(self.__psproc, name) + return object.__getattribute__(self.__subproc, name) except AttributeError: raise AttributeError("%s instance has no attribute '%s'" % (self.__class__.__name__, name)) - def send_signal(self, sig): - return self.__psproc.send_signal(sig) - - def terminate(self): - return self.__psproc.terminate() - - 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) + if self.__subproc.returncode is not None: + return self.__subproc.returncode + # Note: using psutil's wait() on UNIX should make no difference. + # On Windows it does, because PID can still be alive (see + # _pswindows.py counterpart addressing this). Python 2.7 doesn't + # have timeout arg, so this acts as a backport. + ret = Process.wait(self, timeout) + self.__subproc.returncode = ret + return ret # ===================================================================== diff --git a/psutil/tests/__init__.py b/psutil/tests/__init__.py index 8ce76304..6cdf3bc8 100644 --- a/psutil/tests/__init__.py +++ b/psutil/tests/__init__.py @@ -486,21 +486,12 @@ def terminate(proc_or_pid, sig=signal.SIGTERM, wait_timeout=GLOBAL_TIMEOUT): from psutil._psposix import wait_pid def wait(proc, timeout): - if sys.version_info < (3, 3) and \ - isinstance(proc, subprocess.Popen) and \ - not isinstance(proc, psutil.Popen): - # subprocess.Popen instance: emulate missing timeout arg. - ret = None - try: - ret = psutil.Process(proc.pid).wait(timeout) - except psutil.NoSuchProcess: - # Needed to kill zombies. - if POSIX: - ret = wait_pid(proc.pid, timeout) - proc.returncode = ret - return ret - else: - return proc.wait(timeout) + try: + return psutil.Process(proc.pid).wait(timeout) + except psutil.NoSuchProcess: + # Needed to kill zombies. + if POSIX: + return wait_pid(proc.pid, timeout) def term_subproc(proc, timeout): try: diff --git a/psutil/tests/runner.py b/psutil/tests/runner.py index ef135c8d..a7f74964 100755 --- a/psutil/tests/runner.py +++ b/psutil/tests/runner.py @@ -53,11 +53,21 @@ from psutil.tests import TOX VERBOSITY = 1 if TOX else 2 FAILED_TESTS_FNAME = '.failed-tests.txt' NWORKERS = psutil.cpu_count() or 1 +USE_COLORS = term_supports_colors() and not APPVEYOR HERE = os.path.abspath(os.path.dirname(__file__)) loadTestsFromTestCase = unittest.defaultTestLoader.loadTestsFromTestCase +def cprint(msg, color, bold=False, file=None): + if file is None: + file = sys.stderr if color == 'red' else sys.stdout + if USE_COLORS: + print_color(msg, color, bold=bold, file=file) + else: + print(msg, file=file) + + class TestLoader: testdir = HERE @@ -110,24 +120,21 @@ class TestLoader: class ColouredResult(unittest.TextTestResult): - def _print_color(self, s, color, bold=False): - print_color(s, color, bold=bold, file=self.stream) - def addSuccess(self, test): unittest.TestResult.addSuccess(self, test) - self._print_color("OK", "green") + cprint("OK", "green") def addError(self, test, err): unittest.TestResult.addError(self, test, err) - self._print_color("ERROR", "red", bold=True) + cprint("ERROR", "red", bold=True) def addFailure(self, test, err): unittest.TestResult.addFailure(self, test, err) - self._print_color("FAIL", "red") + cprint("FAIL", "red") def addSkip(self, test, reason): unittest.TestResult.addSkip(self, test, reason) - self._print_color("skipped: %s" % reason.strip(), "brown") + cprint("skipped: %s" % reason.strip(), "brown") def printErrorList(self, flavour, errors): flavour = hilite(flavour, "red", bold=flavour == 'ERROR') @@ -139,11 +146,7 @@ class ColouredTextRunner(unittest.TextTestRunner): A coloured text runner which also prints failed tests on KeyboardInterrupt and save failed tests in a file so that they can be re-run. """ - - if term_supports_colors() and not APPVEYOR: - resultclass = ColouredResult - else: - resultclass = unittest.TextTestResult + resultclass = ColouredResult if USE_COLORS else unittest.TextTestResult def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -180,11 +183,11 @@ class ColouredTextRunner(unittest.TextTestRunner): def _exit(self, success): if success: - print_color("SUCCESS", "green", bold=True) + cprint("SUCCESS", "green", bold=True) safe_rmpath(FAILED_TESTS_FNAME) sys.exit(0) else: - print_color("FAILED", "red", bold=True) + cprint("FAILED", "red", bold=True) self._write_last_failed() sys.exit(1) @@ -228,8 +231,8 @@ class ParallelRunner(ColouredTextRunner): par_suite = self._parallelize(par_suite) # run parallel - print_color("starting parallel tests using %s workers" % NWORKERS, - "green", bold=True) + cprint("starting parallel tests using %s workers" % NWORKERS, + "green", bold=True) t = time.time() par = self._run(par_suite) par_elapsed = time.time() - t @@ -239,7 +242,7 @@ class ParallelRunner(ColouredTextRunner): orphans = psutil.Process().children() gone, alive = psutil.wait_procs(orphans, timeout=1) if alive: - print_color("alive processes %s" % alive, "red") + cprint("alive processes %s" % alive, "red") reap_children() # run serial @@ -274,8 +277,7 @@ class ParallelRunner(ColouredTextRunner): def get_runner(parallel=False): def warn(msg): - print_color(msg + " Running serial tests instead.", - "red", file=sys.stderr) + cprint(msg + " Running serial tests instead.", "red") if parallel: if psutil.WINDOWS: warn("Can't run parallel tests on Windows.") |