From b9179ff8576d5d2aac336073e1118bed4f5ef009 Mon Sep 17 00:00:00 2001 From: David Tenty Date: Wed, 24 Jul 2019 15:04:27 +0000 Subject: [PATCH] [AIX][lit] Don't depend on psutil on AIX Summary: On AIX psutil can run into problems with permissions to read the process tree, which causes problems for python timeout tests which need to kill off a test and it's children. This patch adds a workaround by invoking shell via subprocess and using a platform specific option to ps to list all the descendant processes so we can kill them. We add some checks so lit can tell whether timeout tests are supported with out exposing whether we are utilizing the psutil implementation or the alternative. Reviewers: hubert.reinterpretcast, andusy, davide, delcypher Reviewed By: delcypher Subscribers: davide, delcypher, christof, lldb-commits, libcxx-commits, llvm-commits Tags: #lldb, #libc, #llvm Differential Revision: https://reviews.llvm.org/D64251 llvm-svn: 366912 --- libcxx/utils/libcxx/util.py | 35 ++++++------ lldb/lit/lit.cfg.py | 14 ++--- llvm/utils/lit/lit/LitConfig.py | 27 +++++++--- llvm/utils/lit/lit/util.py | 62 +++++++++++++++------- llvm/utils/lit/tests/googletest-timeout.py | 2 +- llvm/utils/lit/tests/lit.cfg | 15 +++--- llvm/utils/lit/tests/shtest-timeout.py | 2 +- 7 files changed, 98 insertions(+), 59 deletions(-) diff --git a/libcxx/utils/libcxx/util.py b/libcxx/utils/libcxx/util.py index 46c09bf66693..2fd95232abb4 100644 --- a/libcxx/utils/libcxx/util.py +++ b/libcxx/utils/libcxx/util.py @@ -253,24 +253,27 @@ def killProcessAndChildren(pid): TODO: Reimplement this without using psutil so we can remove our dependency on it. """ - import psutil - try: - psutilProc = psutil.Process(pid) - # Handle the different psutil API versions + if platform.system() == 'AIX': + subprocess.call('kill -kill $(ps -o pid= -L{})'.format(pid), shell=True) + else: + import psutil try: - # psutil >= 2.x - children_iterator = psutilProc.children(recursive=True) - except AttributeError: - # psutil 1.x - children_iterator = psutilProc.get_children(recursive=True) - for child in children_iterator: + psutilProc = psutil.Process(pid) + # Handle the different psutil API versions try: - child.kill() - except psutil.NoSuchProcess: - pass - psutilProc.kill() - except psutil.NoSuchProcess: - pass + # psutil >= 2.x + children_iterator = psutilProc.children(recursive=True) + except AttributeError: + # psutil 1.x + children_iterator = psutilProc.get_children(recursive=True) + for child in children_iterator: + try: + child.kill() + except psutil.NoSuchProcess: + pass + psutilProc.kill() + except psutil.NoSuchProcess: + pass def executeCommandVerbose(cmd, *args, **kwargs): diff --git a/lldb/lit/lit.cfg.py b/lldb/lit/lit.cfg.py index ad659eaf87f7..b56e41e720b5 100644 --- a/lldb/lit/lit.cfg.py +++ b/lldb/lit/lit.cfg.py @@ -1,6 +1,7 @@ # -*- Python -*- import os +import platform import re import shutil import site @@ -75,13 +76,14 @@ for i in ['module-cache-clang', 'module-cache-lldb']: shutil.rmtree(cachedir) # Set a default per-test timeout of 10 minutes. Setting a timeout per test -# requires the psutil module and lit complains if the value is set but the -# module can't be found. -try: - import psutil # noqa: F401 +# requires that killProcessAndChildren() is supported on the platform and +# lit complains if the value is set but it is not supported. +supported, errormsg = lit_config.maxIndividualTestTimeIsSupported +if supported: lit_config.maxIndividualTestTime = 600 -except ImportError: - pass +else: + lit_config.warning("Could not set a default per-test timeout. " + errormsg) + # If running tests natively, check for CPU features needed for some tests. diff --git a/llvm/utils/lit/lit/LitConfig.py b/llvm/utils/lit/lit/LitConfig.py index 31eeb76288b2..35ff0590bd6b 100644 --- a/llvm/utils/lit/lit/LitConfig.py +++ b/llvm/utils/lit/lit/LitConfig.py @@ -1,6 +1,7 @@ from __future__ import absolute_import import inspect import os +import platform import sys import lit.Test @@ -76,6 +77,19 @@ class LitConfig(object): """ return self._maxIndividualTestTime + @property + def maxIndividualTestTimeIsSupported(self): + """ + Returns a tuple ( , ) + where + `` is True if setting maxIndividualTestTime is supported + on the current host, returns False otherwise. + `` is an empty string if `` is True, + otherwise is contains a string describing why setting + maxIndividualTestTime is not supported. + """ + return lit.util.killProcessAndChildrenIsSupported() + @maxIndividualTestTime.setter def maxIndividualTestTime(self, value): """ @@ -86,16 +100,13 @@ class LitConfig(object): self.fatal('maxIndividualTestTime must set to a value of type int.') self._maxIndividualTestTime = value if self.maxIndividualTestTime > 0: - # The current implementation needs psutil to set + # The current implementation needs psutil on some platforms to set # a timeout per test. Check it's available. # See lit.util.killProcessAndChildren() - try: - import psutil # noqa: F401 - except ImportError: - self.fatal("Setting a timeout per test requires the" - " Python psutil module but it could not be" - " found. Try installing it via pip or via" - " your operating system's package manager.") + supported, errormsg = self.maxIndividualTestTimeIsSupported + if not supported: + self.fatal('Setting a timeout per test not supported. ' + + errormsg) elif self.maxIndividualTestTime < 0: self.fatal('The timeout per test must be >= 0 seconds') diff --git a/llvm/utils/lit/lit/util.py b/llvm/utils/lit/lit/util.py index 2a61ef7736a0..7eb5eaaa2ffa 100644 --- a/llvm/utils/lit/lit/util.py +++ b/llvm/utils/lit/lit/util.py @@ -423,34 +423,56 @@ def findPlatformSdkVersionOnMacOS(config, lit_config): return out.decode() return None +def killProcessAndChildrenIsSupported(): + """ + Returns a tuple ( , ) + where + `` is True if `killProcessAndChildren()` is supported on + the current host, returns False otherwise. + `` is an empty string if `` is True, + otherwise is contains a string describing why the function is + not supported. + """ + if platform.system() == 'AIX': + return (True, "") + try: + import psutil # noqa: F401 + return (True, "") + except ImportError: + return (False, "Requires the Python psutil module but it could" + " not be found. Try installing it via pip or via" + " your operating system's package manager.") def killProcessAndChildren(pid): """This function kills a process with ``pid`` and all its running children - (recursively). It is currently implemented using the psutil module which - provides a simple platform neutral implementation. + (recursively). It is currently implemented using the psutil module on some + platforms which provides a simple platform neutral implementation. - TODO: Reimplement this without using psutil so we can remove - our dependency on it. + TODO: Reimplement this without using psutil on all platforms so we can + remove our dependency on it. """ - import psutil - try: - psutilProc = psutil.Process(pid) - # Handle the different psutil API versions + if platform.system() == 'AIX': + subprocess.call('kill -kill $(ps -o pid= -L{})'.format(pid), shell=True) + else: + import psutil try: - # psutil >= 2.x - children_iterator = psutilProc.children(recursive=True) - except AttributeError: - # psutil 1.x - children_iterator = psutilProc.get_children(recursive=True) - for child in children_iterator: + psutilProc = psutil.Process(pid) + # Handle the different psutil API versions try: - child.kill() - except psutil.NoSuchProcess: - pass - psutilProc.kill() - except psutil.NoSuchProcess: - pass + # psutil >= 2.x + children_iterator = psutilProc.children(recursive=True) + except AttributeError: + # psutil 1.x + children_iterator = psutilProc.get_children(recursive=True) + for child in children_iterator: + try: + child.kill() + except psutil.NoSuchProcess: + pass + psutilProc.kill() + except psutil.NoSuchProcess: + pass try: diff --git a/llvm/utils/lit/tests/googletest-timeout.py b/llvm/utils/lit/tests/googletest-timeout.py index 8b7d10fc1f03..0761ad0796e2 100644 --- a/llvm/utils/lit/tests/googletest-timeout.py +++ b/llvm/utils/lit/tests/googletest-timeout.py @@ -1,4 +1,4 @@ -# REQUIRES: python-psutil +# REQUIRES: lit-max-individual-test-time # Check that the per test timeout is enforced when running GTest tests. # diff --git a/llvm/utils/lit/tests/lit.cfg b/llvm/utils/lit/tests/lit.cfg index 4648b1bfc9c3..ebdcb5000b9a 100644 --- a/llvm/utils/lit/tests/lit.cfg +++ b/llvm/utils/lit/tests/lit.cfg @@ -1,6 +1,7 @@ # -*- Python -*- import os +import platform import sys import lit.formats @@ -56,10 +57,10 @@ if lit_config.params.get('check-coverage', None): os.path.dirname(__file__), ".coveragerc") # Add a feature to detect if psutil is available -try: - import psutil - lit_config.note('Found python psutil module') - config.available_features.add("python-psutil") -except ImportError: - lit_config.warning('Could not import psutil. Some tests will be skipped and' - ' the --timeout command line argument will not work.') +supported, errormsg = lit_config.maxIndividualTestTimeIsSupported +if supported: + config.available_features.add("lit-max-individual-test-time") +else: + lit_config.warning('Setting a timeout per test not supported. ' + errormsg + + ' Some tests will be skipped and the --timeout' + ' command line argument will not work.') diff --git a/llvm/utils/lit/tests/shtest-timeout.py b/llvm/utils/lit/tests/shtest-timeout.py index 1208e7379d0e..56415fe5b3ca 100644 --- a/llvm/utils/lit/tests/shtest-timeout.py +++ b/llvm/utils/lit/tests/shtest-timeout.py @@ -1,4 +1,4 @@ -# REQUIRES: python-psutil +# REQUIRES: lit-max-individual-test-time # llvm.org/PR33944 # UNSUPPORTED: system-windows