From 86c05915f37a5e28f5ec26e2c202f0203951c769 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Tue, 27 Oct 2026 11:09:13 +0000 Subject: [PATCH] Fix race condition in Wi-Fi and Bluetooth settings panels - Remove mounted useRef and replace with active flag and sequence counter - In load(), capture mine = ++seq at start; guard every setState with if (!active || mine !== seq) return - Cleanup sets active = false and clears interval - Add tests that defer first fetch resolution past second to verify stale responses are ignored - Existing StrictMode tests continue to pass --- .../tsk-w4432q-wifi-bluetooth-race-fix.md | 2 ++ .../apps/SettingsApp/BluetoothPanel.test.tsx | 32 +++++++++++++++++++ .../src/apps/SettingsApp/BluetoothPanel.tsx | 20 ++++++++---- .../src/apps/SettingsApp/WifiPanel.test.tsx | 32 +++++++++++++++++++ desktop/src/apps/SettingsApp/WifiPanel.tsx | 20 ++++++++---- 5 files changed, 94 insertions(+), 12 deletions(-) create mode 100644 changelog.d/tsk-w4432q-wifi-bluetooth-race-fix.md diff --git a/changelog.d/tsk-w4432q-wifi-bluetooth-race-fix.md b/changelog.d/tsk-w4432q-wifi-bluetooth-race-fix.md new file mode 100644 index 000000000..d8ab60889 --- /dev/null +++ b/changelog.d/tsk-w4432q-wifi-bluetooth-race-fix.md @@ -0,0 +1,2 @@ +### Fixed +- Fixed race condition in Wi-Fi and Bluetooth settings panels where stale API responses could overwrite newer data. Under React StrictMode or when a slow request overlapped with the 10s refresh interval, an older response could restore old access points, adapters, devices, or errors. Both panels now use a sequence counter (`seq`) and an `active` flag to ignore results from superseded or cleaned-up effect runs. \ No newline at end of file diff --git a/desktop/src/apps/SettingsApp/BluetoothPanel.test.tsx b/desktop/src/apps/SettingsApp/BluetoothPanel.test.tsx index 9f0ad91af..6710b7b5c 100644 --- a/desktop/src/apps/SettingsApp/BluetoothPanel.test.tsx +++ b/desktop/src/apps/SettingsApp/BluetoothPanel.test.tsx @@ -96,6 +96,38 @@ describe("BluetoothPanel", () => { expect(fetchSpy).toHaveBeenCalledTimes(2); vi.useRealTimers(); }, 15000); + + it("ignores stale response when a newer load starts before the first resolves", async () => { + vi.useFakeTimers({ toFake: ["setInterval", "clearInterval"] }); + let resolveFirst: (value: Response) => void; + let resolveSecond: (value: Response) => void; + const firstPromise = new Promise((resolve) => { + resolveFirst = resolve; + }); + const secondPromise = new Promise((resolve) => { + resolveSecond = resolve; + }); + const fetchSpy = vi.spyOn(globalThis, "fetch"); + fetchSpy.mockReturnValueOnce(firstPromise); + fetchSpy.mockReturnValueOnce(secondPromise); + render(); + // Advance timers to trigger the interval (second fetch) + await act(async () => { + vi.advanceTimersByTime(10_000); + }); + // Now resolve the second fetch first (newer load) + await act(async () => { + resolveSecond!(json({ available: true, adapters: [{ path: "/org/bluez/hci0", address: "AA:BB:CC:DD:EE:FF", powered: true }], devices: [{ path: "/org/bluez/hci0/dev_AA_BB_CC_DD_EE_FF", address: "AA:BB:CC:DD:EE:FF", name: "SecondDevice", paired: true, connected: true, trusted: true }] })); + }); + await waitFor(() => expect(screen.getByText("SecondDevice")).toBeInTheDocument()); + // Then resolve the first fetch (older load) + await act(async () => { + resolveFirst!(json({ available: true, adapters: [{ path: "/org/bluez/hci0", address: "AA:BB:CC:DD:EE:FF", powered: true }], devices: [{ path: "/org/bluez/hci0/dev_11_22_33_44_55_66", address: "11:22:33:44:55:66", name: "FirstDevice", paired: true, connected: true, trusted: true }] })); + }); + expect(screen.getByText("SecondDevice")).toBeInTheDocument(); + expect(screen.queryByText("FirstDevice")).not.toBeInTheDocument(); + vi.useRealTimers(); + }); }); describe("BluetoothPanel StrictMode", () => { diff --git a/desktop/src/apps/SettingsApp/BluetoothPanel.tsx b/desktop/src/apps/SettingsApp/BluetoothPanel.tsx index d50ff756a..9d27a691b 100644 --- a/desktop/src/apps/SettingsApp/BluetoothPanel.tsx +++ b/desktop/src/apps/SettingsApp/BluetoothPanel.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useRef } from "react"; +import { useState, useEffect } from "react"; import { Card } from "@/components/ui"; export function BluetoothPanel() { @@ -6,35 +6,43 @@ export function BluetoothPanel() { const [adapters, setAdapters] = useState<{ path: string; address: string; powered: boolean }[]>([]); const [devices, setDevices] = useState<{ path: string; address: string; name: string | null; paired: boolean; connected: boolean; trusted: boolean }[]>([]); const [error, setError] = useState(null); - const mounted = useRef(true); useEffect(() => { - mounted.current = true; + let active = true; + let seq = 0; const load = async () => { + const mine = ++seq; try { const res = await fetch("/api/hardware/bluetooth", { credentials: "include" }); + if (!active || mine !== seq) return; const body = await res.json().catch(() => null); - if (!mounted.current) return; + if (!active || mine !== seq) return; if (!res.ok || body === null || typeof body.available !== "boolean") { + if (!active || mine !== seq) return; setError("Could not load Bluetooth state."); return; } + if (!active || mine !== seq) return; setAvailable(body.available); if (Array.isArray(body.adapters)) { + if (!active || mine !== seq) return; setAdapters(body.adapters); } if (Array.isArray(body.devices)) { + if (!active || mine !== seq) return; setDevices(body.devices); } + if (!active || mine !== seq) return; setError(null); } catch { - if (mounted.current) setError("Could not reach taOS."); + if (!active || mine !== seq) return; + setError("Could not reach taOS."); } }; load(); const id = setInterval(load, 10_000); return () => { - mounted.current = false; + active = false; clearInterval(id); }; }, []); diff --git a/desktop/src/apps/SettingsApp/WifiPanel.test.tsx b/desktop/src/apps/SettingsApp/WifiPanel.test.tsx index 284e9aa18..17e01f11b 100644 --- a/desktop/src/apps/SettingsApp/WifiPanel.test.tsx +++ b/desktop/src/apps/SettingsApp/WifiPanel.test.tsx @@ -93,6 +93,38 @@ it("refreshes access points every 10s and clears interval on unmount", async () expect(fetchSpy).toHaveBeenCalledTimes(2); vi.useRealTimers(); }, 15000); + + it("ignores stale response when a newer load starts before the first resolves", async () => { + vi.useFakeTimers({ toFake: ["setInterval", "clearInterval"] }); + let resolveFirst: (value: Response) => void; + let resolveSecond: (value: Response) => void; + const firstPromise = new Promise((resolve) => { + resolveFirst = resolve; + }); + const secondPromise = new Promise((resolve) => { + resolveSecond = resolve; + }); + const fetchSpy = vi.spyOn(globalThis, "fetch"); + fetchSpy.mockReturnValueOnce(firstPromise); + fetchSpy.mockReturnValueOnce(secondPromise); + render(); + // Advance timers to trigger the interval (second fetch) + await act(async () => { + vi.advanceTimersByTime(10_000); + }); + // Now resolve the second fetch first (newer load) + await act(async () => { + resolveSecond!(json({ available: true, wifi_enabled: true, access_points: [{ ssid: "SecondAP", strength: 90, frequency: 2437, secured: true, active: true }] })); + }); + await waitFor(() => expect(screen.getByText("SecondAP")).toBeInTheDocument()); + // Then resolve the first fetch (older load) + await act(async () => { + resolveFirst!(json({ available: true, wifi_enabled: true, access_points: [{ ssid: "FirstAP", strength: 50, frequency: 2412, secured: true, active: false }] })); + }); + expect(screen.getByText("SecondAP")).toBeInTheDocument(); + expect(screen.queryByText("FirstAP")).not.toBeInTheDocument(); + vi.useRealTimers(); + }); }); describe("WifiPanel StrictMode", () => { diff --git a/desktop/src/apps/SettingsApp/WifiPanel.tsx b/desktop/src/apps/SettingsApp/WifiPanel.tsx index 0bc87c46d..3c0f2d0ef 100644 --- a/desktop/src/apps/SettingsApp/WifiPanel.tsx +++ b/desktop/src/apps/SettingsApp/WifiPanel.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useRef } from "react"; +import { useState, useEffect } from "react"; import { AlertCircle, Wifi as WifiIcon } from "lucide-react"; import { Card } from "@/components/ui"; @@ -7,33 +7,41 @@ export function WifiPanel() { const [wifiEnabled, setWifiEnabled] = useState(null); const [accessPoints, setAccessPoints] = useState<{ ssid: string; strength: number; frequency: number; secured: boolean; active: boolean }[]>([]); const [error, setError] = useState(null); - const mounted = useRef(true); useEffect(() => { - mounted.current = true; + let active = true; + let seq = 0; const load = async () => { + const mine = ++seq; try { const res = await fetch("/api/hardware/wifi", { credentials: "include" }); + if (!active || mine !== seq) return; const body = await res.json().catch(() => null); - if (!mounted.current) return; + if (!active || mine !== seq) return; if (!res.ok || body === null || typeof body.available !== "boolean") { + if (!active || mine !== seq) return; setError("Could not load Wi-Fi state."); return; } + if (!active || mine !== seq) return; setAvailable(body.available); + if (!active || mine !== seq) return; setWifiEnabled(body.wifi_enabled ?? null); if (body.available && body.wifi_enabled && Array.isArray(body.access_points)) { + if (!active || mine !== seq) return; setAccessPoints(body.access_points); } + if (!active || mine !== seq) return; setError(null); } catch { - if (mounted.current) setError("Could not reach taOS."); + if (!active || mine !== seq) return; + setError("Could not reach taOS."); } }; load(); const id = setInterval(load, 10_000); return () => { - mounted.current = false; + active = false; clearInterval(id); }; }, []);