Ensure config entries are not unloaded while their platforms are sett…

…ing up (home-assistant#118767) * Report non-awaited/non-locked config entry platform forwards Its currently possible for config entries to be reloaded while their platforms are being forwarded if platform forwards are not awaited or done after the config entry is setup since the lock will not be held in this case. In https://developers.home-assistant.io/blog/2022/07/08/config_entry_forwards we advised to await platform forwards to ensure this does not happen, however for sleeping devices and late discovered devices, platform forwards may happen later. If config platform forwards are happening during setup, they should be awaited If config entry platform forwards are not happening during setup, instead async_late_forward_entry_setups should be used which will hold the lock to prevent the config entry from being unloaded while its platforms are being setup * Report non-awaited/non-locked config entry platform forwards Its currently possible for config entries to be reloaded while their platforms are being forwarded if platform forwards are not awaited or done after the config entry is setup since the lock will not be held in this case. In https://developers.home-assistant.io/blog/2022/07/08/config_entry_forwards we advised to await platform forwards to ensure this does not happen, however for sleeping devices and late discovered devices, platform forwards may happen later. If config platform forwards are happening during setup, they should be awaited If config entry platform forwards are not happening during setup, instead async_late_forward_entry_setups should be used which will hold the lock to prevent the config entry from being unloaded while its platforms are being setup * run with error on to find them * cert_exp, hold lock * cert_exp, hold lock * shelly async_late_forward_entry_setups * compact * compact * found another * patch up mobileapp * patch up hue tests * patch up smartthings * fix mqtt * fix esphome * zwave_js * mqtt * rework * fixes * fix mocking * fix mocking * do not call async_forward_entry_setup directly * docstrings * docstrings * docstrings * add comments * doc strings * fixed all in core, turn off strict * coverage * coverage * missing * coverage
carasen12 · Jun 5, 2024 · ed0568c · ed0568c
1 parent 67b3be8
commit ed0568c
Show file tree

Hide file tree

Showing 47 changed files with 457 additions and 111 deletions.
diff --git a/.coveragerc b/.coveragerc
@@ -1556,6 +1556,7 @@ omit =
     homeassistant/components/verisure/sensor.py
     homeassistant/components/verisure/switch.py
     homeassistant/components/versasense/*
+    homeassistant/components/vesync/__init__.py
     homeassistant/components/vesync/fan.py
     homeassistant/components/vesync/light.py
     homeassistant/components/vesync/sensor.py

diff --git a/homeassistant/components/ambient_station/__init__.py b/homeassistant/components/ambient_station/__init__.py
@@ -182,7 +182,7 @@ def on_subscribed(data: dict) -> None:
             # already been done):
             if not self._entry_setup_complete:
                 self._hass.async_create_task(
-                    self._hass.config_entries.async_forward_entry_setups(
+                    self._hass.config_entries.async_late_forward_entry_setups(
                         self._entry, PLATFORMS
                     ),
                     eager_start=True,

diff --git a/homeassistant/components/cert_expiry/__init__.py b/homeassistant/components/cert_expiry/__init__.py
@@ -28,7 +28,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: CertExpiryConfigEntry) -
 
     async def _async_finish_startup(_: HomeAssistant) -> None:
         await coordinator.async_refresh()
-        await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
+        await hass.config_entries.async_late_forward_entry_setups(entry, PLATFORMS)
 
     async_at_started(hass, _async_finish_startup)
     return True

diff --git a/homeassistant/components/esphome/entry_data.py b/homeassistant/components/esphome/entry_data.py
@@ -244,15 +244,29 @@ def async_update_entity_infos(self, static_infos: Iterable[EntityInfo]) -> None:
                 callback_(static_info)
 
     async def _ensure_platforms_loaded(
-        self, hass: HomeAssistant, entry: ConfigEntry, platforms: set[Platform]
+        self,
+        hass: HomeAssistant,
+        entry: ConfigEntry,
+        platforms: set[Platform],
+        late: bool,
     ) -> None:
         async with self.platform_load_lock:
             if needed := platforms - self.loaded_platforms:
-                await hass.config_entries.async_forward_entry_setups(entry, needed)
+                if late:
+                    await hass.config_entries.async_late_forward_entry_setups(
+                        entry, needed
+                    )
+                else:
+                    await hass.config_entries.async_forward_entry_setups(entry, needed)
             self.loaded_platforms |= needed
 
     async def async_update_static_infos(
-        self, hass: HomeAssistant, entry: ConfigEntry, infos: list[EntityInfo], mac: str
+        self,
+        hass: HomeAssistant,
+        entry: ConfigEntry,
+        infos: list[EntityInfo],
+        mac: str,
+        late: bool = False,
     ) -> None:
         """Distribute an update of static infos to all platforms."""
         # First, load all platforms
@@ -282,7 +296,7 @@ async def async_update_static_infos(
             ):
                 ent_reg.async_update_entity(old_entry, new_unique_id=new_unique_id)
 
-        await self._ensure_platforms_loaded(hass, entry, needed_platforms)
+        await self._ensure_platforms_loaded(hass, entry, needed_platforms, late)
 
         # Make a dict of the EntityInfo by type and send
         # them to the listeners for each specific EntityInfo type

diff --git a/homeassistant/components/esphome/manager.py b/homeassistant/components/esphome/manager.py
@@ -491,7 +491,7 @@ async def _on_connnect(self) -> None:
 
         entry_data.async_update_device_state()
         await entry_data.async_update_static_infos(
-            hass, entry, entity_infos, device_info.mac_address
+            hass, entry, entity_infos, device_info.mac_address, late=True
         )
         _setup_services(hass, entry_data, services)
 

diff --git a/homeassistant/components/knx/__init__.py b/homeassistant/components/knx/__init__.py
@@ -191,15 +191,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
                 create_knx_exposure(hass, knx_module.xknx, expose_config)
             )
     # always forward sensor for system entities (telegram counter, etc.)
-    await hass.config_entries.async_forward_entry_setup(entry, Platform.SENSOR)
-    await hass.config_entries.async_forward_entry_setups(
-        entry,
-        [
-            platform
-            for platform in SUPPORTED_PLATFORMS
-            if platform in config and platform is not Platform.SENSOR
-        ],
-    )
+    platforms = {platform for platform in SUPPORTED_PLATFORMS if platform in config}
+    platforms.add(Platform.SENSOR)
+    await hass.config_entries.async_forward_entry_setups(entry, platforms)
 
     # set up notify service for backwards compatibility - remove 2024.11
     if NotifySchema.PLATFORM in config:

diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py
@@ -379,7 +379,9 @@ async def _reload_config(call: ServiceCall) -> None:
         new_config: list[ConfigType] = config_yaml.get(DOMAIN, [])
         platforms_used = platforms_from_config(new_config)
         new_platforms = platforms_used - mqtt_data.platforms_loaded
-        await async_forward_entry_setup_and_setup_discovery(hass, entry, new_platforms)
+        await async_forward_entry_setup_and_setup_discovery(
+            hass, entry, new_platforms, late=True
+        )
         # Check the schema before continuing reload
         await async_check_config_schema(hass, config_yaml)
 

diff --git a/homeassistant/components/mqtt/discovery.py b/homeassistant/components/mqtt/discovery.py
@@ -211,7 +211,7 @@ async def _async_component_setup(
         async with platform_setup_lock.setdefault(component, asyncio.Lock()):
             if component not in mqtt_data.platforms_loaded:
                 await async_forward_entry_setup_and_setup_discovery(
-                    hass, config_entry, {component}
+                    hass, config_entry, {component}, late=True
                 )
         _async_add_component(discovery_payload)
 

diff --git a/homeassistant/components/mqtt/util.py b/homeassistant/components/mqtt/util.py
@@ -47,7 +47,10 @@ def platforms_from_config(config: list[ConfigType]) -> set[Platform | str]:
 
 
 async def async_forward_entry_setup_and_setup_discovery(
-    hass: HomeAssistant, config_entry: ConfigEntry, platforms: set[Platform | str]
+    hass: HomeAssistant,
+    config_entry: ConfigEntry,
+    platforms: set[Platform | str],
+    late: bool = False,
 ) -> None:
     """Forward the config entry setup to the platforms and set up discovery."""
     mqtt_data = hass.data[DATA_MQTT]
@@ -69,13 +72,11 @@ async def async_forward_entry_setup_and_setup_discovery(
 
         tasks.append(create_eager_task(tag.async_setup_entry(hass, config_entry)))
     if new_entity_platforms := (new_platforms - {"tag", "device_automation"}):
-        tasks.append(
-            create_eager_task(
-                hass.config_entries.async_forward_entry_setups(
-                    config_entry, new_entity_platforms
-                )
-            )
-        )
+        if late:
+            coro = hass.config_entries.async_late_forward_entry_setups
+        else:
+            coro = hass.config_entries.async_forward_entry_setups
+        tasks.append(create_eager_task(coro(config_entry, new_entity_platforms)))
     if not tasks:
         return
     await asyncio.gather(*tasks)

diff --git a/homeassistant/components/point/__init__.py b/homeassistant/components/point/__init__.py
@@ -205,8 +205,8 @@ async def new_device(device_id, platform):
             config_entries_key = f"{platform}.{DOMAIN}"
             async with self._hass.data[DATA_CONFIG_ENTRY_LOCK]:
                 if config_entries_key not in self._hass.data[CONFIG_ENTRY_IS_SETUP]:
-                    await self._hass.config_entries.async_forward_entry_setup(
-                        self._config_entry, platform
+                    await self._hass.config_entries.async_forward_entry_setups(
+                        self._config_entry, [platform]
                     )
                     self._hass.data[CONFIG_ENTRY_IS_SETUP].add(config_entries_key)
 

diff --git a/homeassistant/components/shelly/coordinator.py b/homeassistant/components/shelly/coordinator.py
@@ -200,7 +200,9 @@ async def _async_device_connect_task(self) -> bool:
             self.hass.config_entries.async_update_entry(self.entry, data=data)
 
         # Resume platform setup
-        await self.hass.config_entries.async_forward_entry_setups(self.entry, platforms)
+        await self.hass.config_entries.async_late_forward_entry_setups(
+            self.entry, platforms
+        )
 
         return True
 

diff --git a/homeassistant/components/tellduslive/__init__.py b/homeassistant/components/tellduslive/__init__.py
@@ -180,8 +180,8 @@ async def _discover(self, device_id):
         )
         async with self._hass.data[DATA_CONFIG_ENTRY_LOCK]:
             if component not in self._hass.data[CONFIG_ENTRY_IS_SETUP]:
-                await self._hass.config_entries.async_forward_entry_setup(
-                    self._config_entry, component
+                await self._hass.config_entries.async_forward_entry_setups(
+                    self._config_entry, [component]
                 )
                 self._hass.data[CONFIG_ENTRY_IS_SETUP].add(component)
         device_ids = []

diff --git a/homeassistant/components/vesync/__init__.py b/homeassistant/components/vesync/__init__.py
@@ -46,7 +46,7 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b
 
     device_dict = await async_process_devices(hass, manager)
 
-    forward_setup = hass.config_entries.async_forward_entry_setup
+    forward_setups = hass.config_entries.async_forward_entry_setups
 
     hass.data[DOMAIN] = {}
     hass.data[DOMAIN][VS_MANAGER] = manager
@@ -97,7 +97,7 @@ async def async_new_device_discovery(service: ServiceCall) -> None:
             return
         if new_switches and not switches:
             switches.extend(new_switches)
-            hass.async_create_task(forward_setup(config_entry, Platform.SWITCH))
+            hass.async_create_task(forward_setups(config_entry, [Platform.SWITCH]))
 
         fan_set = set(fan_devs)
         new_fans = list(fan_set.difference(fans))
@@ -107,7 +107,7 @@ async def async_new_device_discovery(service: ServiceCall) -> None:
             return
         if new_fans and not fans:
             fans.extend(new_fans)
-            hass.async_create_task(forward_setup(config_entry, Platform.FAN))
+            hass.async_create_task(forward_setups(config_entry, [Platform.FAN]))
 
         light_set = set(light_devs)
         new_lights = list(light_set.difference(lights))
@@ -117,7 +117,7 @@ async def async_new_device_discovery(service: ServiceCall) -> None:
             return
         if new_lights and not lights:
             lights.extend(new_lights)
-            hass.async_create_task(forward_setup(config_entry, Platform.LIGHT))
+            hass.async_create_task(forward_setups(config_entry, [Platform.LIGHT]))
 
         sensor_set = set(sensor_devs)
         new_sensors = list(sensor_set.difference(sensors))
@@ -127,7 +127,7 @@ async def async_new_device_discovery(service: ServiceCall) -> None:
             return
         if new_sensors and not sensors:
             sensors.extend(new_sensors)
-            hass.async_create_task(forward_setup(config_entry, Platform.SENSOR))
+            hass.async_create_task(forward_setups(config_entry, [Platform.SENSOR]))
 
     hass.services.async_register(
         DOMAIN, SERVICE_UPDATE_DEVS, async_new_device_discovery

diff --git a/homeassistant/components/zwave_js/__init__.py b/homeassistant/components/zwave_js/__init__.py
@@ -324,8 +324,8 @@ async def async_setup_platform(self, platform: Platform) -> None:
         """Set up platform if needed."""
         if platform not in self.platform_setup_tasks:
             self.platform_setup_tasks[platform] = self.hass.async_create_task(
-                self.hass.config_entries.async_forward_entry_setup(
-                    self.config_entry, platform
+                self.hass.config_entries.async_late_forward_entry_setups(
+                    self.config_entry, [platform]
                 )
             )
         await self.platform_setup_tasks[platform]

diff --git a/homeassistant/config_entries.py b/homeassistant/config_entries.py
@@ -1178,6 +1178,24 @@ class FlowCancelledError(Exception):
     """Error to indicate that a flow has been cancelled."""
 
 
+def _report_non_locked_platform_forwards(entry: ConfigEntry) -> None:
+    """Report non awaited and non-locked platform forwards."""
+    report(
+        f"calls async_forward_entry_setup after the entry for "
+        f"integration, {entry.domain} with title: {entry.title} "
+        f"and entry_id: {entry.entry_id}, has been set up, "
+        "without holding the setup lock that prevents the config "
+        "entry from being set up multiple times. "
+        "Instead await hass.config_entries.async_forward_entry_setup "
+        "during setup of the config entry or call "
+        "hass.config_entries.async_late_forward_entry_setups "
+        "in a tracked task. "
+        "This will stop working in Home Assistant 2025.1",
+        error_if_integration=False,
+        error_if_core=False,
+    )
+
+
 class ConfigEntriesFlowManager(data_entry_flow.FlowManager[ConfigFlowResult]):
     """Manage all the config entry flows that are in progress."""
 
@@ -2024,15 +2042,32 @@ def _async_dispatch(
     async def async_forward_entry_setups(
         self, entry: ConfigEntry, platforms: Iterable[Platform | str]
     ) -> None:
-        """Forward the setup of an entry to platforms."""
+        """Forward the setup of an entry to platforms.
+
+        This method should be awaited before async_setup_entry is finished
+        in each integration. This is to ensure that all platforms are loaded
+        before the entry is set up. This ensures that the config entry cannot
+        be unloaded before all platforms are loaded.
+
+        If platforms must be loaded late (after the config entry is setup),
+        use async_late_forward_entry_setup instead.
+
+        This method is more efficient than async_forward_entry_setup as
+        it can load multiple platforms at once and does not require a separate
+        import executor job for each platform.
+        """
         integration = await loader.async_get_integration(self.hass, entry.domain)
         if not integration.platforms_are_loaded(platforms):
             with async_pause_setup(self.hass, SetupPhases.WAIT_IMPORT_PLATFORMS):
                 await integration.async_get_platforms(platforms)
+        if non_locked_platform_forwards := not entry.setup_lock.locked():
+            _report_non_locked_platform_forwards(entry)
         await asyncio.gather(
             *(
                 create_eager_task(
-                    self._async_forward_entry_setup(entry, platform, False),
+                    self._async_forward_entry_setup(
+                        entry, platform, False, non_locked_platform_forwards
+                    ),
                     name=(
                         f"config entry forward setup {entry.title} "
                         f"{entry.domain} {entry.entry_id} {platform}"
@@ -2043,6 +2078,25 @@ async def async_forward_entry_setups(
             )
         )
 
+    async def async_late_forward_entry_setups(
+        self, entry: ConfigEntry, platforms: Iterable[Platform | str]
+    ) -> None:
+        """Forward the setup of an entry to platforms after setup.
+
+        If platforms must be loaded late (after the config entry is setup),
+        use this method instead of async_forward_entry_setups as it holds
+        the setup lock until the platforms are loaded to ensure that the
+        config entry cannot be unloaded while platforms are loaded.
+        """
+        async with entry.setup_lock:
+            if entry.state is not ConfigEntryState.LOADED:
+                raise OperationNotAllowed(
+                    f"The config entry {entry.title} ({entry.domain}) with entry_id"
+                    f" {entry.entry_id} cannot forward setup for {platforms} "
+                    f"because it is not loaded in the {entry.state} state"
+                )
+            await self.async_forward_entry_setups(entry, platforms)
+
     async def async_forward_entry_setup(
         self, entry: ConfigEntry, domain: Platform | str
     ) -> bool:
@@ -2051,11 +2105,38 @@ async def async_forward_entry_setup(
         By default an entry is setup with the component it belongs to. If that
         component also has related platforms, the component will have to
         forward the entry to be setup by that component.
+
+        This method is deprecated and will stop working in Home Assistant 2025.6.
+
+        Instead, await async_forward_entry_setups as it can load
+        multiple platforms at once and is more efficient since it
+        does not require a separate import executor job for each platform.
+
+        If platforms must be loaded late (after the config entry is setup),
+        use async_late_forward_entry_setup instead.
         """
-        return await self._async_forward_entry_setup(entry, domain, True)
+        if non_locked_platform_forwards := not entry.setup_lock.locked():
+            _report_non_locked_platform_forwards(entry)
+        else:
+            report(
+                "calls async_forward_entry_setup for "
+                f"integration, {entry.domain} with title: {entry.title} "
+                f"and entry_id: {entry.entry_id}, which is deprecated and "
+                "will stop working in Home Assistant 2025.6, "
+                "await async_forward_entry_setups instead",
+                error_if_core=False,
+                error_if_integration=False,
+            )
+        return await self._async_forward_entry_setup(
+            entry, domain, True, non_locked_platform_forwards
+        )
 
     async def _async_forward_entry_setup(
-        self, entry: ConfigEntry, domain: Platform | str, preload_platform: bool
+        self,
+        entry: ConfigEntry,
+        domain: Platform | str,
+        preload_platform: bool,
+        non_locked_platform_forwards: bool,
     ) -> bool:
         """Forward the setup of an entry to a different component."""
         # Setup Component if not set up yet
@@ -2079,6 +2160,12 @@ async def _async_forward_entry_setup(
 
         integration = loader.async_get_loaded_integration(self.hass, domain)
         await entry.async_setup(self.hass, integration=integration)
+
+        # Check again after setup to make sure the lock
+        # is still there because it could have been released
+        # unless we already reported it.
+        if not non_locked_platform_forwards and not entry.setup_lock.locked():
+            _report_non_locked_platform_forwards(entry)
         return True
 
     async def async_unload_platforms(
@@ -2104,7 +2191,11 @@ async def async_unload_platforms(
     async def async_forward_entry_unload(
         self, entry: ConfigEntry, domain: Platform | str
     ) -> bool:
-        """Forward the unloading of an entry to a different component."""
+        """Forward the unloading of an entry to a different component.
+
+        Its is preferred to call async_unload_platforms instead
+        of directly calling this method.
+        """
         # It was never loaded.
         if domain not in self.hass.config.components:
             return True

diff --git a/tests/components/alarm_control_panel/conftest.py b/tests/components/alarm_control_panel/conftest.py
@@ -155,8 +155,8 @@ async def async_setup_entry_init(
         hass: HomeAssistant, config_entry: ConfigEntry
     ) -> bool:
         """Set up test config entry."""
-        await hass.config_entries.async_forward_entry_setup(
-            config_entry, ALARM_CONTROL_PANEL_DOMAIN
+        await hass.config_entries.async_forward_entry_setups(
+            config_entry, [ALARM_CONTROL_PANEL_DOMAIN]
         )
         return True