Carry completion timing under the lock; shut down before close
The completed event's was_on_time travels on the CompletionResult (decided with the write) instead of re-reading latest state a racing reversal could replace; unload clears coordinator listeners before the database close.
This commit is contained in:
@@ -615,16 +615,19 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
|
|||||||
if unload_error is None:
|
if unload_error is None:
|
||||||
unload_error = err
|
unload_error = err
|
||||||
finally:
|
finally:
|
||||||
database = getattr(runtime_data, "database", None)
|
# The coordinator's entity listeners must stop BEFORE the
|
||||||
if database is not None:
|
# database closes: a scheduled refresh starting during the
|
||||||
await database.close()
|
# close await would touch a cleared connection. (The
|
||||||
# Tear the shared coordinator down after the DB close: no
|
# platform unload above already removed the entities; this
|
||||||
# listener may outlive the connection it reads.
|
# clears the coordinator's own remaining listeners.)
|
||||||
coordinator = getattr(runtime_data, "coordinator", None)
|
coordinator = getattr(runtime_data, "coordinator", None)
|
||||||
if coordinator is not None:
|
if coordinator is not None:
|
||||||
shutdown = getattr(coordinator, "async_shutdown", None)
|
shutdown = getattr(coordinator, "async_shutdown", None)
|
||||||
if shutdown is not None:
|
if shutdown is not None:
|
||||||
await shutdown()
|
await shutdown()
|
||||||
|
database = getattr(runtime_data, "database", None)
|
||||||
|
if database is not None:
|
||||||
|
await database.close()
|
||||||
# Domain-global services are torn down only once the FINAL entry
|
# Domain-global services are torn down only once the FINAL entry
|
||||||
# unloads (``hass.data[DOMAIN]`` is now empty), never when a sibling
|
# unloads (``hass.data[DOMAIN]`` is now empty), never when a sibling
|
||||||
# entry is still loaded.
|
# entry is still loaded.
|
||||||
|
|||||||
@@ -374,14 +374,25 @@ class CompletionResult(str):
|
|||||||
appended an event (decided under the same lock as the append), so
|
appended an event (decided under the same lock as the append), so
|
||||||
a caller — the Feature 09 service path firing bus events —
|
a caller — the Feature 09 service path firing bus events —
|
||||||
announces a transition exactly once even under a concurrent
|
announces a transition exactly once even under a concurrent
|
||||||
duplicate call where the loser's write is a no-op.
|
duplicate call where the loser's write is a no-op. A completion
|
||||||
|
also carries the ``was_on_time`` verdict computed under that same
|
||||||
|
lock, so the event payload never re-reads mutable latest state a
|
||||||
|
racing reversal could have replaced.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
appended: bool
|
appended: bool
|
||||||
|
was_on_time: bool | None
|
||||||
|
|
||||||
def __new__(cls, state: str, *, appended: bool) -> "CompletionResult":
|
def __new__(
|
||||||
|
cls,
|
||||||
|
state: str,
|
||||||
|
*,
|
||||||
|
appended: bool,
|
||||||
|
was_on_time: bool | None = None,
|
||||||
|
) -> "CompletionResult":
|
||||||
obj = super().__new__(cls, state)
|
obj = super().__new__(cls, state)
|
||||||
obj.appended = appended
|
obj.appended = appended
|
||||||
|
obj.was_on_time = was_on_time
|
||||||
return obj
|
return obj
|
||||||
|
|
||||||
|
|
||||||
@@ -429,6 +440,10 @@ async def complete_instance(
|
|||||||
f"event_type must be '{EVENT_COMPLETED}' or "
|
f"event_type must be '{EVENT_COMPLETED}' or "
|
||||||
f"'{EVENT_UNCOMPLETED}', got {latest.event_type!r}"
|
f"'{EVENT_UNCOMPLETED}', got {latest.event_type!r}"
|
||||||
)
|
)
|
||||||
|
# Computed under the completion lock and carried on the result:
|
||||||
|
# the bus event must report THIS completion's timing, never a
|
||||||
|
# re-read of the latest event a racing reversal could replace.
|
||||||
|
on_time = _was_on_time(instance, now, today)
|
||||||
await append_event(
|
await append_event(
|
||||||
database,
|
database,
|
||||||
instance.id,
|
instance.id,
|
||||||
@@ -437,10 +452,10 @@ async def complete_instance(
|
|||||||
actor_source=actor_source,
|
actor_source=actor_source,
|
||||||
actor_user_id=actor_user_id,
|
actor_user_id=actor_user_id,
|
||||||
actor_child_id=actor_child_id,
|
actor_child_id=actor_child_id,
|
||||||
was_on_time=_was_on_time(instance, now, today),
|
was_on_time=on_time,
|
||||||
now=now,
|
now=now,
|
||||||
)
|
)
|
||||||
return CompletionResult("done", appended=True)
|
return CompletionResult("done", appended=True, was_on_time=on_time)
|
||||||
|
|
||||||
|
|
||||||
async def uncomplete_instance(
|
async def uncomplete_instance(
|
||||||
|
|||||||
@@ -71,22 +71,24 @@ async def _instance_payload(
|
|||||||
|
|
||||||
|
|
||||||
async def fire_quest_completed(
|
async def fire_quest_completed(
|
||||||
hass, database: NestQuestDatabase, instance_id: int
|
hass,
|
||||||
|
database: NestQuestDatabase,
|
||||||
|
instance_id: int,
|
||||||
|
*,
|
||||||
|
was_on_time: bool | None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Fire ``nestquest_quest_completed`` and, when the completion
|
"""Fire ``nestquest_quest_completed`` and, when the completion
|
||||||
cleared the child's whole day, ``nestquest_child_day_complete``.
|
cleared the child's whole day, ``nestquest_child_day_complete``.
|
||||||
|
|
||||||
Called by the complete_quest service handler AFTER an actual
|
Called by the complete_quest service handler AFTER an actual
|
||||||
completion transition; a no-op re-complete never reaches here.
|
completion transition, with the completion's ``was_on_time`` —
|
||||||
|
computed under the completion lock and carried on the result — so
|
||||||
|
the payload never re-reads mutable latest state a racing reversal
|
||||||
|
could have replaced. A no-op re-complete never reaches here.
|
||||||
"""
|
"""
|
||||||
now_stamp = _now_stamp()
|
now_stamp = _now_stamp()
|
||||||
payload = await _instance_payload(database, instance_id, now_stamp)
|
payload = await _instance_payload(database, instance_id, now_stamp)
|
||||||
latest = await CompletionEventsDao(database).get_latest_for_instance(
|
payload["was_on_time"] = was_on_time
|
||||||
instance_id
|
|
||||||
)
|
|
||||||
payload["was_on_time"] = (
|
|
||||||
latest.was_on_time if latest is not None else None
|
|
||||||
)
|
|
||||||
hass.bus.async_fire(EVENT_QUEST_COMPLETED, payload)
|
hass.bus.async_fire(EVENT_QUEST_COMPLETED, payload)
|
||||||
|
|
||||||
# Day-complete: quests were owed today and none remain (the same
|
# Day-complete: quests were owed today and none remain (the same
|
||||||
|
|||||||
@@ -317,7 +317,12 @@ def _complete_quest(
|
|||||||
**_actor_kwargs(call),
|
**_actor_kwargs(call),
|
||||||
)
|
)
|
||||||
if result.appended:
|
if result.appended:
|
||||||
await fire_quest_completed(hass, runtime.database, instance_id)
|
await fire_quest_completed(
|
||||||
|
hass,
|
||||||
|
runtime.database,
|
||||||
|
instance_id,
|
||||||
|
was_on_time=result.was_on_time,
|
||||||
|
)
|
||||||
await _refresh_entities(runtime)
|
await _refresh_entities(runtime)
|
||||||
|
|
||||||
return _with_errors(_handler)
|
return _with_errors(_handler)
|
||||||
|
|||||||
@@ -157,3 +157,31 @@ async def test_restart_equivalent_restores_all_entity_states(
|
|||||||
assert hass.entities[
|
assert hass.entities[
|
||||||
"nestquest_household_quests_completed_today"
|
"nestquest_household_quests_completed_today"
|
||||||
].native_value == 1
|
].native_value == 1
|
||||||
|
|
||||||
|
|
||||||
|
async def test_unload_shuts_coordinator_before_closing_database(
|
||||||
|
hass, make_entry
|
||||||
|
) -> None:
|
||||||
|
"""The coordinator's listeners are cleared while the database is
|
||||||
|
still connected: a scheduled refresh must never race the close."""
|
||||||
|
entry = wire_entry_to_registry(make_entry(), hass.registry)
|
||||||
|
assert await async_setup_entry(hass, entry) is True
|
||||||
|
coordinator = entry.runtime_data.coordinator
|
||||||
|
database = entry.runtime_data.database
|
||||||
|
seen_connected = []
|
||||||
|
|
||||||
|
async def _probe_shutdown():
|
||||||
|
seen_connected.append(database.connected)
|
||||||
|
|
||||||
|
original_close = database.close
|
||||||
|
|
||||||
|
async def _recording_close():
|
||||||
|
seen_connected.append("closing")
|
||||||
|
await original_close()
|
||||||
|
|
||||||
|
coordinator.async_shutdown = _probe_shutdown
|
||||||
|
database.close = _recording_close
|
||||||
|
assert await async_unload_entry(hass, entry) is True
|
||||||
|
assert seen_connected == [True, "closing"], (
|
||||||
|
"coordinator shutdown must precede the database close"
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user