From e9656172e8a8ebef3fc882181ee5aec2605fa7ce Mon Sep 17 00:00:00 2001 From: Andrew Coles Date: Wed, 11 Mar 2026 09:56:38 +0000 Subject: [PATCH] Update get_history in HAHistory to take a copy of the history of an entity, to avoid race conditions. get_history previously returned (a list containing) the history stored in HAHistory. This history could then be accessed concurrently with changes being made to it by update_entity, with no locking. At a minimum, because update_entity sorts the history -- which is not thread safe -- the history may appear transiently empty during these updates. This race condition caused glitches elsewhere in the code; at a minimum, with load data appearing to be transiently empty, leading to poor estimates. This commit fixes the issue by making get_history return a *copy* of the history; and to hold the lock while making this copy, to ensure thread safety. To militate against the performance impact of this, minute_data in utils.py is updated to take an extra argument -- can_modify_history, default to False. If set to True, this will stop it from taking a copy it would otherwise make of the history object provided to it. fetch.py is then updated to set it to true in some places, as appropriate. Hence the fix has almost no impact on performance. Fixes springfall2008/batpred#3511 --- apps/predbat/fetch.py | 3 +++ apps/predbat/ha.py | 11 +++++++++++ apps/predbat/utils.py | 4 +++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/apps/predbat/fetch.py b/apps/predbat/fetch.py index 7809232d0..baa1c7fb7 100644 --- a/apps/predbat/fetch.py +++ b/apps/predbat/fetch.py @@ -577,6 +577,7 @@ def minute_data_import_export(self, max_days_previous, now_utc, key, scale=1.0, clean_increment=increment, accumulate=import_today, required_unit=required_unit, + can_modify_history=True, # history is not accessed after this point, so minute_data can freely modify it ) else: if history is None: @@ -639,6 +640,7 @@ def minute_data_load(self, now_utc, entity_name, max_days_previous, load_scaling accumulate=load_minutes, required_unit=required_unit, interpolate=interpolate, + can_modify_history=True, # history is not accessed after this point, so minute_data can freely modify it ) else: if history is None: @@ -855,6 +857,7 @@ def fetch_sensor_data(self, save=True): divide_by=1.0, scale=1.0, required_unit="kWh", + can_modify_history=True, # soc_kwh_data is not accessed after this point, so minute_data can freely modify it ) # Fetch sensor data for cars, e.g. car plan, car energy, car sessions etc. diff --git a/apps/predbat/ha.py b/apps/predbat/ha.py index e418bbbc4..417d625bd 100644 --- a/apps/predbat/ha.py +++ b/apps/predbat/ha.py @@ -26,6 +26,7 @@ import traceback import threading import time +import copy from utils import str2time from const import TIME_FORMAT_HA, TIMEOUT, TIME_FORMAT_HA_TZ from component_base import ComponentBase @@ -100,6 +101,16 @@ def get_history(self, entity_id, days=30, tracked=True): self.update_entity(entity_id, history_data) result = [history_data] + if result is not None: + + # Returning result itself is not thread-safe, as it introduces a race condition -- it can be accessed + # during calls to update_entity, where it may e.g. appear to be empty during the sort(). + # Hence, return a copy, while holding the history_lock to ensure result isn't modified during the copy. + + with self.history_lock: + result = copy.deepcopy(result) + + return result def prune_history(self, now): diff --git a/apps/predbat/utils.py b/apps/predbat/utils.py index 409a4b109..6e7bed1f3 100644 --- a/apps/predbat/utils.py +++ b/apps/predbat/utils.py @@ -315,6 +315,7 @@ def minute_data( max_increment=MAX_INCREMENT, interpolate=False, debug=False, + can_modify_history=False, ): """ Turns data from HA into a hash of data indexed by minute with the data being the value @@ -335,7 +336,8 @@ def minute_data( if not history: return mdata, io_adjusted - history = copy.deepcopy(history) # Copy to avoid modifying original history + if not can_modify_history: + history = copy.deepcopy(history) # Copy to avoid modifying original history # Glitch filter, cleans glitches in the data and removes bad values, only for incrementing data if clean_increment and backwards: