Ensure homekit never picks a port that another config entry uses (#45433)

homeassistant/components/homekit/__init__.py

@@ -238,12 +238,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
     port = conf[CONF_PORT]
     _LOGGER.debug("Begin setup HomeKit for %s", name)
 
-    # If the previous instance hasn't cleaned up yet
-    # we need to wait a bit
-    if not await hass.async_add_executor_job(port_is_available, port):
-        _LOGGER.warning("The local port %s is in use", port)
-        raise ConfigEntryNotReady
-
     if CONF_ENTRY_INDEX in conf and conf[CONF_ENTRY_INDEX] == 0:
         _LOGGER.debug("Migrating legacy HomeKit data for %s", name)
         hass.async_add_executor_job(
@@ -275,7 +269,16 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
         entry.entry_id,
     )
     zeroconf_instance = await zeroconf.async_get_instance(hass)
-    await hass.async_add_executor_job(homekit.setup, zeroconf_instance)
+
+    # If the previous instance hasn't cleaned up yet
+    # we need to wait a bit
+    try:
+        await hass.async_add_executor_job(homekit.setup, zeroconf_instance)
+    except (OSError, AttributeError) as ex:
+        _LOGGER.warning(
+            "%s could not be setup because the local port %s is in use", name, port
+        )
+        raise ConfigEntryNotReady from ex
 
     undo_listener = entry.add_update_listener(_async_update_listener)
 

homeassistant/components/homekit/config_flow.py

@@ -40,7 +40,7 @@ from .const import (
     VIDEO_CODEC_COPY,
 )
 from .const import DOMAIN  # pylint:disable=unused-import
-from .util import find_next_available_port
+from .util import async_find_next_available_port
 
 CONF_CAMERA_COPY = "camera_copy"
 CONF_INCLUDE_EXCLUDE_MODE = "include_exclude_mode"
@@ -162,7 +162,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         if user_input is not None:
             return self.async_create_entry(title=self.entry_title, data=self.hk_data)
 
-        self.hk_data[CONF_PORT] = await self._async_available_port()
+        self.hk_data[CONF_PORT] = await async_find_next_available_port(
+            self.hass, DEFAULT_CONFIG_FLOW_PORT
+        )
         self.hk_data[CONF_NAME] = self._async_available_name(
             self.hk_data[CONF_HOMEKIT_MODE]
         )
@@ -205,12 +207,6 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
             title=f"{user_input[CONF_NAME]}:{user_input[CONF_PORT]}", data=user_input
         )
 
-    async def _async_available_port(self):
-        """Return an available port the bridge."""
-        return await self.hass.async_add_executor_job(
-            find_next_available_port, DEFAULT_CONFIG_FLOW_PORT
-        )
-
     @callback
     def _async_current_names(self):
         """Return a set of bridge names."""

homeassistant/components/homekit/util.py

@@ -15,6 +15,7 @@ from homeassistant.const import (
     ATTR_CODE,
     ATTR_SUPPORTED_FEATURES,
     CONF_NAME,
+    CONF_PORT,
     CONF_TYPE,
     TEMP_CELSIUS,
 )
@@ -445,7 +446,7 @@ def _get_test_socket():
     return test_socket
 
 
-def port_is_available(port: int):
+def port_is_available(port: int) -> bool:
     """Check to see if a port is available."""
     test_socket = _get_test_socket()
     try:
@@ -456,10 +457,24 @@ def port_is_available(port: int):
     return True
 
 
-def find_next_available_port(start_port: int):
+async def async_find_next_available_port(hass: HomeAssistant, start_port: int) -> int:
+    """Find the next available port not assigned to a config entry."""
+    exclude_ports = set()
+    for entry in hass.config_entries.async_entries(DOMAIN):
+        if CONF_PORT in entry.data:
+            exclude_ports.add(entry.data[CONF_PORT])
+
+    return await hass.async_add_executor_job(
+        _find_next_available_port, start_port, exclude_ports
+    )
+
+
+def _find_next_available_port(start_port: int, exclude_ports: set) -> int:
     """Find the next available port starting with the given port."""
     test_socket = _get_test_socket()
     for port in range(start_port, MAX_PORT):
+        if port in exclude_ports:
+            continue
         try:
             test_socket.bind(("", port))
             return port
@@ -469,7 +484,7 @@ def find_next_available_port(start_port: int):
             continue
 
 
-def pid_is_alive(pid):
+def pid_is_alive(pid) -> bool:
     """Check to see if a process is alive."""
     try:
         os.kill(pid, 0)

