8000 Hold a lock to prevent concurrent setup of config entries by bdraco · Pull Request #116482 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Hold a lock to prevent concurrent setup of config entries #116482

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Apr 30, 2024
Merged

Hold a lock to prevent concurrent setup of config entries #116482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4a56c24
Hold a lock to prevent concurrent setup of config entries
bdraco Apr 30, 2024
327dfc4
fix test
bdraco Apr 30, 2024
acfca83
fix some mocking
bdraco Apr 30, 2024
bfec4fa
coverage
bdraco Apr 30, 2024
2a035af
Merge branch 'dev' into hold_lock_to_prevent_concurrent_setup
bdraco Apr 30, 2024
84b7928
more cover
bdraco Apr 30, 2024
0557bf7
Merge remote-tracking branch 'upstream/hold_lock_to_prevent_concurren…
bdraco Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions homeassistant/config_entries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ class ConfigEntry(Generic[_DataT]):
update_listeners: list[UpdateListenerType]
_async_cancel_retry_setup: Callable[[], Any] | None
_on_unload: list[Callable[[], Coroutine[Any, Any, None] | None]] | None
reload_lock: asyncio.Lock
setup_lock: asyncio.Lock
_reauth_lock: asyncio.Lock
_reconfigure_lock: asyncio.Lock
_tasks: set[asyncio.Future[Any]]
Expand Down Expand Up @@ -403,7 +403,7 @@ def __init__(
_setter(self, "_on_unload", None)

# Reload lock to prevent conflicting reloads
_setter(self, "reload_lock", asyncio.Lock())
_setter(self, "setup_lock", asyncio.Lock())
# Reauth lock to prevent concurrent reauth flows
_setter(self, "_reauth_lock", asyncio.Lock())
# Reconfigure lock to prevent concurrent reconfigure flows
Expand Down Expand Up @@ -702,19 +702,17 @@ def _async_setup_again(self, hass: HomeAssistant, *_: Any) -> None:
# has started so we do not block shutdown
if not hass.is_stopping:
hass.async_create_background_task(
self._async_setup_retry(hass),
self.async_setup_locked(hass),
f"config entry retry {self.domain} {self.title}",
eager_start=True,
)

async def _async_setup_retry(self, hass: HomeAssistant) -> None:
"""Retry setup.

We hold the reload lock during setup retry to ensure
that nothing can reload the entry while we are retrying.
"""
async with self.reload_lock:
await self.async_setup(hass)
async def async_setup_locked(
self, hass: HomeAssistant, integration: loader.Integration | None = None
) -> None:
"""Set up while holding the setup lock."""
async with self.setup_lock:
await self.async_setup(hass, integration=integration)

@callback
def async_shutdown(self) -> None:
Expand Down Expand Up @@ -1794,7 +1792,15 @@ async def async_reload(self, entry_id: str) -> bool:
# attempts.
entry.async_cancel_retry_setup()

async with entry.reload_lock:
if entry.domain not in self.hass.config.components:
# If the component is not loaded, just load it as
# the config entry will be loaded as well. We need
# to do this before holding the lock to avoid a
# deadlock.
await async_setup_component(self.hass, entry.domain, self._hass_config)
return entry.state is ConfigEntryState.LOADED
Comment on lines +1795 to +1801
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly what self.async_setup(entry_id) does, but we need to do it without holding the lock since async_setup_component will hold the lock


async with entry.setup_lock:
unload_result = await self.async_unload(entry_id)

if not unload_result or 8000 entry.disabled_by:
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ async def _async_setup_component(
await asyncio.gather(
*(
create_eager_task(
entry.async_setup(hass, integration=integration),
entry.async_setup_locked(hass, integration=integration),
name=f"config entry setup {entry.title} {entry.domain} {entry.entry_id}",
)
for entry in entries
Expand Down
3 changes: 3 additions & 0 deletions tests/components/androidtv_remote/test_config_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ async def test_user_flow_already_configured_host_changed_reloads_entry(
state=ConfigEntryState.LOADED,
)
mock_config_entry.add_to_hass(hass)
hass.config.components.add(DOMAIN)

result = await hass.config_entries.flow.async_init(
DOMAIN, context={"source": config_entries.SOURCE_USER}
Expand Down Expand Up @@ -640,6 +641,7 @@ async def test_zeroconf_flow_already_configured_host_changed_reloads_entry(
state=ConfigEntryState.LOADED,
)
mock_config_entry.add_to_hass(hass)
hass.config.components.add(DOMAIN)

result = await hass.config_entries.flow.async_init(
DOMAIN,
Expand Down Expand Up @@ -769,6 +771,7 @@ async def test_reauth_flow_success(
state=ConfigEntryState.LOADED,
)
mock_config_entry.add_to_hass(hass)
hass.config.components.add(DOMAIN)

mock_config_entry.async_start_reauth(hass)
await hass.async_block_till_done()
Expand Down
5 changes: 5 additions & 0 deletions tests/components/config/test_config_entries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ async def test_reload_entry(hass: HomeAssistant, client) -> None:
domain="kitchen_sink", state=core_ce.ConfigEntryState.LOADED
)
entry.add_to_hass(hass)
hass.config.components.add("kitchen_sink")
resp = await client.post(
f"/api/config/config_entries/entry/{entry.entry_id}/reload"
)
Expand Down Expand Up @@ -298,6 +299,7 @@ async def test_reload_entry_in_failed_state(
"""Test reloading an entry via the API that has already failed to unload."""
entry = MockConfigEntry(domain="demo", state=core_ce.ConfigEntryState.FAILED_UNLOAD)
entry.add_to_hass(hass)
hass.config.components.add("demo")
resp = await client.post(
f"/api/config/config_entries/entry/{entry.entry_id}/reload"
)
Expand Down Expand Up @@ -326,6 +328,7 @@ async def test_reload_entry_in_setup_retry(
entry = MockConfigEntry(domain="comp", state=core_ce.ConfigEntryState.SETUP_RETRY)
entry.supports_unload = True
entry.add_to_hass(hass)
hass.config.components.add("comp")

with patch.dict(HANDLERS, {"comp": ConfigFlow, "test": ConfigFlow}):
resp = await client.post(
Expand Down Expand Up @@ -1109,6 +1112,7 @@ async def test_update_prefrences(
domain="kitchen_sink", state=core_ce.ConfigEntryState.LOADED
)
entry.add_to_hass(hass)
hass.config.components.add("kitchen_sink")

assert entry.pref_disable_new_entities is False
assert entry.pref_disable_polling is False
Expand Down Expand Up @@ -1209,6 +1213,7 @@ async def test_disable_entry(
)
entry.add_to_hass(hass)
assert entry.disabled_by is None
hass.config.components.add("kitchen_sink")

# Disable
await ws_client.send_json(
Expand Down
1 change: 1 addition & 0 deletions tests/components/mqtt/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1873,6 +1873,7 @@ async def test_reload_entry_with_restored_subscriptions(
# Setup the MQTT entry
entry = MockConfigEntry(domain=mqtt.DOMAIN, data={mqtt.CONF_BROKER: "test-broker"})
entry.add_to_hass(hass)
hass.config.components.add(mqtt.DOMAIN)
mqtt_client_mock.connect.return_value = 0
with patch("homeassistant.config.load_yaml_config_file", return_value={}):
await entry.async_setup(hass)
Expand Down
2 changes: 2 additions & 0 deletions tests/components/opower/test_config_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ async def test_form_valid_reauth(
) -> None:
"""Test that we can handle a valid reauth."""
mock_config_entry.mock_state(hass, ConfigEntryState.LOADED)
hass.config.components.add(DOMAIN)
mock_config_entry.async_start_reauth(hass)
await hass.async_block_till_done()

Expand Down Expand Up @@ -328,6 +329,7 @@ async def test_form_valid_reauth_with_mfa(
},
)
mock_config_entry.mock_state(hass, ConfigEntryState.LOADED)
hass.config.components.add(DOMAIN)
mock_config_entry.async_start_reauth(hass)
await hass.async_block_till_done()

Expand Down
104 changes: 99 additions & 5 deletions tests/test_config_entries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,7 @@ async def test_as_dict(snapshot: SnapshotAssertion) -> None:
"error_reason_translation_placeholders",
"_async_cancel_retry_setup",
"_on_unload",
"reload_lock",
"setup_lock",
"_reauth_lock",
"_tasks",
"_background_tasks",
Expand Down Expand Up @@ -1632,7 +1632,6 @@ async def test_entry_reload_succeed(
mock_platform(hass, "comp.config_flow", None)

assert await manager.async_reload(entry.entry_id)
assert len(async_unload_entry.mock_calls) == 1
assert len(async_setup.mock_calls) == 1
assert len(async_setup_entry.mock_calls) == 1
assert entry.state is config_entries.ConfigEntryState.LOADED
Expand Down Expand Up @@ -1707,6 +1706,8 @@ async def test_entry_reload_error(
),
)

hass.config.components.add("comp")

with pytest.raises(config_entries.OperationNotAllowed, match=str(state)):
assert await manager.async_reload(entry.entry_id)

Expand Down Expand Up @@ -1738,8 +1739,11 @@ async def test_entry_disable_succeed(
),
)
mock_platform(hass, "comp.config_flow", None)
hass.config.components.add("comp")

# Disable
assert len(async_setup.mock_calls) == 0
assert len(async_setup_entry.mock_calls) == 0
assert await manager.async_set_disabled_by(
entry.entry_id, config_entries.ConfigEntryDisabler.USER
)
Expand All @@ -1751,7 +1755,7 @@ async def test_entry_disable_succeed(
# Enable
assert await manager.async_set_disabled_by(entry.entry_id, None)
assert len(async_unload_entry.mock_calls) == 1
assert len(async_setup.mock_calls) == 1
assert len(async_setup.mock_calls) == 0
assert len(async_setup_entry.mock_calls) == 1
assert entry.state is config_entries.ConfigEntryState.LOADED

Expand All @@ -1775,6 +1779,7 @@ async def test_entry_disable_without_reload_support(
),
)
mock_platform(hass, "comp.config_flow", None)
hass.config.components.add("comp")

# Disable
assert not await manager.async_set_disabled_by(
Expand Down Expand Up @@ -1951,7 +1956,7 @@ async def test_reload_entry_entity_registry_works(
)
await hass.async_block_till_done()

assert len(mock_unload_entry.mock_calls) == 2
assert len(mock_unload_entry.mock_calls) == 1


async def test_unique_id_persisted(
Expand Down Expand Up @@ -3392,6 +3397,7 @@ async def test_entry_reload_calls_on_unload_listeners(
),
)
mock_platform(hass, "comp.config_flow", None)
hass.config.components.add("comp")

mock_unload_callback = Mock()

Expand Down Expand Up @@ -3944,8 +3950,9 @@ async def test_deprecated_disabled_by_str_set(
caplog: pytest.LogCaptureFixture,
) -> None:
"""Test deprecated str set disabled_by enumizes and logs a warning."""
entry = MockConfigEntry()
entry = MockConfigEntry(domain="comp")
entry.add_to_manager(manager)
hass.config.components.add("comp")
assert await manager.async_set_disabled_by(
entry.entry_id, config_entries.ConfigEntryDisabler.USER.value
)
Expand All @@ -3963,6 +3970,47 @@ async def test_entry_reload_concurrency(
async_setup = AsyncMock(return_value=True)
loaded = 1

async def _async_setup_entry(*args, **kwargs):
await asyncio.sleep(0)
nonlocal loaded
loaded += 1
return loaded == 1

async def _async_unload_entry(*args, **kwargs):
await asyncio.sleep(0)
nonlocal loaded
loaded -= 1
return loaded == 0

mock_integration(
hass,
MockModule(
"comp",
async_setup=async_setup,
async_setup_entry=_async_setup_entry,
async_unload_entry=_async_unload_entry,
),
)
mock_platform(hass, "comp.config_flow", None)
hass.config.components.add("comp")
tasks = [
asyncio.create_task(manager.async_reload(entry.entry_id)) for _ in range(15)
]
await asyncio.gather(*tasks)
assert entry.state is config_entries.ConfigEntryState.LOADED
assert loaded == 1


async def test_entry_reload_concurrency_not_setup_setup(
hass: HomeAssistant, manager: config_entries.ConfigEntries
) -> None:
"""Test multiple reload calls do not cause a reload race."""
entry = MockConfigEntry(domain="comp", state=config_entries.ConfigEntryState.LOADED)
entry.add_to_hass(hass)

async_setup = AsyncMock(return_value=True)
loaded = 0

async def _async_setup_entry(*args, **kwargs):
await asyncio.sleep(0)
nonlocal loaded
Expand Down Expand Up @@ -4074,6 +4122,7 @@ async def test_disallow_entry_reload_with_setup_in_progress(
domain="comp", state=config_entries.ConfigEntryState.SETUP_IN_PROGRESS
)
entry.add_to_hass(hass)
hass.config.components.add("comp")

with pytest.raises(
config_entries.OperationNotAllowed,
Expand Down Expand Up @@ -5016,3 +5065,48 @@ async def test_updating_non_added_entry_raises(hass: HomeAssistant) -> None:

with pytest.raises(config_entries.UnknownEntry, match=entry.entry_id):
hass.config_entries.async_update_entry(entry, unique_id="new_id")


async def test_reload_during_setup(hass: HomeAssistant) -> None:
"""Test reload during setup waits."""
entry = MockConfigEntry(domain="comp", data={"value": "initial"})
entry.add_to_hass(hass)

setup_start_future = hass.loop.create_future()
setup_finish_future = hass.loop.create_future()
in_setup = False
setup_calls = 0

async def mock_async_setup_entry(hass, entry):
"""Mock setting up an entry."""
nonlocal in_setup
nonlocal setup_calls
setup_calls += 1
assert not in_setup
in_setup = True
setup_start_future.set_result(None)
await setup_finish_future
in_setup = False
return True

mock_integration(
hass,
MockModule(
"comp",
async_setup_entry=mock_async_setup_entry,
async_unload_entry=AsyncMock(return_value=True),
),
)
mock_platform(hass, "comp.config_flow", None)

setup_task = hass.async_create_task(async_setup_component(hass, "comp", {}))

await setup_start_future # ensure we are in the setup
reload_task = hass.async_create_task(
hass.config_entries.async_reload(entry.entry_id)
)
await asyncio.sleep(0)
setup_finish_future.set_result(None)
await setup_task
await reload_task
assert setup_calls == 2
0