Stop publishing and saving from the start of unload; keep deletions final
Home Assistant marks a coordinator shut down only after the platforms are unloaded, and the model save checked for shutdown before waiting for the storage lock. A deletion could therefore be followed by a save that had been waiting for it, recreating the file. Also, unloading cancels a running scheduled update, which released the lock while the file write continued in a worker thread. - The coordinator is marked closing before platforms unload (restored if unloading fails); closing updates, relay callbacks and saves do nothing. - The save checks closing and a removed-model mark after acquiring the lock, and runs shielded so the lock is held until the write finishes. - Removal marks the model removed under the lock before deleting it. - A failed setup also shuts the coordinator down. Tests: unload and reload during a running optimization, removal during a save, a save waiting for a deletion, simultaneous saves, repeated setup/unload, shutdown ordering and failed setup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
87437cbd48
commit
28ff953ddd
4 files changed
+281
-5
No files matched your search
@@ -12,7 +12,7 @@ from homeassistant.helpers.event import async_track_state_change_event
|
||||
|
||||
from .const import CONF_HEATER, CONF_ROOMS, CONF_WEIGHTS, DOMAIN, CONF_OUTSIDE, WEATHER_SOURCES
|
||||
from .weather_sources import sources_for
|
||||
from .coordinator import HeatingMPCCoordinator, storage_lock
|
||||
from .coordinator import HeatingMPCCoordinator, mark_model_removed, storage_lock
|
||||
from .zones import SUBENTRY_ZONE, ZONE_KEY, ZONE_NAME, legacy_zone_data
|
||||
|
||||
PLATFORMS: list[Platform] = [Platform.SENSOR, Platform.BINARY_SENSOR]
|
||||
@@ -29,9 +29,11 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
|
||||
hass.data.setdefault(DOMAIN, {})[entry.entry_id] = coordinator
|
||||
await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
|
||||
except BaseException:
|
||||
# HA skips unload callbacks for unexpected setup errors, so clean up here.
|
||||
# HA skips unload callbacks for unexpected setup errors, so clean up here,
|
||||
# including a refresh scheduled by entities that were already added.
|
||||
unsub_relay()
|
||||
hass.data.get(DOMAIN, {}).pop(entry.entry_id, None)
|
||||
await coordinator.async_shutdown()
|
||||
raise
|
||||
entry.async_on_unload(unsub_relay)
|
||||
# Remove only obsolete zone forecasts belonging to this entry/platform. Zones
|
||||
@@ -53,17 +55,25 @@ async def async_reload_entry(hass: HomeAssistant, entry: ConfigEntry) -> None:
|
||||
|
||||
|
||||
async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
|
||||
coordinator = hass.data.get(DOMAIN, {}).get(entry.entry_id)
|
||||
if coordinator is not None:
|
||||
# Before anything is unloaded: an update still running must not publish or save.
|
||||
coordinator.async_begin_shutdown()
|
||||
ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
|
||||
if ok:
|
||||
hass.data[DOMAIN].pop(entry.entry_id, None)
|
||||
elif coordinator is not None:
|
||||
coordinator.async_cancel_shutdown()
|
||||
return ok
|
||||
|
||||
|
||||
async def async_remove_entry(hass: HomeAssistant, entry: ConfigEntry) -> None:
|
||||
"""Remove the learned model when this integration instance is deleted."""
|
||||
store = Store(hass, 1, f"{DOMAIN}.{entry.entry_id}")
|
||||
# Wait for a save still in progress, which would otherwise recreate the file.
|
||||
# Wait for a save still in progress, which would otherwise recreate the file;
|
||||
# a save still waiting for the lock sees the mark and does not write.
|
||||
async with storage_lock(hass, entry.entry_id):
|
||||
mark_model_removed(hass, entry.entry_id)
|
||||
await store.async_remove()
|
||||
hass.data.get(f"{DOMAIN}_storage_locks", {}).pop(entry.entry_id, None)
|
||||
|
||||
|
||||
@@ -56,6 +56,15 @@ def storage_lock(hass: HomeAssistant, entry_id: str) -> asyncio.Lock:
|
||||
return hass.data.setdefault(f"{DOMAIN}_storage_locks", {}).setdefault(entry_id, asyncio.Lock())
|
||||
|
||||
|
||||
def mark_model_removed(hass: HomeAssistant, entry_id: str) -> None:
|
||||
"""Record, under the storage lock, that the entry's model file was deleted for good."""
|
||||
hass.data.setdefault(f"{DOMAIN}_removed", set()).add(entry_id)
|
||||
|
||||
|
||||
def model_removed(hass: HomeAssistant, entry_id: str) -> bool:
|
||||
return entry_id in hass.data.get(f"{DOMAIN}_removed", ())
|
||||
|
||||
|
||||
def entry_zone_list(entry: ConfigEntry, config: dict[str, Any]) -> list[Zone]:
|
||||
"""Zones from the entry's zone subentries; one zone per legacy room sensor otherwise."""
|
||||
subentries = getattr(entry, "subentries", None) or {}
|
||||
@@ -148,18 +157,57 @@ class HeatingMPCCoordinator(DataUpdateCoordinator[dict[str, Any]]):
|
||||
self._off_samples = 0
|
||||
# Increases with every update; an older update finishing late never publishes.
|
||||
self._run = 0
|
||||
self._closing = False
|
||||
|
||||
async def async_restore_model(self) -> None:
|
||||
async with storage_lock(self.hass, self.entry.entry_id):
|
||||
await self._async_restore_model()
|
||||
|
||||
async def _async_save(self, outside_source: str | None) -> None:
|
||||
async def _async_save(self, outside_source: str | None) -> bool:
|
||||
"""Write the model unless unloading began or the model was deleted.
|
||||
|
||||
Shielded: unloading cancels a running scheduled update, but the file write
|
||||
itself continues in a worker thread. The lock must stay held until that
|
||||
write has finished, or a deletion could run first and be undone by it.
|
||||
"""
|
||||
write = self.hass.async_create_task(self._async_write(outside_source), f"{DOMAIN} model save")
|
||||
return await asyncio.shield(write)
|
||||
|
||||
async def _async_write(self, outside_source: str | None) -> bool:
|
||||
async with storage_lock(self.hass, self.entry.entry_id):
|
||||
# Checked after acquiring the lock, not before: a save that waited for
|
||||
# a deletion to finish must not write the file again.
|
||||
if self.closing or model_removed(self.hass, self.entry.entry_id):
|
||||
return False
|
||||
await self._store.async_save(self._payload(outside_source))
|
||||
return True
|
||||
|
||||
@property
|
||||
def closing(self) -> bool:
|
||||
"""True from the start of unloading: nothing is published, saved or recorded."""
|
||||
return self._closing or getattr(self, "_shutdown_requested", False)
|
||||
|
||||
@callback
|
||||
def async_begin_shutdown(self) -> None:
|
||||
"""Mark unloading before anything is unloaded.
|
||||
|
||||
Home Assistant's own shutdown of the coordinator runs only after platforms
|
||||
are unloaded; an update still running in that window must not publish or save.
|
||||
"""
|
||||
self._closing = True
|
||||
|
||||
@callback
|
||||
def async_cancel_shutdown(self) -> None:
|
||||
"""Unloading failed and the entry stays loaded: carry on."""
|
||||
self._closing = False
|
||||
|
||||
async def async_shutdown(self) -> None:
|
||||
self._closing = True
|
||||
await super().async_shutdown()
|
||||
|
||||
def _stale(self, run: int) -> bool:
|
||||
"""True when a newer update started or the entry is unloading."""
|
||||
return run != self._run or getattr(self, "_shutdown_requested", False)
|
||||
return run != self._run or self.closing
|
||||
|
||||
async def _async_restore_model(self) -> None:
|
||||
stored = await self._store.async_load() or {}
|
||||
@@ -303,6 +351,8 @@ class HeatingMPCCoordinator(DataUpdateCoordinator[dict[str, Any]]):
|
||||
@callback
|
||||
def async_relay_changed(self, event: Event) -> None:
|
||||
"""Record relay state changes as they happen; read-only state listener."""
|
||||
if self.closing:
|
||||
return # a callback queued before the listener was removed
|
||||
old, new = event.data.get("old_state"), event.data.get("new_state")
|
||||
if new is None:
|
||||
self.record_relay_state(event.time_fired, "removed")
|
||||
@@ -527,6 +577,8 @@ class HeatingMPCCoordinator(DataUpdateCoordinator[dict[str, Any]]):
|
||||
snapshot = {entity: self.hass.states.get(entity)
|
||||
for entity in set(self.sensors) | weather_entity_ids}
|
||||
self._run += 1
|
||||
if self.closing:
|
||||
return self._superseded({"status": "inputs_changed"})
|
||||
run = self._run
|
||||
inputs_before = self._input_snapshot()
|
||||
sensor_quality = {}
|
||||
|
||||
Reference in new issue
Block a user