From ba4120d779c0acea285e89af04af8f76c3d9262f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 29 Feb 2024 09:01:03 -1000 Subject: [PATCH] Fallback to event loop import on deadlock (#111868) --- homeassistant/loader.py | 32 +++++++- tests/test_loader.py | 162 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 189 insertions(+), 5 deletions(-) diff --git a/homeassistant/loader.py b/homeassistant/loader.py index 1bbd22d4070..1ff98ff6ff2 100644 --- a/homeassistant/loader.py +++ b/homeassistant/loader.py @@ -852,7 +852,14 @@ class Integration: # Some integrations fail on import because they call functions incorrectly. # So we do it before validating config to catch these errors. if load_executor: - comp = await self.hass.async_add_executor_job(self.get_component) + try: + comp = await self.hass.async_add_executor_job(self.get_component) + except ImportError as ex: + load_executor = False + _LOGGER.debug("Failed to import %s in executor", domain, exc_info=ex) + # If importing in the executor deadlocks because there is a circular + # dependency, we fall back to the event loop. + comp = self.get_component() else: comp = self.get_component() @@ -885,6 +892,9 @@ class Integration: ) except ImportError: raise + except RuntimeError as err: + # _DeadlockError inherits from RuntimeError + raise ImportError(f"RuntimeError importing {self.pkg_path}: {err}") from err except Exception as err: _LOGGER.exception( "Unexpected exception importing component %s", self.pkg_path @@ -913,9 +923,18 @@ class Integration: ) try: if load_executor: - platform = await self.hass.async_add_executor_job( - self._load_platform, platform_name - ) + try: + platform = await self.hass.async_add_executor_job( + self._load_platform, platform_name + ) + except ImportError as ex: + _LOGGER.debug( + "Failed to import %s in executor", domain, exc_info=ex + ) + load_executor = False + # If importing in the executor deadlocks because there is a circular + # dependency, we fall back to the event loop. + platform = self._load_platform(platform_name) else: platform = self._load_platform(platform_name) import_future.set_result(platform) @@ -983,6 +1002,11 @@ class Integration: ] missing_platforms_cache[full_name] = ex raise + except RuntimeError as err: + # _DeadlockError inherits from RuntimeError + raise ImportError( + f"RuntimeError importing {self.pkg_path}.{platform_name}: {err}" + ) from err except Exception as err: _LOGGER.exception( "Unexpected exception importing platform %s.%s", diff --git a/tests/test_loader.py b/tests/test_loader.py index d173e3e8aa6..babe1abcdd2 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -1,6 +1,8 @@ """Test to verify that we can load components.""" import asyncio -from unittest.mock import Mock, patch +import sys +from typing import Any +from unittest.mock import MagicMock, Mock, patch import pytest @@ -1033,3 +1035,161 @@ async def test_hass_components_use_reported( "Detected that custom integration 'test_integration_frame'" " accesses hass.components.http. This is deprecated" ) in caplog.text + + +async def test_async_get_component_deadlock_fallback( + hass: HomeAssistant, caplog: pytest.LogCaptureFixture +) -> None: + """Verify async_get_component fallback to importing in the event loop on deadlock.""" + executor_import_integration = _get_test_integration( + hass, "executor_import", True, import_executor=True + ) + assert executor_import_integration.import_executor is True + module_mock = MagicMock() + import_attempts = 0 + + def mock_import(module: str, *args: Any, **kwargs: Any) -> Any: + nonlocal import_attempts + if module == "homeassistant.components.executor_import": + import_attempts += 1 + + if import_attempts == 1: + # _DeadlockError inherits from RuntimeError + raise RuntimeError( + "Detected deadlock trying to import homeassistant.components.executor_import" + ) + + return module_mock + + assert "homeassistant.components.executor_import" not in sys.modules + assert "custom_components.executor_import" not in sys.modules + with patch("homeassistant.loader.importlib.import_module", mock_import): + module = await executor_import_integration.async_get_component() + + assert ( + "Detected deadlock trying to import homeassistant.components.executor_import" + in caplog.text + ) + assert "loaded_executor=False" in caplog.text + assert module is module_mock + + +async def test_async_get_component_raises_after_import_failure( + hass: HomeAssistant, caplog: pytest.LogCaptureFixture +) -> None: + """Verify async_get_component raises if we fail to import in both the executor and loop.""" + executor_import_integration = _get_test_integration( + hass, "executor_import", True, import_executor=True + ) + assert executor_import_integration.import_executor is True + module_mock = MagicMock() + import_attempts = 0 + + def mock_import(module: str, *args: Any, **kwargs: Any) -> Any: + nonlocal import_attempts + if module == "homeassistant.components.executor_import": + import_attempts += 1 + + if import_attempts == 1: + # _DeadlockError inherits from RuntimeError + raise RuntimeError( + "Detected deadlock trying to import homeassistant.components.executor_import" + ) + + if import_attempts == 2: + raise ImportError("Failed import homeassistant.components.executor_import") + return module_mock + + assert "homeassistant.components.executor_import" not in sys.modules + assert "custom_components.executor_import" not in sys.modules + with patch( + "homeassistant.loader.importlib.import_module", mock_import + ), pytest.raises(ImportError): + await executor_import_integration.async_get_component() + + assert ( + "Detected deadlock trying to import homeassistant.components.executor_import" + in caplog.text + ) + assert "loaded_executor=False" not in caplog.text + + +async def test_async_get_platform_deadlock_fallback( + hass: HomeAssistant, caplog: pytest.LogCaptureFixture +) -> None: + """Verify async_get_platform fallback to importing in the event loop on deadlock.""" + executor_import_integration = _get_test_integration( + hass, "executor_import", True, import_executor=True + ) + assert executor_import_integration.import_executor is True + module_mock = MagicMock() + import_attempts = 0 + + def mock_import(module: str, *args: Any, **kwargs: Any) -> Any: + nonlocal import_attempts + if module == "homeassistant.components.executor_import.config_flow": + import_attempts += 1 + + if import_attempts == 1: + # _DeadlockError inherits from RuntimeError + raise RuntimeError( + "Detected deadlock trying to import homeassistant.components.executor_import" + ) + + return module_mock + + assert "homeassistant.components.executor_import" not in sys.modules + assert "custom_components.executor_import" not in sys.modules + with patch("homeassistant.loader.importlib.import_module", mock_import): + module = await executor_import_integration.async_get_platform("config_flow") + + assert ( + "Detected deadlock trying to import homeassistant.components.executor_import" + in caplog.text + ) + assert "loaded_executor=False" in caplog.text + assert module is module_mock + + +async def test_async_get_platform_raises_after_import_failure( + hass: HomeAssistant, caplog: pytest.LogCaptureFixture +) -> None: + """Verify async_get_platform raises if we fail to import in both the executor and loop.""" + executor_import_integration = _get_test_integration( + hass, "executor_import", True, import_executor=True + ) + assert executor_import_integration.import_executor is True + module_mock = MagicMock() + import_attempts = 0 + + def mock_import(module: str, *args: Any, **kwargs: Any) -> Any: + nonlocal import_attempts + if module == "homeassistant.components.executor_import.config_flow": + import_attempts += 1 + + if import_attempts == 1: + # _DeadlockError inherits from RuntimeError + raise RuntimeError( + "Detected deadlock trying to import homeassistant.components.executor_import" + ) + + if import_attempts == 2: + # _DeadlockError inherits from RuntimeError + raise ImportError( + "Error trying to import homeassistant.components.executor_import" + ) + + return module_mock + + assert "homeassistant.components.executor_import" not in sys.modules + assert "custom_components.executor_import" not in sys.modules + with patch( + "homeassistant.loader.importlib.import_module", mock_import + ), pytest.raises(ImportError): + await executor_import_integration.async_get_platform("config_flow") + + assert ( + "Detected deadlock trying to import homeassistant.components.executor_import" + in caplog.text + ) + assert "loaded_executor=False" not in caplog.text