diff --git a/mobile/app/index.tsx b/mobile/app/index.tsx index ad9574632d4..205c7acac70 100644 --- a/mobile/app/index.tsx +++ b/mobile/app/index.tsx @@ -538,7 +538,6 @@ export default function HomeScreen() { } } // Why: key on host-id set + each client's identity so resubs fire when forceReconnect swaps a host's client, not on every render. - // eslint-disable-next-line react-hooks/exhaustive-deps }, [ allClients .map((e) => `${e.hostId}:${clientKey(e.client)}`) diff --git a/mobile/src/transport/host-list-load-sharing.ts b/mobile/src/transport/host-list-load-sharing.ts index 40c1fa5a011..ab1e9e2fe89 100644 --- a/mobile/src/transport/host-list-load-sharing.ts +++ b/mobile/src/transport/host-list-load-sharing.ts @@ -1,12 +1,13 @@ import type { HostProfile } from './types' -// Why: Keychain reads are slow (50-200ms) and loadHosts() runs on every screen -// mount, so concurrent callers share one pass instead of each paying for their -// own. A pass that started before a write still holds the pre-write snapshot, -// though — handing it to a caller that loaded AFTER the write returns the host -// that caller just removed, or the name it just replaced (issue #8791). So every -// durable commit drops the shared pass and the next caller reads fresh. +// Why: concurrent callers share a slow Keychain pass; durable writes invalidate +// it so later loads cannot receive stale snapshots (#8791). let inflight: Promise | null = null +let revision = 0 + +export function getHostListLoadRevision(): number { + return revision +} export function shareHostListLoad(load: () => Promise): Promise { if (inflight) { @@ -25,5 +26,6 @@ export function shareHostListLoad(load: () => Promise): Promise { ) }) + it('does not let a late pre-write token read poison the token cache', async () => { + const newHost = { + id: 'host-new', + name: 'New Host', + endpoint: 'ws://127.0.0.1:3', + publicKeyB64: 'key-new', + deviceToken: 'token-new', + lastConnected: 0 + } + storedHostsRaw = JSON.stringify([{ ...newHost, deviceToken: undefined }]) + let resolvePrewriteTokenRead: (token: string) => void = () => {} + secureStoreMock.getItemAsync.mockReturnValue( + new Promise((resolve) => { + resolvePrewriteTokenRead = resolve + }) + ) + + const parkedLoad = loadHosts() + await vi.waitFor(() => { + expect(secureStoreMock.getItemAsync).toHaveBeenCalled() + }) + await saveHost(newHost) + resolvePrewriteTokenRead('token-old') + await parkedLoad + + const hosts = await loadHosts() + expect(hosts.find((host) => host.id === newHost.id)?.deviceToken).toBe(newHost.deviceToken) + }) + it('still shares one Keychain pass across loads with no write between them', async () => { const releaseKeychain = gateKeychainReads() const first = loadHosts() diff --git a/mobile/src/transport/host-store.ts b/mobile/src/transport/host-store.ts index 3aa3587fd62..c932f9b6e66 100644 --- a/mobile/src/transport/host-store.ts +++ b/mobile/src/transport/host-store.ts @@ -8,7 +8,7 @@ import { type StoredHostProfile } from './types' import { getNextHostNameFromHosts } from './host-names' -import { dropSharedHostListLoad, shareHostListLoad } from './host-list-load-sharing' +import * as hostListLoads from './host-list-load-sharing' import { retryPendingHostCredentialCleanups, scheduleHostCredentialCleanup @@ -103,7 +103,7 @@ export async function loadHosts(): Promise { // Why: writers hold the mutation chain across their full RMW; wait so a load doesn't race a half-written list. await hostListMutation // Why: deduplicate concurrent loadHosts() calls so simultaneously mounting screens share one Keychain read pass. - return shareHostListLoad(doLoadHosts) + return hostListLoads.shareHostListLoad(doLoadHosts) } async function doLoadHosts(): Promise { @@ -125,6 +125,7 @@ async function doLoadHosts(): Promise { for (const stored of storedHosts) { let token = tokenCache.get(stored.id) if (!token) { + const readRevision = hostListLoads.getHostListLoadRevision() let fetched: string | null try { fetched = await readDeviceToken(stored.id) @@ -137,7 +138,9 @@ async function doLoadHosts(): Promise { continue } token = fetched - tokenCache.set(stored.id, token) + if (readRevision === hostListLoads.getHostListLoadRevision()) { + tokenCache.set(stored.id, token) + } } const overlay = overlays.get(stored.id) out.push({ @@ -191,7 +194,7 @@ async function mutateStoredHosts( const current = await readStoredHostsForMutation() const next = update(current) await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(next)) - dropSharedHostListLoad() + hostListLoads.dropSharedHostListLoad() }) hostListMutation = mutation.catch(() => {}) return mutation @@ -245,7 +248,7 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise // Why: write metadata before the keychain token so a crash leaves recoverable orphaned metadata, not an orphaned token that persists forever. await writeDeviceToken(stored.id, validated.deviceToken) tokenCache.set(stored.id, validated.deviceToken) - dropSharedHostListLoad() + hostListLoads.dropSharedHostListLoad() if (validated.endpoints) { await saveMobileRelayHostOverlay({ v: 2, @@ -254,7 +257,7 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise relayHostId: validated.relayHostId, relay: validated.relay }) - dropSharedHostListLoad() + hostListLoads.dropSharedHostListLoad() } const overlayRemovalIds = [...duplicateHostIds] if (!validated.endpoints && updatedExistingHost) { @@ -263,7 +266,7 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise if (overlayRemovalIds.length > 0) { // Why: reusing an id for direct-only re-pairing must not retain routing metadata from the previous transport state. await removeMobileRelayHostOverlays(overlayRemovalIds) - dropSharedHostListLoad() + hostListLoads.dropSharedHostListLoad() } for (const duplicateHostId of duplicateHostIds) { tokenCache.delete(duplicateHostId) @@ -280,7 +283,7 @@ export async function removeHost(hostId: string): Promise { tokenCache.delete(hostId) try { await removeMobileRelayHostOverlay(hostId) - dropSharedHostListLoad() + hostListLoads.dropSharedHostListLoad() } catch { // Base removal is authoritative; a retained overlay can't resurrect the host and is cleaned on a later retry. } @@ -340,5 +343,5 @@ export async function updateLastConnected(hostId: string): Promise { export function resetHostStoreForTests(): void { hostListMutation = Promise.resolve() tokenCache.clear() - dropSharedHostListLoad() + hostListLoads.dropSharedHostListLoad() }