From ef666acbd201ef37cdff5ff3257fe37da860980a Mon Sep 17 00:00:00 2001 From: Giampaolo Rodola Date: Tue, 1 Aug 2023 08:10:04 +0200 Subject: [PATCH] psutil.cpu_percent() is not thread safe (fixes #1703) (#2282) `psutil.cpu_percent(interval=None)` and `psutil.cpu_times_percent(interval=None)` are non-blocking functions returning the CPU percent consumption since the last time they were called. In order to do so they use a global variable, in which the last CPU timings are saved, and that means they are not thread safe. E.g., if 10 threads call `cpu_percent(interval=None)` with a 1 second interval, only 1 thread out of 10 will get the right result, as it will "invalidate" the timings for the other 9. Problem can be reproduced with the following script: ```python import threading, time, psutil NUM_WORKERS = psutil.cpu_count() def measure_cpu(): while 1: print(psutil.cpu_percent()) time.sleep(1) for x in range(NUM_WORKERS): threading.Thread(target=measure_cpu).start() while 1: print() time.sleep(1.1) ``` The output looks like this, and it shows how inconsistent CPU measurements are between different threads (notice 0.0 values): ``` 3.5 3.5 0.0 0.0 2.8 2.8 0.0 0.0 2.5 2.5 0.0 0.0 2.5 2.5 2.5 2.5 3.3 3.3 3.3 50.0 2.8 0.0 0.0 0.0 ``` After patch: ``` 0.0 0.0 0.0 0.0 2.0 2.3 2.3 2.3 5.5 5.3 5.5 5.5 3.3 3.3 3.0 3.0 9.0 8.9 9.0 9.4 30.0 30.0 29.6 30.0 24.7 24.7 24.7 24.7 ``` --- HISTORY.rst | 14 +++++++-- docs/index.rst | 9 ++++++ psutil/__init__.py | 71 ++++++++++++++++------------------------------ 3 files changed, 46 insertions(+), 48 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 34ed06515..80db358e4 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -5,10 +5,20 @@ XXXX-XX-XX -- 2241_, [NetBSD]: can't compile On NetBSD 10.99.3/amd64. (patch by Thomas - Klausner) +**Enhancements** + +- 1703_: `cpu_percent()`_ and `cpu_times_percent()`_ are now thread safe, + meaning they can be called from different threads and still return + meaningful and independent results. Before, if (say) 10 threads called + ``cpu_percent(interval=None)`` at the same time, only 1 thread out of 10 + would get the right result. - 2266_: if `Process`_ class is passed a very high PID, raise `NoSuchProcess`_ instead of OverflowError. (patch by Xuehai Pan) + +**Bug fixes** + +- 2241_, [NetBSD]: can't compile On NetBSD 10.99.3/amd64. (patch by Thomas + Klausner) - 2268_: ``bytes2human()`` utility function was unable to properly represent negative values. - 2252_: [Windows]: `psutil.disk_usage`_ fails on Python 3.12+. (patch by Matthieu Darbois) diff --git a/docs/index.rst b/docs/index.rst index 6b0408dea..a43e8f51e 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -176,6 +176,10 @@ CPU utilization as a percentage for each CPU. First element of the list refers to first CPU, second element to second CPU and so on. The order of the list is consistent across calls. + Internally this function maintains a global map (a dict) where each key is + the ID of the calling thread (`threading.get_ident`_). This means it can be + called from different threads, at different intervals, and still return + meaningful and independent results. >>> import psutil >>> # blocking @@ -194,6 +198,8 @@ CPU it will return a meaningless ``0.0`` value which you are supposed to ignore. + .. versionchanged:: 5.9.6 function is now thread safe. + .. function:: cpu_times_percent(interval=None, percpu=False) Same as :func:`cpu_percent()` but provides utilization percentages for each @@ -212,6 +218,8 @@ CPU .. versionchanged:: 4.1.0 two new *interrupt* and *dpc* fields are returned on Windows. + .. versionchanged:: 5.9.6 function is now thread safe. + .. function:: cpu_count(logical=True) Return the number of logical CPUs in the system (same as `os.cpu_count`_ @@ -3052,6 +3060,7 @@ Timeline .. _`subprocess.Popen`: https://docs.python.org/3/library/subprocess.html#subprocess.Popen .. _`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 +.. _`threading.get_ident`: https://docs.python.org/3/library/threading.html#threading.get_ident .. _`threading.Thread`: https://docs.python.org/3/library/threading.html#threading.Thread .. _Tidelift security contact: https://tidelift.com/security .. _Tidelift Subscription: https://tidelift.com/subscription/pkg/pypi-psutil?utm_source=pypi-psutil&utm_medium=referral&utm_campaign=readme diff --git a/psutil/__init__.py b/psutil/__init__.py index 2b1567065..68f3c7ca9 100644 --- a/psutil/__init__.py +++ b/psutil/__init__.py @@ -211,7 +211,7 @@ AF_LINK = _psplatform.AF_LINK __author__ = "Giampaolo Rodola'" -__version__ = "5.9.5" +__version__ = "5.9.6" version_info = tuple([int(num) for num in __version__.split('.')]) _timer = getattr(time, 'monotonic', time.time) @@ -1629,16 +1629,18 @@ def cpu_times(percpu=False): try: - _last_cpu_times = cpu_times() + _last_cpu_times = {threading.current_thread().ident: cpu_times()} except Exception: # Don't want to crash at import time. - _last_cpu_times = None + _last_cpu_times = {} try: - _last_per_cpu_times = cpu_times(percpu=True) + _last_per_cpu_times = { + threading.current_thread().ident: cpu_times(percpu=True) + } except Exception: # Don't want to crash at import time. - _last_per_cpu_times = None + _last_per_cpu_times = {} def _cpu_tot_time(times): @@ -1732,8 +1734,7 @@ def cpu_percent(interval=None, percpu=False): 2.9 >>> """ - global _last_cpu_times - global _last_per_cpu_times + tid = threading.current_thread().ident blocking = interval is not None and interval > 0.0 if interval is not None and interval < 0: raise ValueError("interval is not positive (got %r)" % interval) @@ -1756,14 +1757,9 @@ def calculate(t1, t2): t1 = cpu_times() time.sleep(interval) else: - t1 = _last_cpu_times - if t1 is None: - # Something bad happened at import time. We'll - # get a meaningful result on the next call. See: - # https://github.com/giampaolo/psutil/pull/715 - t1 = cpu_times() - _last_cpu_times = cpu_times() - return calculate(t1, _last_cpu_times) + t1 = _last_cpu_times.get(tid) or cpu_times() + _last_cpu_times[tid] = cpu_times() + return calculate(t1, _last_cpu_times[tid]) # per-cpu usage else: ret = [] @@ -1771,23 +1767,17 @@ def calculate(t1, t2): tot1 = cpu_times(percpu=True) time.sleep(interval) else: - tot1 = _last_per_cpu_times - if tot1 is None: - # Something bad happened at import time. We'll - # get a meaningful result on the next call. See: - # https://github.com/giampaolo/psutil/pull/715 - tot1 = cpu_times(percpu=True) - _last_per_cpu_times = cpu_times(percpu=True) - for t1, t2 in zip(tot1, _last_per_cpu_times): + tot1 = _last_per_cpu_times.get(tid) or cpu_times(percpu=True) + _last_per_cpu_times[tid] = cpu_times(percpu=True) + for t1, t2 in zip(tot1, _last_per_cpu_times[tid]): ret.append(calculate(t1, t2)) return ret -# Use separate global vars for cpu_times_percent() so that it's -# independent from cpu_percent() and they can both be used within -# the same program. -_last_cpu_times_2 = _last_cpu_times -_last_per_cpu_times_2 = _last_per_cpu_times +# Use a separate dict for cpu_times_percent(), so it's independent from +# cpu_percent() and they can both be used within the same program. +_last_cpu_times_2 = _last_cpu_times.copy() +_last_per_cpu_times_2 = _last_per_cpu_times.copy() def cpu_times_percent(interval=None, percpu=False): @@ -1803,8 +1793,7 @@ def cpu_times_percent(interval=None, percpu=False): *interval* and *percpu* arguments have the same meaning as in cpu_percent(). """ - global _last_cpu_times_2 - global _last_per_cpu_times_2 + tid = threading.current_thread().ident blocking = interval is not None and interval > 0.0 if interval is not None and interval < 0: raise ValueError("interval is not positive (got %r)" % interval) @@ -1832,14 +1821,9 @@ def calculate(t1, t2): t1 = cpu_times() time.sleep(interval) else: - t1 = _last_cpu_times_2 - if t1 is None: - # Something bad happened at import time. We'll - # get a meaningful result on the next call. See: - # https://github.com/giampaolo/psutil/pull/715 - t1 = cpu_times() - _last_cpu_times_2 = cpu_times() - return calculate(t1, _last_cpu_times_2) + t1 = _last_cpu_times_2.get(tid) or cpu_times() + _last_cpu_times_2[tid] = cpu_times() + return calculate(t1, _last_cpu_times_2[tid]) # per-cpu usage else: ret = [] @@ -1847,14 +1831,9 @@ def calculate(t1, t2): tot1 = cpu_times(percpu=True) time.sleep(interval) else: - tot1 = _last_per_cpu_times_2 - if tot1 is None: - # Something bad happened at import time. We'll - # get a meaningful result on the next call. See: - # https://github.com/giampaolo/psutil/pull/715 - tot1 = cpu_times(percpu=True) - _last_per_cpu_times_2 = cpu_times(percpu=True) - for t1, t2 in zip(tot1, _last_per_cpu_times_2): + tot1 = _last_per_cpu_times_2.get(tid) or cpu_times(percpu=True) + _last_per_cpu_times_2[tid] = cpu_times(percpu=True) + for t1, t2 in zip(tot1, _last_per_cpu_times_2[tid]): ret.append(calculate(t1, t2)) return ret