-
Notifications
You must be signed in to change notification settings - Fork 53
fix: autolock timer persistence, race conditions, and reload safety #601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
85d73cd
e2e42f4
5beff20
38cdbc0
d84b73e
4f2e50e
2b6f605
657d09b
bdb4840
6809cd3
b6b87f3
1cd9201
d6f83d0
9858c2b
86b0f99
7391c0a
a083c34
9861be2
11f1372
2778cad
5a0bb94
d8ce706
8dafbb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,3 +23,6 @@ package-lock.json | |
| .cursor/ | ||
| .aider* | ||
| .continue/ | ||
|
|
||
| # Serena MCP local cache | ||
| .serena/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,6 +122,9 @@ def __init__(self, hass: HomeAssistant) -> None: | |
| self._timer_store: Store[dict[str, TimerStoreEntry]] = Store( | ||
| hass, TIMER_STORAGE_VERSION, TIMER_STORAGE_KEY | ||
| ) | ||
| # Shared across all KeymasterTimer instances writing to _timer_store so | ||
| # concurrent persists/removes can't drop each other's entries. | ||
| self._timer_store_lock = asyncio.Lock() | ||
|
|
||
| async def initial_setup(self) -> None: | ||
| """Trigger the initial async_setup.""" | ||
|
|
@@ -928,6 +931,7 @@ async def _setup_timer(self, kmlock: KeymasterLock) -> None: | |
| call_action=functools.partial(self._timer_triggered, kmlock), | ||
| timer_id=f"{kmlock.keymaster_config_entry_id}_autolock", | ||
| store=self._timer_store, | ||
| store_lock=self._timer_store_lock, | ||
| ) | ||
| if kmlock.autolock_timer.is_running: | ||
| self.async_set_updated_data(dict(self.kmlocks)) | ||
|
|
@@ -1075,64 +1079,73 @@ async def _update_lock(self, new: KeymasterLock) -> bool: | |
| or not old.code_slots | ||
| ): | ||
| return False | ||
| await KeymasterCoordinator._unsubscribe_listeners(old) | ||
| # _LOGGER.debug("[update_lock] %s: old: %s", new.lock_name, old) | ||
| del_code_slots: list[int] = [ | ||
| old.starting_code_slot + i for i in range(old.number_of_code_slots) | ||
| ] | ||
| for code_slot_num in range( | ||
| new.starting_code_slot, | ||
| new.starting_code_slot + new.number_of_code_slots, | ||
| ): | ||
| if code_slot_num in del_code_slots: | ||
| del_code_slots.remove(code_slot_num) | ||
|
|
||
| new.lock_state = old.lock_state | ||
| new.door_state = old.door_state | ||
| new.autolock_enabled = old.autolock_enabled | ||
| new.autolock_min_day = old.autolock_min_day | ||
| new.autolock_min_night = old.autolock_min_night | ||
| new.retry_lock = old.retry_lock | ||
| for code_slot_num, new_kmslot in new.code_slots.items(): | ||
| if code_slot_num not in old.code_slots: | ||
| continue | ||
| old_kmslot: KeymasterCodeSlot = old.code_slots[code_slot_num] | ||
| new_kmslot.enabled = old_kmslot.enabled | ||
| new_kmslot.name = old_kmslot.name | ||
| new_kmslot.pin = old_kmslot.pin | ||
| new_kmslot.override_parent = old_kmslot.override_parent | ||
| new_kmslot.notifications = old_kmslot.notifications | ||
| new_kmslot.accesslimit_count_enabled = old_kmslot.accesslimit_count_enabled | ||
| new_kmslot.accesslimit_count = old_kmslot.accesslimit_count | ||
| new_kmslot.accesslimit_date_range_enabled = old_kmslot.accesslimit_date_range_enabled | ||
| new_kmslot.accesslimit_date_range_start = old_kmslot.accesslimit_date_range_start | ||
| new_kmslot.accesslimit_date_range_end = old_kmslot.accesslimit_date_range_end | ||
| new_kmslot.accesslimit_day_of_week_enabled = old_kmslot.accesslimit_day_of_week_enabled | ||
| if not new_kmslot.accesslimit_day_of_week: | ||
| continue | ||
| for dow_num, new_dow in new_kmslot.accesslimit_day_of_week.items(): | ||
| if not old_kmslot.accesslimit_day_of_week: | ||
| continue | ||
| old_dow: KeymasterCodeSlotDayOfWeek = old_kmslot.accesslimit_day_of_week[dow_num] | ||
| new_dow.dow_enabled = old_dow.dow_enabled | ||
| new_dow.limit_by_time = old_dow.limit_by_time | ||
| new_dow.include_exclude = old_dow.include_exclude | ||
| new_dow.time_start = old_dow.time_start | ||
| new_dow.time_end = old_dow.time_end | ||
| self.kmlocks[new.keymaster_config_entry_id] = new | ||
| # _LOGGER.debug("[update_lock] %s: new: %s", new.lock_name, new) | ||
| _LOGGER.debug("[update_lock] Code slot entities to delete: %s", del_code_slots) | ||
| for code_slot_num in del_code_slots: | ||
| await delete_code_slot_entities( | ||
| hass=self.hass, | ||
| keymaster_config_entry_id=new.keymaster_config_entry_id, | ||
| code_slot_num=code_slot_num, | ||
| ) | ||
| await self._rebuild_lock_relationships() | ||
| await self._update_door_and_lock_state() | ||
| await self._update_listeners(self.kmlocks[new.keymaster_config_entry_id]) | ||
| await self._setup_timer(self.kmlocks[new.keymaster_config_entry_id]) | ||
| await self.async_refresh() | ||
| # Unsubscribe listeners FIRST so in-flight provider/door callbacks | ||
| # can't reach the about-to-be-detached timer (where start()/cancel() | ||
| # would silently fail or no-op). Then detach the old timer; its | ||
| # detach() awaits any in-flight _on_expired so mutations it made on | ||
| # old (e.g. pending_retry_lock) are visible to the state copy. | ||
| # Once detached, the old timer is unusable until the new timer is | ||
| # set up — wrap the rest in try/except that restores listeners and | ||
| # the autolock on whichever kmlock is current if any step fails. | ||
| try: | ||
| await KeymasterCoordinator._unsubscribe_listeners(old) | ||
| if old.autolock_timer: | ||
|
Comment on lines
+1082
to
+1092
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledging — the in-flight |
||
| await old.autolock_timer.detach() | ||
| # _LOGGER.debug("[update_lock] %s: old: %s", new.lock_name, old) | ||
| del_code_slots: list[int] = [ | ||
| old.starting_code_slot + i for i in range(old.number_of_code_slots) | ||
|
raman325 marked this conversation as resolved.
|
||
| ] | ||
| for code_slot_num in range( | ||
| new.starting_code_slot, | ||
| new.starting_code_slot + new.number_of_code_slots, | ||
| ): | ||
| if code_slot_num in del_code_slots: | ||
| del_code_slots.remove(code_slot_num) | ||
|
|
||
| new.inherit_state_from(old) | ||
| self.kmlocks[new.keymaster_config_entry_id] = new | ||
| # _LOGGER.debug("[update_lock] %s: new: %s", new.lock_name, new) | ||
| _LOGGER.debug("[update_lock] Code slot entities to delete: %s", del_code_slots) | ||
| for code_slot_num in del_code_slots: | ||
| await delete_code_slot_entities( | ||
| hass=self.hass, | ||
| keymaster_config_entry_id=new.keymaster_config_entry_id, | ||
| code_slot_num=code_slot_num, | ||
| ) | ||
| await self._rebuild_lock_relationships() | ||
| await self._update_door_and_lock_state() | ||
| await self._update_listeners(self.kmlocks[new.keymaster_config_entry_id]) | ||
| await self._setup_timer(self.kmlocks[new.keymaster_config_entry_id]) | ||
| await self.async_refresh() | ||
| except Exception: | ||
| current = self.kmlocks.get(new.keymaster_config_entry_id) | ||
| if current is not None: | ||
| _LOGGER.exception( | ||
| "[update_lock] %s: Update failed mid-flight; " | ||
| "restoring autolock timer and listeners to keep state from being silently lost", | ||
| new.lock_name, | ||
| ) | ||
| # Restore in REVERSE order of the teardown (mirror the | ||
| # listeners-then-detach order in the try block): | ||
| # 1. _setup_timer first so the rebound timer is fully | ||
| # attached before any listener callback can reach it. | ||
| # 2. _update_listeners last so callbacks only fire after | ||
| # both the timer and listeners exist. | ||
| try: | ||
| await self._setup_timer(current) | ||
| except Exception: | ||
|
raman325 marked this conversation as resolved.
|
||
| _LOGGER.exception( | ||
| "[update_lock] %s: Failed to restore autolock timer after rollback", | ||
| new.lock_name, | ||
| ) | ||
| try: | ||
| await self._update_listeners(current) | ||
| except Exception: | ||
| _LOGGER.exception( | ||
| "[update_lock] %s: Failed to restore listeners after rollback", | ||
| new.lock_name, | ||
| ) | ||
| raise | ||
| return True | ||
|
|
||
| async def _delete_lock(self, kmlock: KeymasterLock, _: dt) -> None: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.