From 31fedb1702c110e0a280ccc738e09b1ae69a326b Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Mon, 29 Dec 2014 12:06:21 -0800 Subject: [PATCH] Bug 1116194 - Catch errors calling psutil; r=ted The build system / mach currently has a very hacky virtualenv setup. Essentially, it resorts to sys.path munging instead of a proper, isolated environment. During initialization, mach installs python/psutil in sys.path. Later on, some code does an |import psutil|. This fails iff the psutil C extension can't be found. If there is a psutil C extension installed outside of mach and python/psutil, |import psutil| may load it. The version mismatch isn't detected until an extension-using psutil API is called. This has manifested inside |mach build| via the resource monitor as an |AttributeError: 'module' object has no attribute 'linux_sysinfo'| exception during psutil.virtual_memory(). The proper fix for this is for the Python environment to ensure the psutil C extension is built before attempting to import and use psutil. Arguably, psutil itself should perform some kind of version check when it imports the C extension to ensure things are in sync and fail at import time. Fixing mach and the build system Python environment to build psutil earlier/properly is a long outstanding bug. It needs to be addressed. But it is considerable effort. This patch continues the long history of wallpapering over psutil import/run failures because using a proper virutalenv from mach/build system is a lot of work. Sad panda. --- .../mozsystemmonitor/resourcemonitor.py | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/testing/mozbase/mozsystemmonitor/mozsystemmonitor/resourcemonitor.py b/testing/mozbase/mozsystemmonitor/mozsystemmonitor/resourcemonitor.py index 59db40c1e40a..dd2d8f448617 100644 --- a/testing/mozbase/mozsystemmonitor/mozsystemmonitor/resourcemonitor.py +++ b/testing/mozbase/mozsystemmonitor/mozsystemmonitor/resourcemonitor.py @@ -5,6 +5,7 @@ import multiprocessing import sys import time +import warnings # psutil will raise NotImplementedError if the platform is not supported. try: @@ -178,15 +179,26 @@ class SystemResourceMonitor(object): self._running = False self._stopped = False + self._process = None if psutil is None: return - cpu_percent = psutil.cpu_percent(0.0, True) - cpu_times = psutil.cpu_times(False) - io = get_disk_io_counters() - virt = psutil.virtual_memory() - swap = psutil.swap_memory() + # This try..except should not be needed! However, some tools (like + # |mach build|) attempt to load psutil before properly creating a + # virtualenv by building psutil. As a result, python/psutil may be in + # sys.path and its .py files may pick up the psutil C extension from + # the system install. If the versions don't match, we typically see + # failures invoking one of these functions. + try: + cpu_percent = psutil.cpu_percent(0.0, True) + cpu_times = psutil.cpu_times(False) + io = get_disk_io_counters() + virt = psutil.virtual_memory() + swap = psutil.swap_memory() + except Exception as e: + warnings.warn('psutil failed to run: %s' % e) + return self._cpu_cores = len(cpu_percent) self._cpu_times_type = type(cpu_times) @@ -215,7 +227,7 @@ class SystemResourceMonitor(object): You should only call this once per instance. """ - if psutil is None: + if not self._process: return self._running = True @@ -229,7 +241,7 @@ class SystemResourceMonitor(object): Currently, data is not available until you call stop(). """ - if psutil is None: + if not self._process: self._stopped = True return