Skip to content

Commit 02dfc50

Browse files
mruegclaude
andauthored
Shut down activated network connections after use (#7333)
apply_changes() gets a fresh NetworkConnection from activate_connection() or add_and_activate_connection() to wait for the activation to finish. Connecting it registers property sync signal listeners and creates child IP configuration and settings objects, but the object was dropped without being shut down, leaking those listeners on every network update and on every re-activation during startup. Shut the connection down once the activation wait is done, including when it fails or times out. For background activations the job takes over and shuts it down when it finishes. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent d79eff6 commit 02dfc50

2 files changed

Lines changed: 136 additions & 12 deletions

File tree

‎supervisor/host/network.py‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ async def _activate_connection_job(
535535
try:
536536
await self._wait_for_activation(con)
537537
finally:
538+
con.shutdown()
538539
# update_only means not done by user so don't force a check afterwards
539540
await self.update(force_connectivity_check=not update_only)
540541

@@ -617,16 +618,22 @@ async def apply_changes(
617618

618619
job_id: str | None = None
619620
if con:
620-
if background_activation:
621-
job, _ = await self.sys_jobs.schedule_job(
622-
self._activate_connection_job,
623-
JobSchedulerOptions(),
624-
con,
625-
update_only=update_only,
626-
)
627-
job_id = job.uuid
628-
else:
629-
await self._wait_for_activation(con)
621+
try:
622+
if background_activation:
623+
job, _ = await self.sys_jobs.schedule_job(
624+
self._activate_connection_job,
625+
JobSchedulerOptions(),
626+
con,
627+
update_only=update_only,
628+
)
629+
job_id = job.uuid
630+
# The job shuts the connection down once it is done
631+
con = None
632+
else:
633+
await self._wait_for_activation(con)
634+
finally:
635+
if con:
636+
con.shutdown()
630637

631638
if job_id is None:
632639
# update_only means not done by user so don't force a check afterwards

‎tests/host/test_network.py‎

Lines changed: 119 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
"""Test network manager."""
22

33
import asyncio
4+
from collections.abc import AsyncGenerator
45
from ipaddress import IPv4Address, IPv6Address
6+
from typing import Any
57
from unittest.mock import AsyncMock, patch
68

79
from dbus_fast import DBusError, Variant
@@ -10,14 +12,20 @@
1012
from supervisor.const import CoreState
1113
from supervisor.coresys import CoreSys
1214
from supervisor.dbus.const import InterfaceMethod
15+
from supervisor.dbus.network.connection import NetworkConnection
1316
from supervisor.dbus.network.setting import (
1417
CONF_ATTR_802_WIRELESS_SECURITY,
1518
CONF_ATTR_802_WIRELESS_SECURITY_PSK,
1619
)
1720
from supervisor.dbus.network.setting.generate import get_connection_from_interface
18-
from supervisor.exceptions import HostNetworkError, HostNotSupportedError
21+
from supervisor.exceptions import (
22+
HostNetworkActivationFailedError,
23+
HostNetworkError,
24+
HostNotSupportedError,
25+
)
1926
from supervisor.homeassistant.const import WSEvent, WSType
20-
from supervisor.host.const import MulticastDnsMode, WifiMode
27+
from supervisor.host.configuration import WifiConfig
28+
from supervisor.host.const import AuthMethod, MulticastDnsMode, WifiMode
2129

2230
from tests.dbus_service_mocks.base import DBusServiceMock
2331
from tests.dbus_service_mocks.network_active_connection import (
@@ -64,6 +72,44 @@ async def fixture_device_wlan0_service(
6472
]
6573

6674

75+
@pytest.fixture(name="activated_connections")
76+
async def fixture_activated_connections(
77+
coresys: CoreSys,
78+
) -> AsyncGenerator[list[NetworkConnection]]:
79+
"""Record active connections returned by NetworkManager activation calls."""
80+
activated: list[NetworkConnection] = []
81+
network = coresys.dbus.network
82+
activate_connection = network.activate_connection
83+
add_and_activate_connection = network.add_and_activate_connection
84+
85+
async def _activate_connection(*args: Any) -> NetworkConnection:
86+
con = await activate_connection(*args)
87+
activated.append(con)
88+
return con
89+
90+
async def _add_and_activate_connection(*args: Any) -> tuple[Any, NetworkConnection]:
91+
settings, con = await add_and_activate_connection(*args)
92+
activated.append(con)
93+
return settings, con
94+
95+
with (
96+
patch.object(network, "activate_connection", new=_activate_connection),
97+
patch.object(
98+
network, "add_and_activate_connection", new=_add_and_activate_connection
99+
),
100+
):
101+
yield activated
102+
103+
104+
def _assert_connection_shut_down(con: NetworkConnection) -> None:
105+
"""Assert an activated connection and its child objects are shut down."""
106+
assert con.is_shutdown
107+
assert not con.is_connected
108+
assert con.settings.is_shutdown
109+
assert con.ipv4.is_shutdown
110+
assert con.ipv6.is_shutdown
111+
112+
67113
async def test_load(
68114
coresys: CoreSys,
69115
network_manager_service: NetworkManagerService,
@@ -625,3 +671,74 @@ async def test_apply_changes_v2_reenable_reuses_profile(
625671
"/org/freedesktop/NetworkManager/Devices/1",
626672
"/",
627673
) in network_manager_service.ActivateConnection.calls
674+
675+
676+
@pytest.mark.parametrize(
677+
("interface_name", "wifi"),
678+
[
679+
pytest.param("eth0", None, id="update"),
680+
pytest.param(
681+
"wlan0",
682+
WifiConfig(
683+
WifiMode.INFRASTRUCTURE, "MY_TEST", AuthMethod.WPA_PSK, "password", None
684+
),
685+
id="create",
686+
),
687+
],
688+
)
689+
async def test_apply_changes_shuts_down_activated_connection(
690+
coresys: CoreSys,
691+
activated_connections: list[NetworkConnection],
692+
interface_name: str,
693+
wifi: WifiConfig | None,
694+
):
695+
"""Test the connection returned by activation is shut down once activated."""
696+
await coresys.host.network.load()
697+
698+
interface = coresys.host.network.get(interface_name)
699+
interface.enabled = True
700+
interface.wifi = wifi
701+
await coresys.host.network.apply_changes(interface)
702+
703+
assert len(activated_connections) == 1
704+
_assert_connection_shut_down(activated_connections[0])
705+
# The interface tracks the active connection with its own object
706+
assert not coresys.dbus.network.get("eth0").connection.is_shutdown
707+
708+
709+
async def test_apply_changes_shuts_down_failed_connection(
710+
coresys: CoreSys,
711+
active_connection_service: ActiveConnectionService,
712+
activated_connections: list[NetworkConnection],
713+
):
714+
"""Test the connection returned by activation is shut down if activation fails."""
715+
await coresys.host.network.load()
716+
717+
original_state = active_connection_service.fixture.state
718+
active_connection_service.fixture.state = 4 # DEACTIVATED
719+
try:
720+
with pytest.raises(HostNetworkActivationFailedError):
721+
await coresys.host.network.apply_changes(coresys.host.network.get("eth0"))
722+
finally:
723+
active_connection_service.fixture.state = original_state
724+
725+
assert len(activated_connections) == 1
726+
_assert_connection_shut_down(activated_connections[0])
727+
728+
729+
async def test_apply_changes_background_shuts_down_activated_connection(
730+
coresys: CoreSys,
731+
activated_connections: list[NetworkConnection],
732+
):
733+
"""Test the connection is shut down once a background activation finishes."""
734+
await coresys.host.network.load()
735+
736+
job_id = await coresys.host.network.apply_changes(
737+
coresys.host.network.get("eth0"), background_activation=True
738+
)
739+
job = coresys.jobs.get_job(job_id)
740+
while not job.done:
741+
await asyncio.sleep(0)
742+
743+
assert len(activated_connections) == 1
744+
_assert_connection_shut_down(activated_connections[0])

0 commit comments

Comments
 (0)