From f72f4324d7591627473122c3732b794b61c47de9 Mon Sep 17 00:00:00 2001 From: David Berenstein Date: Thu, 20 Aug 2026 08:12:59 +0200 Subject: [PATCH 1/2] fix: lock CPU power history swap The 1 Hz monitor scheduler thread appends to `CPU._power_history` while the measurement scheduler thread drains it. The read-then-rebind in `total_power()` is not atomic, so any sample appended between the list comprehension and the rebinding was written to the discarded list and lost, biasing the reported `cpu_power`. Take the swap under a lock, held only for the O(1) rebinding so a slow `_get_power_from_cpus()` backend never blocks the monitor thread. Also drop the unreachable empty-history branch: a sample is always appended before the average. The test asserts the drain invariant rather than the drain shape, so it does not pin the implementation. Closes #1315 Co-Authored-By: Claude Opus 5 (1M context) --- codecarbon/external/hardware.py | 18 ++++++++++------- tests/test_cpu_load.py | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/codecarbon/external/hardware.py b/codecarbon/external/hardware.py index 9c6d113cc..2648b7fa4 100644 --- a/codecarbon/external/hardware.py +++ b/codecarbon/external/hardware.py @@ -4,6 +4,7 @@ import math import re +import threading import time from abc import ABC, abstractmethod from dataclasses import dataclass @@ -207,6 +208,8 @@ def __init__( ): assert tracking_mode in ["machine", "process"] self._power_history: List[Power] = [] + # the monitor thread appends while the measurement thread drains + self._power_history_lock = threading.Lock() self._output_dir = output_dir self._mode = mode self._model = model @@ -393,12 +396,12 @@ def _get_energy_from_cpus(self, delay: Time) -> Energy: return Energy.from_energy(energy) def total_power(self) -> Power: - self._power_history.append(self._get_power_from_cpus()) - power_history_in_W = [power.W for power in self._power_history] - self._power_history = [] - if not power_history_in_W: - logger.warning("No power samples collected, returning 0 W") - return Power.from_watts(0) + latest_sample = self._get_power_from_cpus() + with self._power_history_lock: + power_history = self._power_history + self._power_history = [] + power_history.append(latest_sample) + power_history_in_W = [power.W for power in power_history] cpu_power = sum(power_history_in_W) / len(power_history_in_W) return Power.from_watts(cpu_power) @@ -432,7 +435,8 @@ def start(self): def monitor_power(self): cpu_power = self._get_power_from_cpus() - self._power_history.append(cpu_power) + with self._power_history_lock: + self._power_history.append(cpu_power) def get_model(self): return self._model diff --git a/tests/test_cpu_load.py b/tests/test_cpu_load.py index 7f6243043..e6605e11c 100644 --- a/tests/test_cpu_load.py +++ b/tests/test_cpu_load.py @@ -1,3 +1,4 @@ +import threading import unittest from time import sleep from unittest import mock @@ -172,6 +173,40 @@ def test_cpu_total_power_fetches_sample_when_history_is_empty( mocked_get_power_from_cpus.assert_called_once() self.assertEqual(cpu._power_history, []) + @mock.patch( + "codecarbon.external.hardware.CPU._get_power_from_cpus", + return_value=Power.from_watts(1), + ) + def test_cpu_total_power_keeps_samples_added_while_draining( + self, + mocked_get_power_from_cpus, + mocked_is_psutil_available, + mocked_is_powergadget_available, + mocked_is_rapl_available, + ): + """A sample appended by the monitor thread while total_power drains the + history must not be lost (see issue #1315).""" + cpu = CPU.from_utils( + None, MODE_CPU_LOAD, "Intel(R) Core(TM) i7-7600U CPU @ 2.80GHz", 100 + ) + cpu._power_history = [Power.from_watts(1)] + appended = threading.Event() + + def monitor(): + cpu.monitor_power() + appended.set() + + monitor_thread = threading.Thread(target=monitor) + with cpu._power_history_lock: + monitor_thread.start() + # The monitor thread must wait instead of appending to a history + # total_power is about to discard. + self.assertFalse(appended.wait(0.2)) + monitor_thread.join(1) + + self.assertTrue(appended.is_set()) + self.assertEqual(len(cpu._power_history), 2) + @mock.patch( "codecarbon.external.hardware.CPU._get_power_from_cpus", return_value=Power.from_watts(30), From 44ef6353c73d7e40cab78b96bcf473f50649d6bc Mon Sep 17 00:00:00 2001 From: David Berenstein Date: Wed, 23 Sep 2026 16:47:11 +0900 Subject: [PATCH 2/2] test(cpu): pin that a sample landing after the swap reaches the next window Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_cpu_load.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/test_cpu_load.py b/tests/test_cpu_load.py index e6605e11c..78243c5b6 100644 --- a/tests/test_cpu_load.py +++ b/tests/test_cpu_load.py @@ -207,6 +207,47 @@ def monitor(): self.assertTrue(appended.is_set()) self.assertEqual(len(cpu._power_history), 2) + def test_cpu_sample_appended_right_after_the_swap_goes_to_next_window( + self, + mocked_is_psutil_available, + mocked_is_powergadget_available, + mocked_is_rapl_available, + ): + """A monitor sample that lands just after total_power swaps the history + is counted in the next total_power() call, not lost.""" + cpu = CPU.from_utils( + None, MODE_CPU_LOAD, "Intel(R) Core(TM) i7-7600U CPU @ 2.80GHz", 100 + ) + cpu._power_history = [Power.from_watts(10)] + real_lock = cpu._power_history_lock + + class MonitorAfterSwap: + fired = False + + def __enter__(self): + real_lock.acquire() + + def __exit__(self, *exc): + real_lock.release() + if not MonitorAfterSwap.fired: + MonitorAfterSwap.fired = True + monitor = threading.Thread(target=cpu.monitor_power, name="mon") + monitor.start() + monitor.join(1) + + def fake_power(): + watts = 100 if threading.current_thread().name == "mon" else 1 + return Power.from_watts(watts) + + cpu._power_history_lock = MonitorAfterSwap() + with mock.patch.object(cpu, "_get_power_from_cpus", side_effect=fake_power): + first = cpu.total_power() # drains [10] plus latest 1 + second = cpu.total_power() # the 100 W sample plus latest 1 + + self.assertEqual(first.W, (10 + 1) / 2) + self.assertEqual(second.W, (100 + 1) / 2) + self.assertEqual(cpu._power_history, []) + @mock.patch( "codecarbon.external.hardware.CPU._get_power_from_cpus", return_value=Power.from_watts(30),