Skip to content

Commit effcf6c

Browse files
committed
fix: prevent same-name BLE discovery starvation
1 parent 2fed7e1 commit effcf6c

2 files changed

Lines changed: 36 additions & 19 deletions

File tree

src/transport/ReactNativeBleTransport.test.ts

Lines changed: 30 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

src/transport/ReactNativeBleTransport.ts

Lines changed: 6 additions & 9 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 {
@@ -55,12 +55,10 @@ export class ReactNativeBleTransport implements BleTransport {
5555
this.#scanDevices.set(device.id, device);
5656
this.#scanKeys.add(scanKey);
5757
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:');
58+
// Suppress repeats only for this peripheral. A rotated address must be
59+
// probed again: neither a shared nor a cached name establishes identity.
60+
// Cap retained keys so long scans cannot accumulate unbounded state.
61+
retainCompletedKey = desktop?.platform === 'windows' && this.#scanKeys.size <= 256;
6462
if (active && operation === this.#operation && desktop) onDesktop(desktop);
6563
}).catch(() => undefined).finally(() => {
6664
if (operation === this.#operation) {
@@ -474,8 +472,7 @@ export class ReactNativeBleTransport implements BleTransport {
474472
}
475473

476474
#scanKey(device: Device): string {
477-
const name = bluetoothDeviceDisplayName({ name: device.name, localName: device.localName }, this.platform);
478-
return name ? `name:${name}` : `id:${device.id}`;
475+
return `id:${device.id}`;
479476
}
480477

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

0 commit comments

Comments
 (0)