tests/components/homekit/test_config_flow.py

@@ -49,7 +49,7 @@ async def test_setup_in_bridge_mode(hass):
     assert result2["step_id"] == "bridge_mode"
 
     with patch(
-        "homeassistant.components.homekit.config_flow.find_next_available_port",
+        "homeassistant.components.homekit.config_flow.async_find_next_available_port",
         return_value=12345,
     ):
         result3 = await hass.config_entries.flow.async_configure(
@@ -108,7 +108,7 @@ async def test_setup_in_accessory_mode(hass):
     assert result2["step_id"] == "accessory_mode"
 
     with patch(
-        "homeassistant.components.homekit.config_flow.find_next_available_port",
+        "homeassistant.components.homekit.config_flow.async_find_next_available_port",
         return_value=12345,
     ):
         result3 = await hass.config_entries.flow.async_configure(
@@ -629,7 +629,7 @@ async def test_converting_bridge_to_accessory_mode(hass):
     assert result2["step_id"] == "bridge_mode"
 
     with patch(
-        "homeassistant.components.homekit.config_flow.find_next_available_port",
+        "homeassistant.components.homekit.config_flow.async_find_next_available_port",
         return_value=12345,
     ):
         result3 = await hass.config_entries.flow.async_configure(

tests/components/homekit/test_homekit.py

@@ -1020,10 +1020,7 @@ async def test_raise_config_entry_not_ready(hass, mock_zeroconf):
     )
     entry.add_to_hass(hass)
 
-    with patch(
+    with patch(f"{PATH_HOMEKIT}.HomeKit.setup", side_effect=OSError):
-        "homeassistant.components.homekit.port_is_available",
-        return_value=False,
-    ):
         assert not await hass.config_entries.async_setup(entry.entry_id)
         await hass.async_block_till_done()
 

tests/components/homekit/test_util.py

@@ -3,6 +3,7 @@ import pytest
 import voluptuous as vol
 
 from homeassistant.components.homekit.const import (
+    BRIDGE_NAME,
     CONF_FEATURE,
     CONF_FEATURE_LIST,
     CONF_LINKED_BATTERY_SENSOR,
@@ -21,11 +22,11 @@ from homeassistant.components.homekit.const import (
     TYPE_VALVE,
 )
 from homeassistant.components.homekit.util import (
+    async_find_next_available_port,
     cleanup_name_for_homekit,
     convert_to_float,
     density_to_air_quality,
     dismiss_setup_message,
-    find_next_available_port,
     format_sw_version,
     port_is_available,
     show_setup_message,
@@ -43,6 +44,7 @@ from homeassistant.const import (
     ATTR_CODE,
     ATTR_SUPPORTED_FEATURES,
     CONF_NAME,
+    CONF_PORT,
     CONF_TYPE,
     STATE_UNKNOWN,
     TEMP_CELSIUS,
@@ -52,7 +54,7 @@ from homeassistant.core import State
 
 from .util import async_init_integration
 
-from tests.common import async_mock_service
+from tests.common import MockConfigEntry, async_mock_service
 
 
 def test_validate_entity_config():
@@ -251,14 +253,30 @@ async def test_dismiss_setup_msg(hass):
 
 async def test_port_is_available(hass):
     """Test we can get an available port and it is actually available."""
-    next_port = await hass.async_add_executor_job(
+    next_port = await async_find_next_available_port(hass, DEFAULT_CONFIG_FLOW_PORT)
-        find_next_available_port, DEFAULT_CONFIG_FLOW_PORT
+
-    )
     assert next_port
 
     assert await hass.async_add_executor_job(port_is_available, next_port)
 
 
+async def test_port_is_available_skips_existing_entries(hass):
+    """Test we can get an available port and it is actually available."""
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        data={CONF_NAME: BRIDGE_NAME, CONF_PORT: DEFAULT_CONFIG_FLOW_PORT},
+        options={},
+    )
+    entry.add_to_hass(hass)
+
+    next_port = await async_find_next_available_port(hass, DEFAULT_CONFIG_FLOW_PORT)
+
+    assert next_port
+    assert next_port != DEFAULT_CONFIG_FLOW_PORT
+
+    assert await hass.async_add_executor_job(port_is_available, next_port)
+
+
 async def test_format_sw_version():
     """Test format_sw_version method."""
     assert format_sw_version("soho+3.6.8+soho-release-rt120+10") == "3.6.8"