mirror of
https://github.com/home-assistant/supervisor.git
synced 2026-08-18 19:02:33 +01:00
95bb8fe6ab
* Make Core.shutdown idempotent and safe to call concurrently After #6887, Core.shutdown() now runs in the SIGTERM path during host shutdown in addition to the existing host.control reboot/shutdown and backup restore paths. Multiple concurrent callers were possible (e.g. SIGTERM arriving while a reboot API call is mid-flight), so __main__.py debounced the signal handler by stashing the in-flight task in a single- element list and bailing out on the second SIGTERM. Move the idempotency into Core.shutdown() itself, where it belongs: - A second call while shutdown is in progress awaits the in-flight shutdown via an asyncio.Event rather than re-running the sequence. - Calls during STOPPING/CLOSE return early (Supervisor is already going away; the work is moot). - Calls during STARTING_STATES (INITIALIZE/STARTUP/SETUP) return early too. There is nothing coherent to gracefully stop before startup completes, and on the SIGTERM-during-startup path the caller cancels startup_task first, so waiting for it to complete would deadlock. - The sequence is wrapped in try/finally so the completion event is set even when an inner step raises. With that in place the closure workaround in __main__.py collapses to a plain coresys.create_task(stop_supervisor()): repeat SIGTERMs spawn extra tasks but each just observes the in-flight shutdown and waits. Tests cover the four state branches and confirm the event is reset between repeated shutdown cycles (backup restore re-enters RUNNING). * Split Core.shutdown() into teardown_services + shutdown PR feedback (@mdegat01) flagged the "supports repeated use" comment on _shutdown_event.clear() as describing a use case that does not exist. Investigating, the real source of confusion is that the old shutdown() did two different things stitched together: - Stop user-facing containers (add-ons + Home Assistant Core), which backup restore uses while leaving the host alone. - Run the full shutdown ceremony (state transition to SHUTDOWN, stop plugins), which only the SIGTERM signal handler and the host reboot/power-off API want. That dispatch was implemented with an asymmetric state transition ("only set SHUTDOWN if state == RUNNING") and a plugin-shutdown gate ("only stop plugins if state in (STOPPING, SHUTDOWN)"). It worked but made the intent of each branch hard to read, broke the reentrancy guard on the restore path (state never reaches SHUTDOWN, so concurrent callers fall through every early return), and forced the misleading "repeated cycles" framing on the event handling. Split into two methods with one job each: - teardown_services(): stop add-ons + Home Assistant Core. Does not change Core state and does not stop plugins. Backup restore calls this directly so HA Core's watchdog stays registered (it only disables on transitions into CLOSING_STATES) and plugins keep running for the restore body to use. - shutdown(): real shutdown ceremony. Unconditionally transitions to SHUTDOWN, calls teardown_services(), then stops plugins. The reentrancy guard (state == SHUTDOWN -> await event) now works correctly because every caller transitions state on entry. One-shot per process lifetime; no clear() needed. Update backups/manager.py:867 to call teardown_services() instead. remove_homeassistant_container moves to teardown_services() since restore is the only caller that passes it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Release shutdown waiters when set_state() is cancelled PR feedback from Copilot: set_state() updates Core._state synchronously (line 84 in core.py) before awaiting _write_run_state(). If the shutdown task is cancelled while awaiting that write, in-memory state is already SHUTDOWN but the function exits before entering the try/finally that sets _shutdown_event. Any concurrent or later shutdown() caller then sees state == SHUTDOWN and blocks forever on _shutdown_event.wait(). Move the set_state(SHUTDOWN) call inside the try so finally always runs and releases waiters. CancelledError still propagates to the caller after finally as expected; we just no longer leak the lock. Add a regression test that simulates cancellation inside _write_run_state() and asserts both that state has moved to SHUTDOWN and that _shutdown_event is set. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>