Fix async_unload teardown race in scripts#169562
Draft
arturpragacz wants to merge 4 commits intohome-assistant:devfrom
Draft
Fix async_unload teardown race in scripts#169562arturpragacz wants to merge 4 commits intohome-assistant:devfrom
arturpragacz wants to merge 4 commits intohome-assistant:devfrom
Conversation
Contributor
|
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Contributor
|
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Contributor
|
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Contributor
|
Hey there @ntilley905, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a teardown race in homeassistant.helpers.script.Script by making async_unload a coroutine that fences off new runs before draining in-flight runs and cleaning up resources.
Changes:
- Convert
Script.async_unloadtoasync def, set_unloaded = Truebefore stopping, and move shutdown fencing earlier inasync_run. - Update core call sites to
await script.async_unload()where scripts are torn down. - Adjust and extend tests to reflect the new unload semantics (including unloading while a run is in-flight).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/helpers/script.py | Makes script unloading async, blocks new runs before draining, and introduces _async_unload for synchronous teardown. |
| homeassistant/components/websocket_api/commands.py | Awaits script unload in websocket “execute_script” cleanup. |
| homeassistant/components/wake_on_lan/switch.py | Awaits unload of the WOL off-script during entity removal. |
| homeassistant/components/script/init.py | Awaits unload of script entities when removed. |
| homeassistant/components/intent_script/init.py | Switches intent action teardown to a single awaited unload. |
| homeassistant/components/automation/init.py | Awaits unload of automation action script on entity removal. |
| tests/helpers/test_script.py | Updates unload-related tests and adds coverage for unloading while running. |
| tests/components/wake_on_lan/test_switch.py | Updates expectations around script stop/unload during entity removal. |
emontnemery
reviewed
May 4, 2026
Comment on lines
+1796
to
+1801
| if DATA_NEW_SCRIPT_RUNS_NOT_ALLOWED in self._hass.data: | ||
| self._log("Home Assistant is shutting down, starting script blocked") | ||
| return None | ||
| # The fences above rely on there being no await between these checks | ||
| # and the _runs.append below, so that setting either flag is | ||
| # sufficient to block new runs from being added. |
Contributor
There was a problem hiding this comment.
For context: Moved because we should check this before checking the Context
Comment on lines
+1924
to
+1925
| if self._unloaded: | ||
| return |
Contributor
There was a problem hiding this comment.
Why do we need to allow unload more than once?
Contributor
There was a problem hiding this comment.
The race fixed by this PR should be asserted by test(s).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Fix
async_unloadteardown race in scripts.Script.async_unloadcould race:async_stopsnapshots_runsonce, so a freshasync_run()slipping in during the await left_runsnon-empty, raisedRuntimeError, and leaked the script inhass.data[DATA_SCRIPTS].To fix, make
async_unloadasync and set_unloaded = Truebefore draining, fencing off new runs at the existingasync_runguard.async_unloadwas added in #169036.Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: