Skip to content

Commit 496ae85

Browse files
authored
WifiManager: guard init_wifi_state (commaai#37413)
* failing test * fix * rename * better
1 parent 5c630b2 commit 496ae85

2 files changed

Lines changed: 39 additions & 4 deletions

File tree

system/ui/lib/tests/test_handle_state_change.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,33 @@ def user_taps_b_during_dbus(*args, **kwargs):
347347
assert wm._wifi_state.ssid == "B"
348348
assert wm._wifi_state.status == ConnectStatus.CONNECTING
349349

350+
def test_init_wifi_state_race_user_tap_during_dbus(self, mocker):
351+
"""User taps B while _init_wifi_state's DBus calls are in flight.
352+
353+
_init_wifi_state runs from set_active(True) or worker error paths. It does
354+
2 DBus calls (device State property + _get_active_wifi_connection) then
355+
unconditionally writes _wifi_state. If the user taps a network during those
356+
calls, _set_connecting("B") is overwritten with stale NM ground truth.
357+
"""
358+
wm = _make_wm(mocker, connections={"A": "/path/A", "B": "/path/B"})
359+
wm._wifi_device = "/dev/wifi0"
360+
wm._router_main = mocker.MagicMock()
361+
362+
state_reply = mocker.MagicMock()
363+
state_reply.body = [('u', NMDeviceState.ACTIVATED)]
364+
wm._router_main.send_and_get_reply.return_value = state_reply
365+
366+
def user_taps_b_during_dbus(*args, **kwargs):
367+
wm._set_connecting("B")
368+
return ("/path/A", {})
369+
370+
wm._get_active_wifi_connection.side_effect = user_taps_b_during_dbus
371+
372+
wm._init_wifi_state()
373+
374+
assert wm._wifi_state.ssid == "B"
375+
assert wm._wifi_state.status == ConnectStatus.CONNECTING
376+
350377

351378
# ---------------------------------------------------------------------------
352379
# Full sequences (NM signal order from real devices)

system/ui/lib/wifi_manager.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ def __init__(self):
197197
self._networks_updated: list[Callable[[list[Network]], None]] = []
198198
self._disconnected: list[Callable[[], None]] = []
199199

200-
self._lock = threading.Lock()
200+
self._scan_lock = threading.Lock()
201201
self._scan_thread = threading.Thread(target=self._network_scanner, daemon=True)
202202
self._state_thread = threading.Thread(target=self._monitor_state, daemon=True)
203203
self._initialize()
@@ -227,6 +227,8 @@ def worker():
227227
cloudlog.warning("No WiFi device found")
228228
return
229229

230+
epoch = self._user_epoch
231+
230232
dev_addr = DBusAddress(self._wifi_device, bus_name=NM, interface=NM_DEVICE_IFACE)
231233
dev_state = self._router_main.send_and_get_reply(Properties(dev_addr).get('State')).body[0][1]
232234

@@ -239,6 +241,10 @@ def worker():
239241
conn_path, _ = self._get_active_wifi_connection()
240242
if conn_path:
241243
wifi_state.ssid = next((s for s, p in self._connections.items() if p == conn_path), None)
244+
245+
if self._user_epoch != epoch:
246+
return
247+
242248
self._wifi_state = wifi_state
243249

244250
if block:
@@ -281,11 +287,13 @@ def current_network_metered(self) -> MeteredType:
281287

282288
@property
283289
def connecting_to_ssid(self) -> str | None:
284-
return self._wifi_state.ssid if self._wifi_state.status == ConnectStatus.CONNECTING else None
290+
wifi_state = self._wifi_state
291+
return wifi_state.ssid if wifi_state.status == ConnectStatus.CONNECTING else None
285292

286293
@property
287294
def connected_ssid(self) -> str | None:
288-
return self._wifi_state.ssid if self._wifi_state.status == ConnectStatus.CONNECTED else None
295+
wifi_state = self._wifi_state
296+
return wifi_state.ssid if wifi_state.status == ConnectStatus.CONNECTED else None
289297

290298
@property
291299
def tethering_password(self) -> str:
@@ -822,7 +830,7 @@ def _update_networks(self, block: bool = True):
822830
return
823831

824832
def worker():
825-
with self._lock:
833+
with self._scan_lock:
826834
if self._wifi_device is None:
827835
cloudlog.warning("No WiFi device found")
828836
return

0 commit comments

Comments
 (0)