Skip to content

Commit 88df628

Browse files
enaboappsenaboapps
authored
Merge pull request #161 from switchifyapp/codex/same-name-discovery-160
fix: prevent same-name BLE discovery starvation
2 parents 2fed7e1 + 9c67dcf commit 88df628

2 files changed

Lines changed: 139 additions & 24 deletions

File tree

‎src/transport/ReactNativeBleTransport.test.ts‎

Lines changed: 103 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ describe('ReactNativeBleTransport', () => {
588588
expect(found).not.toHaveBeenCalled();
589589
});
590590

591-
it('deduplicates rotating addresses for the same named PC while a probe is in flight', async () => {
591+
it('probes a distinct same-name address while another probe is in flight', async () => {
592592
let scanCallback!: (error: Error | null, value: Device | null) => void;
593593
let releaseConnect!: (value: Device) => void;
594594
const connected = device();
@@ -602,15 +602,15 @@ describe('ReactNativeBleTransport', () => {
602602
scanCallback(null, first);
603603
await waitFor(() => (first.connect as jest.Mock).mock.calls.length === 1);
604604
scanCallback(null, rotated);
605-
expect(rotated.isConnected).not.toHaveBeenCalled();
605+
expect(rotated.isConnected).toHaveBeenCalledTimes(1);
606606

607607
stop();
608608
releaseConnect(connected);
609609
await waitFor(() => (connected.cancelConnection as jest.Mock).mock.calls.length === 1);
610610
expect(found).not.toHaveBeenCalled();
611611
});
612612

613-
it('deduplicates a rotated Windows address after the first probe completes', async () => {
613+
it('deduplicates a known Windows address but probes a new same-name address', async () => {
614614
let scanCallback!: (error: Error | null, value: Device | null) => void;
615615
const firstConnected = device({
616616
readCharacteristicForService: jest.fn(async () => ({ value: fromByteArray(new TextEncoder().encode('{"protocolVersion":1,"desktopId":"pc-1","displayName":"A9_MAX","platform":"windows"}')) } as Characteristic)),
@@ -624,14 +624,16 @@ describe('ReactNativeBleTransport', () => {
624624

625625
scanCallback(null, first);
626626
await waitFor(() => found.mock.calls.some(([desktop]) => desktop.desktopId === 'pc-1'));
627+
scanCallback(null, first);
627628
scanCallback(null, rotated);
628629
await Promise.resolve();
629630

630-
expect(rotated.isConnected).not.toHaveBeenCalled();
631+
expect(first.connect).toHaveBeenCalledTimes(1);
632+
expect(rotated.isConnected).toHaveBeenCalledTimes(1);
631633
stop();
632634
});
633635

634-
it('discovers multiple Macs that share the Switchify PC Bluetooth name', async () => {
636+
it.each(['android', 'ios'] as const)('discovers overlapping same-name PCs on %s', async (platform) => {
635637
let scanCallback!: (error: Error | null, value: Device | null) => void;
636638
const makeMac = (id: string, desktopId: string, displayName: string) => {
637639
const connected = device({
@@ -642,21 +644,38 @@ describe('ReactNativeBleTransport', () => {
642644
const first = makeMac('mac-1', 'pc-1', 'First Mac');
643645
const second = makeMac('mac-2', 'pc-2', 'Second Mac');
644646
const native = manager({ startDeviceScan: jest.fn((_uuids, _options, callback) => { scanCallback = callback; }) });
645-
const transport = new ReactNativeBleTransport(native, 'ios');
647+
const transport = new ReactNativeBleTransport(native, platform);
646648
const found = jest.fn();
647649
const stop = transport.scan(found, jest.fn());
648650

649651
scanCallback(null, first);
650-
await waitFor(() => found.mock.calls.some(([desktop]) => desktop.desktopId === 'pc-1'));
651652
scanCallback(null, second);
653+
await waitFor(() => found.mock.calls.some(([desktop]) => desktop.desktopId === 'pc-1'));
652654
await waitFor(() => found.mock.calls.some(([desktop]) => desktop.desktopId === 'pc-2'));
653655

654656
expect(first.connect).toHaveBeenCalledTimes(1);
655657
expect(second.connect).toHaveBeenCalledTimes(1);
656658
stop();
657659
});
658660

659-
it('can resolve the second of two same-name PCs', async () => {
661+
it('limits concurrent same-name probes to four and cancels them on stop', async () => {
662+
let callback!: (error: Error | null, value: Device | null) => void;
663+
const native = manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) });
664+
const transport = new ReactNativeBleTransport(native, 'android');
665+
const peers = Array.from({ length: 5 }, (_, index) => device({
666+
id: `peer-${index}`, name: 'Switchify PC',
667+
isConnected: jest.fn(() => new Promise<boolean>(() => undefined)),
668+
}));
669+
const stop = transport.scan(jest.fn(), jest.fn());
670+
peers.forEach((peer) => callback(null, peer));
671+
peers.slice(0, 4).forEach((peer) => expect(peer.isConnected).toHaveBeenCalledTimes(1));
672+
expect(peers[4]!.isConnected).not.toHaveBeenCalled();
673+
stop();
674+
peers.slice(0, 4).forEach((peer) => expect(peer.cancelConnection).toHaveBeenCalled());
675+
await transport.disconnect();
676+
});
677+
678+
it.each(['android', 'ios'] as const)('resolves overlapping same-name PCs on %s without starving the target', async (platform) => {
660679
let scanCallback!: (error: Error | null, value: Device | null) => void;
661680
const firstConnected = device();
662681
const first = device({ id: 'mac-1', name: 'Switchify PC', isConnected: jest.fn(async () => false), connect: jest.fn(async () => firstConnected) });
@@ -668,12 +687,13 @@ describe('ReactNativeBleTransport', () => {
668687
});
669688
const second = device({ id: 'mac-2', name: 'Switchify PC', isConnected: jest.fn(async () => false), connect: jest.fn(async () => secondConnected) });
670689
const native = manager({ startDeviceScan: jest.fn((_uuids, _options, callback) => { scanCallback = callback; }) });
671-
const transport = new ReactNativeBleTransport(native, 'ios');
690+
const transport = new ReactNativeBleTransport(native, platform);
672691

673692
const resolving = transport.resolveAndConnect('pc-2');
674693
await waitFor(() => typeof scanCallback === 'function');
675694
scanCallback(null, first);
676-
await waitFor(() => (firstConnected.cancelConnection as jest.Mock).mock.calls.length === 1);
695+
scanCallback(null, second);
696+
scanCallback(null, first);
677697
scanCallback(null, second);
678698
await expect(resolving).resolves.toMatchObject({ desktopId: 'pc-2', peripheralId: 'mac-2' });
679699

@@ -682,6 +702,79 @@ describe('ReactNativeBleTransport', () => {
682702
expect(secondConnected.cancelConnection).not.toHaveBeenCalled();
683703
});
684704

705+
it.each(['android', 'ios'] as const)('queues a target behind four active probes on %s', async (platform) => {
706+
let callback!: (error: Error | null, value: Device | null) => void;
707+
const releases: (() => void)[] = [];
708+
const blockers = Array.from({ length: 4 }, (_, index) => device({
709+
id: `other-${index}`, name: 'Switchify PC',
710+
discoverAllServicesAndCharacteristics: jest.fn(() => new Promise<Device>((resolve) => {
711+
releases.push(() => resolve(device()));
712+
})),
713+
}));
714+
const target = device({ id: 'target', name: 'Switchify PC', readCharacteristicForService: jest.fn(async () => ({
715+
value: fromByteArray(new TextEncoder().encode('{"protocolVersion":1,"desktopId":"wanted"}')),
716+
} as Characteristic)) });
717+
const transport = new ReactNativeBleTransport(manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) }), platform);
718+
const resolving = transport.resolveAndConnect('wanted');
719+
await waitFor(() => !!callback);
720+
blockers.forEach((peer) => callback(null, peer));
721+
await waitFor(() => releases.length === 4);
722+
callback(null, target);
723+
expect(target.isConnected).not.toHaveBeenCalled();
724+
releases.forEach((release) => release());
725+
await expect(resolving).resolves.toMatchObject({ desktopId: 'wanted' });
726+
expect(target.isConnected).toHaveBeenCalledTimes(1);
727+
await transport.disconnect();
728+
});
729+
730+
it('retains the newest Windows address by evicting the oldest at capacity', async () => {
731+
let callback!: (error: Error | null, value: Device | null) => void;
732+
const found = jest.fn();
733+
const transport = new ReactNativeBleTransport(manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) }), 'android');
734+
const stop = transport.scan(found, jest.fn());
735+
const peers = Array.from({ length: 257 }, (_, index) => device({ id: `windows-${index}` }));
736+
for (const [index, peer] of peers.entries()) {
737+
callback(null, peer);
738+
await waitFor(() => found.mock.calls.length === index + 1);
739+
}
740+
callback(null, peers[256]!);
741+
expect(peers[256]!.isConnected).toHaveBeenCalledTimes(1);
742+
callback(null, peers[0]!);
743+
expect(peers[0]!.isConnected).toHaveBeenCalledTimes(2);
744+
stop();
745+
await transport.disconnect();
746+
});
747+
748+
it('does not drain waiting probes while preparing a claimed target', async () => {
749+
let callback!: (error: Error | null, value: Device | null) => void;
750+
let releaseMtu!: () => void;
751+
const releases: (() => void)[] = [];
752+
const target = device({ id: 'target', requestMTU: jest.fn(() => new Promise<Device>((resolve) => {
753+
releaseMtu = () => resolve(target);
754+
})) });
755+
const others = Array.from({ length: 3 }, (_, index) => device({
756+
id: `other-${index}`, discoverAllServicesAndCharacteristics: jest.fn(() => new Promise<Device>((resolve) => {
757+
releases.push(() => resolve(device()));
758+
})),
759+
}));
760+
const queued = device({ id: 'queued' });
761+
const transport = new ReactNativeBleTransport(manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) }), 'android');
762+
const result = transport.resolveAndConnect('pc-1');
763+
await waitFor(() => !!callback);
764+
others.forEach((peer) => callback(null, peer));
765+
callback(null, target);
766+
callback(null, queued);
767+
await waitFor(() => !!releaseMtu && releases.length === 3);
768+
releases.forEach((release) => release());
769+
await waitFor(() => others.every((peer) => (peer.readCharacteristicForService as jest.Mock).mock.calls.length === 1));
770+
callback(null, queued);
771+
expect(queued.isConnected).not.toHaveBeenCalled();
772+
releaseMtu();
773+
await result;
774+
expect(queued.isConnected).not.toHaveBeenCalled();
775+
await transport.disconnect();
776+
});
777+
685778
it('waits for cancelled discovery probe cleanup before a real connection', async () => {
686779
let scanCallback!: (error: Error | null, value: Device | null) => void;
687780
let releaseDiscovery!: (value: Device) => void;

‎src/transport/ReactNativeBleTransport.ts‎

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { BLE_DESCRIPTORS, BLE_UUIDS } from '@/domain/protocol/constants';
66
import { parseStatus } from '@/domain/protocol/responses';
77
import type { ConnectionStage, ConnectionStageOutcome, DiagnosticLog } from '@/diagnostics/DiagnosticLog';
88
import type { BleAvailability, BleTransport, DiscoveredDesktop, Unsubscribe } from './BleTransport';
9-
import { bluetoothDeviceDisplayName, desktopDisplayName } from './desktopDisplayName';
9+
import { desktopDisplayName } from './desktopDisplayName';
1010
import { ReadResponsePoller } from './ReadResponsePoller';
1111

1212
export class ReactNativeBleTransport implements BleTransport {
@@ -46,34 +46,46 @@ export class ReactNativeBleTransport implements BleTransport {
4646
scan(onDesktop: (desktop: DiscoveredDesktop) => void, onError: (error: Error) => void): Unsubscribe {
4747
const operation = ++this.#operation;
4848
let active = true;
49-
this.#managerOrCreate().startDeviceScan([BLE_UUIDS.service], null, (error, device) => {
49+
const waiting = new Map<string, Device>();
50+
const onAdvertisement = (error: Error | null, device: Device | null) => {
5051
if (!active || operation !== this.#operation) return;
5152
if (error) { onError(error); return; }
52-
if (!device || this.#scanDevices.has(device.id) || this.#scanKeys.has(this.#scanKey(device)) || this.#scanDevices.size >= 4) return;
53+
if (!device || this.#scanDevices.has(device.id) || this.#scanKeys.has(this.#scanKey(device))) return;
54+
if (this.#scanDevices.size >= 4) {
55+
if (waiting.size < 32) waiting.set(device.id, device);
56+
return;
57+
}
58+
waiting.delete(device.id);
5359
const scanKey = this.#scanKey(device);
5460
let retainCompletedKey = false;
5561
this.#scanDevices.set(device.id, device);
5662
this.#scanKeys.add(scanKey);
5763
const task = this.#readStatus(device).then((desktop) => {
58-
// Windows commonly rotates its private BLE address while retaining the
59-
// computer name. Keep that name claimed for this scan after a
60-
// successful probe so one PC cannot repeatedly open GATT connections.
61-
// macOS uses the shared name "Switchify PC", so its key must be
62-
// released after each probe to allow multiple Macs to be discovered.
63-
retainCompletedKey = desktop?.platform === 'windows' && scanKey.startsWith('name:');
64+
// Suppress repeats only for this peripheral. A rotated address must be
65+
// probed again: neither a shared nor a cached name establishes identity.
66+
// Cap retained keys so long scans cannot accumulate unbounded state.
67+
retainCompletedKey = desktop?.platform === 'windows';
68+
if (active && operation === this.#operation && retainCompletedKey && this.#scanKeys.size > 256) {
69+
const oldest = [...this.#scanKeys].find((key) => ![...this.#scanDevices.values()].some((peer) => this.#scanKey(peer) === key));
70+
if (oldest) this.#scanKeys.delete(oldest);
71+
}
6472
if (active && operation === this.#operation && desktop) onDesktop(desktop);
6573
}).catch(() => undefined).finally(() => {
6674
if (operation === this.#operation) {
6775
this.#scanDevices.delete(device.id);
6876
if (!retainCompletedKey) this.#scanKeys.delete(scanKey);
77+
const next = waiting.values().next().value;
78+
if (next) onAdvertisement(null, next);
6979
}
7080
});
7181
this.#scanTasks.add(task);
7282
void task.finally(() => this.#scanTasks.delete(task));
73-
});
83+
};
84+
this.#managerOrCreate().startDeviceScan([BLE_UUIDS.service], null, onAdvertisement);
7485
return () => {
7586
if (!active) return;
7687
active = false;
88+
waiting.clear();
7789
if (operation === this.#operation) this.#operation += 1;
7890
this.#managerOrCreate().stopDeviceScan();
7991
this.#cancelNativeOperations();
@@ -100,10 +112,12 @@ export class ReactNativeBleTransport implements BleTransport {
100112
let active = true;
101113
let claimedDeviceId: string | null = null;
102114
let cancellation: Promise<void> | null = null;
115+
const waiting = new Map<string, Device>();
103116
const succeed = (desktop: DiscoveredDesktop) => {
104117
if (!active) return;
105118
this.#recordStage('resolution', 'succeeded', operation);
106119
active = false;
120+
waiting.clear();
107121
clearTimeout(timer);
108122
this.#managerOrCreate().stopDeviceScan();
109123
this.#scanKeys.clear();
@@ -114,6 +128,7 @@ export class ReactNativeBleTransport implements BleTransport {
114128
if (!active) return cancellation ?? Promise.resolve();
115129
this.#recordStage('resolution', outcome, operation);
116130
active = false;
131+
waiting.clear();
117132
clearTimeout(timer);
118133
this.#managerOrCreate().stopDeviceScan();
119134
const probes = [...this.#scanDevices.values()];
@@ -135,16 +150,22 @@ export class ReactNativeBleTransport implements BleTransport {
135150
this.#resolutionCancel = cancel;
136151
const timer = setTimeout(() => { void cancel(new Error('Saved PC discovery timed out.'), 'timed_out'); }, this.nativeTimeoutMs);
137152
const onAdvertisement = (error: Error | null, device: Device | null) => {
138-
if (!active || operation !== this.#operation) return;
153+
if (!active || operation !== this.#operation || claimedDeviceId !== null) return;
139154
if (error) { void cancel(new Error('Saved PC discovery failed.')); return; }
140-
if (!device || this.#scanDevices.has(device.id) || this.#scanKeys.has(this.#scanKey(device)) || this.#scanDevices.size >= 4) return;
155+
if (!device || this.#scanDevices.has(device.id) || this.#scanKeys.has(this.#scanKey(device))) return;
156+
if (this.#scanDevices.size >= 4) {
157+
if (waiting.size < 32) waiting.set(device.id, device);
158+
return;
159+
}
160+
waiting.delete(device.id);
141161
this.#scanDevices.set(device.id, device);
142162
this.#scanKeys.add(this.#scanKey(device));
143163
const task = this.#readStatus(device, (desktop) => {
144164
if (!active || operation !== this.#operation) return false;
145165
this.#recordStage('selected_match', desktop.desktopId === desktopId ? 'succeeded' : 'not_matched', operation);
146166
if (desktop.desktopId !== desktopId || claimedDeviceId !== null) return false;
147167
claimedDeviceId = device.id;
168+
waiting.clear();
148169
return true;
149170
}).then(async (desktop) => {
150171
if (!active || operation !== this.#operation || desktop?.desktopId !== desktopId || claimedDeviceId !== device.id) return;
@@ -176,6 +197,8 @@ export class ReactNativeBleTransport implements BleTransport {
176197
if (operation === this.#operation) {
177198
this.#scanDevices.delete(device.id);
178199
this.#scanKeys.delete(this.#scanKey(device));
200+
const next = waiting.values().next().value;
201+
if (next) onAdvertisement(null, next);
179202
}
180203
});
181204
this.#scanTasks.add(task);
@@ -474,8 +497,7 @@ export class ReactNativeBleTransport implements BleTransport {
474497
}
475498

476499
#scanKey(device: Device): string {
477-
const name = bluetoothDeviceDisplayName({ name: device.name, localName: device.localName }, this.platform);
478-
return name ? `name:${name}` : `id:${device.id}`;
500+
return `id:${device.id}`;
479501
}
480502

481503
#bounded<T>(operation: Promise<T>, timeoutMs = this.nativeTimeoutMs): Promise<T> {

0 commit comments

Comments
 (0)