Replace __del__ with explicit disconnect (#3892)

supervisor/dbus/interface.py

@@ -35,10 +35,6 @@ class DBusInterface(ABC):
         """Return True, if they is connected to D-Bus."""
         return self.dbus is not None
 
-    def __del__(self) -> None:
-        """Disconnect on delete."""
-        self.disconnect()
-
     async def connect(self, bus: MessageBus) -> None:
         """Connect to D-Bus."""
         self.dbus = await DBus.connect(bus, self.bus_name, self.object_path)

supervisor/dbus/network/__init__.py

@@ -1,4 +1,5 @@
 """Network Manager implementation for DBUS."""
+import asyncio
 import logging
 from typing import Any
 
@@ -210,6 +211,14 @@ class NetworkManager(DBusInterfaceProxy):
 
             interfaces[interface.name] = interface
 
+        # Disconnect removed devices
+        for device in set(curr_devices.keys()) - set(
+            self.properties[DBUS_ATTR_DEVICES]
+        ):
+            asyncio.get_event_loop().run_in_executor(
+                None, curr_devices[device].disconnect
+            )
+
         self._interfaces = interfaces
 
     def disconnect(self) -> None:

supervisor/dbus/network/connection.py

@@ -1,5 +1,6 @@
 """Connection object for Network Manager."""
 
+import asyncio
 from typing import Any
 
 from supervisor.dbus.network.setting import NetworkSetting
@@ -77,16 +78,40 @@ class NetworkConnection(DBusInterfaceProxy):
         """Return settings."""
         return self._settings
 
+    @settings.setter
+    def settings(self, settings: NetworkSetting | None) -> None:
+        """Set settings."""
+        if self._settings and self._settings is not settings:
+            asyncio.get_event_loop().run_in_executor(None, self._settings.disconnect)
+
+        self._settings = settings
+
     @property
     def ipv4(self) -> IpConfiguration | None:
         """Return a ip4 configuration object for the connection."""
         return self._ipv4
 
+    @ipv4.setter
+    def ipv4(self, ipv4: IpConfiguration | None) -> None:
+        """Set ipv4 configuration."""
+        if self._ipv4 and self._ipv4 is not ipv4:
+            asyncio.get_event_loop().run_in_executor(None, self._ipv4.disconnect)
+
+        self._ipv4 = ipv4
+
     @property
     def ipv6(self) -> IpConfiguration | None:
         """Return a ip6 configuration object for the connection."""
         return self._ipv6
 
+    @ipv6.setter
+    def ipv6(self, ipv6: IpConfiguration | None) -> None:
+        """Set ipv6 configuration."""
+        if self._ipv6 and self._ipv6 is not ipv6:
+            asyncio.get_event_loop().run_in_executor(None, self._ipv6.disconnect)
+
+        self._ipv6 = ipv6
+
     @dbus_connected
     async def update(self, changed: dict[str, Any] | None = None) -> None:
         """Update connection information."""
@@ -102,46 +127,44 @@ class NetworkConnection(DBusInterfaceProxy):
         # IPv4
         if not changed or DBUS_ATTR_IP4CONFIG in changed:
             if (
-                self._ipv4
+                self.ipv4
-                and self._ipv4.is_connected
+                and self.ipv4.is_connected
-                and self._ipv4.object_path == self.properties[DBUS_ATTR_IP4CONFIG]
+                and self.ipv4.object_path == self.properties[DBUS_ATTR_IP4CONFIG]
             ):
-                await self._ipv4.update()
+                await self.ipv4.update()
             elif self.properties[DBUS_ATTR_IP4CONFIG] != DBUS_OBJECT_BASE:
-                self._ipv4 = IpConfiguration(self.properties[DBUS_ATTR_IP4CONFIG])
+                self.ipv4 = IpConfiguration(self.properties[DBUS_ATTR_IP4CONFIG])
-                await self._ipv4.connect(self.dbus.bus)
+                await self.ipv4.connect(self.dbus.bus)
             else:
-                self._ipv4 = None
+                self.ipv4 = None
 
         # IPv6
         if not changed or DBUS_ATTR_IP6CONFIG in changed:
             if (
-                self._ipv6
+                self.ipv6
-                and self._ipv6.is_connected
+                and self.ipv6.is_connected
-                and self._ipv6.object_path == self.properties[DBUS_ATTR_IP6CONFIG]
+                and self.ipv6.object_path == self.properties[DBUS_ATTR_IP6CONFIG]
             ):
-                await self._ipv6.update()
+                await self.ipv6.update()
             elif self.properties[DBUS_ATTR_IP6CONFIG] != DBUS_OBJECT_BASE:
-                self._ipv6 = IpConfiguration(
+                self.ipv6 = IpConfiguration(self.properties[DBUS_ATTR_IP6CONFIG], False)
-                    self.properties[DBUS_ATTR_IP6CONFIG], False
+                await self.ipv6.connect(self.dbus.bus)
-                )
-                await self._ipv6.connect(self.dbus.bus)
             else:
-                self._ipv6 = None
+                self.ipv6 = None
 
         # Settings
         if not changed or DBUS_ATTR_CONNECTION in changed:
             if (
-                self._settings
+                self.settings
-                and self._settings.is_connected
+                and self.settings.is_connected
-                and self._settings.object_path == self.properties[DBUS_ATTR_CONNECTION]
+                and self.settings.object_path == self.properties[DBUS_ATTR_CONNECTION]
             ):
-                await self._settings.reload()
+                await self.settings.reload()
             elif self.properties[DBUS_ATTR_CONNECTION] != DBUS_OBJECT_BASE:
-                self._settings = NetworkSetting(self.properties[DBUS_ATTR_CONNECTION])
+                self.settings = NetworkSetting(self.properties[DBUS_ATTR_CONNECTION])
-                await self._settings.connect(self.dbus.bus)
+                await self.settings.connect(self.dbus.bus)
             else:
-                self._settings = None
+                self.settings = None
 
     def disconnect(self) -> None:
         """Disconnect from D-Bus."""

supervisor/dbus/network/interface.py

@@ -1,5 +1,6 @@
 """NetworkInterface object for Network Manager."""
 
+import asyncio
 from typing import Any
 
 from dbus_fast.aio.message_bus import MessageBus
@@ -51,7 +52,7 @@ class NetworkInterface(DBusInterfaceProxy):
 
     @property
     @dbus_property
-    def type(self) -> int:
+    def type(self) -> DeviceType:
         """Return interface type."""
         return self.properties[DBUS_ATTR_DEVICE_TYPE]
 
@@ -72,6 +73,14 @@ class NetworkInterface(DBusInterfaceProxy):
         """Return the connection used for this interface."""
         return self._connection
 
+    @connection.setter
+    def connection(self, connection: NetworkConnection | None) -> None:
+        """Set connection for interface."""
+        if self._connection and self._connection is not connection:
+            asyncio.get_event_loop().run_in_executor(None, self._connection.disconnect)
+
+        self._connection = connection
+
     @property
     def settings(self) -> NetworkSetting | None:
         """Return the connection settings used for this interface."""
@@ -82,6 +91,14 @@ class NetworkInterface(DBusInterfaceProxy):
         """Return the wireless data for this interface."""
         return self._wireless
 
+    @wireless.setter
+    def wireless(self, wireless: NetworkWireless | None) -> None:
+        """Set wireless for interface."""
+        if self._wireless and self._wireless is not wireless:
+            asyncio.get_event_loop().run_in_executor(None, self._wireless.disconnect)
+
+        self._wireless = wireless
+
     async def connect(self, bus: MessageBus) -> None:
         """Connect to D-Bus."""
         return await super().connect(bus)
@@ -98,29 +115,29 @@ class NetworkInterface(DBusInterfaceProxy):
         # If active connection exists
         if not changed or DBUS_ATTR_ACTIVE_CONNECTION in changed:
             if (
-                self._connection
+                self.connection
-                and self._connection.is_connected
+                and self.connection.is_connected
-                and self._connection.object_path
+                and self.connection.object_path
                 == self.properties[DBUS_ATTR_ACTIVE_CONNECTION]
             ):
                 await self.connection.update()
             elif self.properties[DBUS_ATTR_ACTIVE_CONNECTION] != DBUS_OBJECT_BASE:
-                self._connection = NetworkConnection(
+                self.connection = NetworkConnection(
                     self.properties[DBUS_ATTR_ACTIVE_CONNECTION]
                 )
-                await self._connection.connect(self.dbus.bus)
+                await self.connection.connect(self.dbus.bus)
             else:
-                self._connection = None
+                self.connection = None
 
         # Wireless
         if not changed or DBUS_ATTR_DEVICE_TYPE in changed:
             if self.type != DeviceType.WIRELESS:
-                self._wireless = None
+                self.wireless = None
             elif self.wireless and self.wireless.is_connected:
-                await self._wireless.update()
+                await self.wireless.update()
             else:
-                self._wireless = NetworkWireless(self.object_path)
+                self.wireless = NetworkWireless(self.object_path)
-                await self._wireless.connect(self.dbus.bus)
+                await self.wireless.connect(self.dbus.bus)
 
     def disconnect(self) -> None:
         """Disconnect from D-Bus."""

supervisor/dbus/network/setting/__init__.py

@@ -169,7 +169,9 @@ class NetworkSetting(DBusInterface):
 
     def disconnect(self) -> None:
         """Disconnect from D-Bus."""
-        self.dbus.Settings.Connection.off_updated(self.reload)
+        if self.is_connected:
+            self.dbus.Settings.Connection.off_updated(self.reload)
+
         super().disconnect()
 
     @dbus_connected

supervisor/dbus/network/wireless.py

@@ -37,6 +37,14 @@ class NetworkWireless(DBusInterfaceProxy):
         """Return details about active connection."""
         return self._active
 
+    @active.setter
+    def active(self, active: NetworkWirelessAP | None) -> None:
+        """Set active wireless AP."""
+        if self._active and self._active is not active:
+            asyncio.get_event_loop().run_in_executor(None, self._active.disconnect)
+
+        self._active = active
+
     @dbus_connected
     async def request_scan(self) -> None:
         """Request a new AP scan."""
@@ -63,16 +71,16 @@ class NetworkWireless(DBusInterfaceProxy):
         # Get details from current active
         if not changed or DBUS_ATTR_ACTIVE_ACCESSPOINT in changed:
             if (
-                self._active
+                self.active
-                and self._active.is_connected
+                and self.active.is_connected
-                and self._active.object_path
+                and self.active.object_path
                 == self.properties[DBUS_ATTR_ACTIVE_ACCESSPOINT]
             ):
-                await self._active.update()
+                await self.active.update()
             elif self.properties[DBUS_ATTR_ACTIVE_ACCESSPOINT] != DBUS_OBJECT_BASE:
-                self._active = NetworkWirelessAP(
+                self.active = NetworkWirelessAP(
                     self.properties[DBUS_ATTR_ACTIVE_ACCESSPOINT]
                 )
-                await self._active.connect(self.dbus.bus)
+                await self.active.connect(self.dbus.bus)
             else:
-                self._active = None
+                self.active = None

tests/dbus/network/test_connection.py

@@ -0,0 +1,47 @@
+"""Test connection object."""
+
+import asyncio
+
+from supervisor.dbus.network import NetworkManager
+
+from tests.common import fire_property_change_signal
+from tests.const import TEST_INTERFACE
+
+
+async def test_old_ipv4_disconnect(network_manager: NetworkManager):
+    """Test old ipv4 disconnects on ipv4 change."""
+    connection = network_manager.interfaces[TEST_INTERFACE].connection
+    ipv4 = connection.ipv4
+    assert ipv4.is_connected is True
+
+    fire_property_change_signal(connection, {"Ip4Config": "/"})
+    await asyncio.sleep(0)
+
+    assert connection.ipv4 is None
+    assert ipv4.is_connected is False
+
+
+async def test_old_ipv6_disconnect(network_manager: NetworkManager):
+    """Test old ipv6 disconnects on ipv6 change."""
+    connection = network_manager.interfaces[TEST_INTERFACE].connection
+    ipv6 = connection.ipv6
+    assert ipv6.is_connected is True
+
+    fire_property_change_signal(connection, {"Ip6Config": "/"})
+    await asyncio.sleep(0)
+
+    assert connection.ipv6 is None
+    assert ipv6.is_connected is False
+
+
+async def test_old_settings_disconnect(network_manager: NetworkManager):
+    """Test old settings disconnects on settings change."""
+    connection = network_manager.interfaces[TEST_INTERFACE].connection
+    settings = connection.settings
+    assert settings.is_connected is True
+
+    fire_property_change_signal(connection, {"Connection": "/"})
+    await asyncio.sleep(0)
+
+    assert connection.settings is None
+    assert settings.is_connected is False

tests/dbus/network/test_interface.py

@@ -57,3 +57,29 @@ async def test_network_interface_wlan(network_manager: NetworkManager):
     interface = network_manager.interfaces[TEST_INTERFACE_WLAN]
     assert interface.name == TEST_INTERFACE_WLAN
     assert interface.type == DeviceType.WIRELESS
+
+
+async def test_old_connection_disconnect(network_manager: NetworkManager):
+    """Test old connection disconnects on connection change."""
+    interface = network_manager.interfaces[TEST_INTERFACE]
+    connection = interface.connection
+    assert connection.is_connected is True
+
+    fire_property_change_signal(interface, {"ActiveConnection": "/"})
+    await asyncio.sleep(0)
+
+    assert interface.connection is None
+    assert connection.is_connected is False
+
+
+async def test_old_wireless_disconnect(network_manager: NetworkManager):
+    """Test old wireless disconnects on type change."""
+    interface = network_manager.interfaces[TEST_INTERFACE_WLAN]
+    wireless = interface.wireless
+    assert wireless.is_connected is True
+
+    fire_property_change_signal(interface, {"DeviceType": DeviceType.ETHERNET})
+    await asyncio.sleep(0)
+
+    assert interface.wireless is None
+    assert wireless.is_connected is False

tests/dbus/network/test_network_manager.py

@@ -11,7 +11,7 @@ from supervisor.exceptions import HostNotSupportedError
 from .setting.test_init import SETTINGS_WITH_SIGNATURE
 
 from tests.common import fire_property_change_signal
-from tests.const import TEST_INTERFACE
+from tests.const import TEST_INTERFACE, TEST_INTERFACE_WLAN
 
 # pylint: disable=protected-access
 
@@ -93,3 +93,17 @@ async def test_add_and_activate_connection(
         "/org/freedesktop/NetworkManager-org.freedesktop.NetworkManager.AddAndActivateConnection",
         "/org/freedesktop/NetworkManager/Settings/1-org.freedesktop.NetworkManager.Settings.Connection.GetSettings",
     ]
+
+
+async def test_removed_devices_disconnect(network_manager: NetworkManager):
+    """Test removed devices are disconnected."""
+    wlan = network_manager.interfaces[TEST_INTERFACE_WLAN]
+    assert wlan.is_connected is True
+
+    fire_property_change_signal(
+        network_manager, {"Devices": ["/org/freedesktop/NetworkManager/Devices/1"]}
+    )
+    await asyncio.sleep(0)
+
+    assert TEST_INTERFACE_WLAN not in network_manager.interfaces
+    assert wlan.is_connected is False

tests/dbus/network/test_wireless.py

@@ -4,32 +4,39 @@ import asyncio
 from supervisor.dbus.network import NetworkManager
 
 from tests.common import fire_property_change_signal
+from tests.const import TEST_INTERFACE_WLAN
 
 
 async def test_wireless(network_manager: NetworkManager):
     """Test wireless properties."""
-    assert network_manager.interfaces["wlan0"].wireless.active is None
+    assert network_manager.interfaces[TEST_INTERFACE_WLAN].wireless.active is None
 
     fire_property_change_signal(
-        network_manager.interfaces["wlan0"].wireless,
+        network_manager.interfaces[TEST_INTERFACE_WLAN].wireless,
         {"ActiveAccessPoint": "/org/freedesktop/NetworkManager/AccessPoint/43099"},
     )
     await asyncio.sleep(0)
     assert (
-        network_manager.interfaces["wlan0"].wireless.active.mac == "E4:57:40:A9:D7:DE"
+        network_manager.interfaces[TEST_INTERFACE_WLAN].wireless.active.mac
+        == "E4:57:40:A9:D7:DE"
     )
 
     fire_property_change_signal(
-        network_manager.interfaces["wlan0"].wireless, {}, ["ActiveAccessPoint"]
+        network_manager.interfaces[TEST_INTERFACE_WLAN].wireless,
+        {},
+        ["ActiveAccessPoint"],
     )
     await asyncio.sleep(0)
-    assert network_manager.interfaces["wlan0"].wireless.active is None
+    assert network_manager.interfaces[TEST_INTERFACE_WLAN].wireless.active is None
 
 
 async def test_request_scan(network_manager: NetworkManager, dbus: list[str]):
     """Test request scan."""
     dbus.clear()
-    assert await network_manager.interfaces["wlan0"].wireless.request_scan() is None
+    assert (
+        await network_manager.interfaces[TEST_INTERFACE_WLAN].wireless.request_scan()
+        is None
+    )
     assert dbus == [
         "/org/freedesktop/NetworkManager/Devices/3-org.freedesktop.NetworkManager.Device.Wireless.RequestScan"
     ]
@@ -39,7 +46,7 @@ async def test_get_all_access_points(network_manager: NetworkManager, dbus: list
     """Test get all access points."""
     dbus.clear()
     accesspoints = await network_manager.interfaces[
-        "wlan0"
+        TEST_INTERFACE_WLAN
     ].wireless.get_all_accesspoints()
     assert len(accesspoints) == 2
     assert accesspoints[0].mac == "E4:57:40:A9:D7:DE"
@@ -49,3 +56,22 @@ async def test_get_all_access_points(network_manager: NetworkManager, dbus: list
     assert dbus == [
         "/org/freedesktop/NetworkManager/Devices/3-org.freedesktop.NetworkManager.Device.Wireless.GetAllAccessPoints"
     ]
+
+
+async def test_old_active_ap_disconnects(network_manager: NetworkManager):
+    """Test old access point disconnects on active ap change."""
+    wireless = network_manager.interfaces[TEST_INTERFACE_WLAN].wireless
+    fire_property_change_signal(
+        wireless,
+        {"ActiveAccessPoint": "/org/freedesktop/NetworkManager/AccessPoint/43099"},
+    )
+    await asyncio.sleep(0)
+
+    active = wireless.active
+    assert active.is_connected is True
+
+    fire_property_change_signal(wireless, {"ActiveAccessPoint": "/"})
+    await asyncio.sleep(0)
+
+    assert wireless.active is None
+    assert active.is_connected is False