Add concurrent connection protection to BLE managers
Prevents race conditions when multiple connection attempts are made to the same device simultaneously by: - Adding connectingDevices Set to track in-progress connections - Checking for concurrent connections before starting new attempt - Returning early if device is already connected - Using finally block to ensure cleanup of connecting state - Clearing connectingDevices set on cleanup Includes comprehensive test suite for concurrent connection scenarios including edge cases like rapid connect/disconnect cycles and cleanup during connection attempts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
d9914b74b2
commit
2c1b37877d
@ -23,6 +23,7 @@ export class RealBLEManager implements IBLEManager {
|
||||
private connectionStates = new Map<string, BLEDeviceConnection>();
|
||||
private eventListeners: BLEEventListener[] = [];
|
||||
private scanning = false;
|
||||
private connectingDevices = new Set<string>(); // Track devices currently being connected
|
||||
|
||||
// Lazy initialization to prevent crash on app startup
|
||||
private get manager(): BleManager {
|
||||
@ -171,6 +172,28 @@ export class RealBLEManager implements IBLEManager {
|
||||
|
||||
async connectDevice(deviceId: string): Promise<boolean> {
|
||||
try {
|
||||
// Check if connection is already in progress
|
||||
if (this.connectingDevices.has(deviceId)) {
|
||||
throw new Error('Connection already in progress for this device');
|
||||
}
|
||||
|
||||
// Check if already connected
|
||||
const existingDevice = this.connectedDevices.get(deviceId);
|
||||
if (existingDevice) {
|
||||
const isConnected = await existingDevice.isConnected();
|
||||
if (isConnected) {
|
||||
this.updateConnectionState(deviceId, BLEConnectionState.READY, existingDevice.name || undefined);
|
||||
this.emitEvent(deviceId, 'ready');
|
||||
return true;
|
||||
}
|
||||
// Device was in map but disconnected, remove it
|
||||
this.connectedDevices.delete(deviceId);
|
||||
this.updateConnectionState(deviceId, BLEConnectionState.DISCONNECTED);
|
||||
}
|
||||
|
||||
// Mark device as connecting
|
||||
this.connectingDevices.add(deviceId);
|
||||
|
||||
// Update state to CONNECTING
|
||||
this.updateConnectionState(deviceId, BLEConnectionState.CONNECTING);
|
||||
|
||||
@ -192,20 +215,6 @@ export class RealBLEManager implements IBLEManager {
|
||||
throw new Error(error);
|
||||
}
|
||||
|
||||
// Check if already connected
|
||||
const existingDevice = this.connectedDevices.get(deviceId);
|
||||
if (existingDevice) {
|
||||
const isConnected = await existingDevice.isConnected();
|
||||
if (isConnected) {
|
||||
this.updateConnectionState(deviceId, BLEConnectionState.READY, existingDevice.name || undefined);
|
||||
this.emitEvent(deviceId, 'ready');
|
||||
return true;
|
||||
}
|
||||
// Device was in map but disconnected, remove it
|
||||
this.connectedDevices.delete(deviceId);
|
||||
this.updateConnectionState(deviceId, BLEConnectionState.DISCONNECTED);
|
||||
}
|
||||
|
||||
const device = await this.manager.connectToDevice(deviceId, {
|
||||
timeout: 10000, // 10 second timeout
|
||||
});
|
||||
@ -238,6 +247,9 @@ export class RealBLEManager implements IBLEManager {
|
||||
this.updateConnectionState(deviceId, BLEConnectionState.ERROR, undefined, errorMessage);
|
||||
this.emitEvent(deviceId, 'connection_failed', { error: errorMessage });
|
||||
return false;
|
||||
} finally {
|
||||
// Always remove from connecting set when done (success or failure)
|
||||
this.connectingDevices.delete(deviceId);
|
||||
}
|
||||
}
|
||||
|
||||
@ -568,9 +580,10 @@ export class RealBLEManager implements IBLEManager {
|
||||
}
|
||||
}
|
||||
|
||||
// Clear the maps
|
||||
// Clear the maps and sets
|
||||
this.connectedDevices.clear();
|
||||
this.connectionStates.clear();
|
||||
this.connectingDevices.clear();
|
||||
|
||||
// Clear event listeners
|
||||
this.eventListeners = [];
|
||||
|
||||
@ -17,6 +17,7 @@ export class MockBLEManager implements IBLEManager {
|
||||
private connectedDevices = new Set<string>();
|
||||
private connectionStates = new Map<string, BLEDeviceConnection>();
|
||||
private eventListeners: BLEEventListener[] = [];
|
||||
private connectingDevices = new Set<string>(); // Track devices currently being connected
|
||||
private mockDevices: WPDevice[] = [
|
||||
{
|
||||
id: 'mock-743',
|
||||
@ -116,6 +117,21 @@ export class MockBLEManager implements IBLEManager {
|
||||
|
||||
async connectDevice(deviceId: string): Promise<boolean> {
|
||||
try {
|
||||
// Check if connection is already in progress
|
||||
if (this.connectingDevices.has(deviceId)) {
|
||||
throw new Error('Connection already in progress for this device');
|
||||
}
|
||||
|
||||
// Check if already connected
|
||||
if (this.connectedDevices.has(deviceId)) {
|
||||
this.updateConnectionState(deviceId, BLEConnectionState.READY);
|
||||
this.emitEvent(deviceId, 'ready');
|
||||
return true;
|
||||
}
|
||||
|
||||
// Mark device as connecting
|
||||
this.connectingDevices.add(deviceId);
|
||||
|
||||
this.updateConnectionState(deviceId, BLEConnectionState.CONNECTING);
|
||||
await delay(500);
|
||||
|
||||
@ -135,6 +151,9 @@ export class MockBLEManager implements IBLEManager {
|
||||
this.updateConnectionState(deviceId, BLEConnectionState.ERROR, undefined, errorMessage);
|
||||
this.emitEvent(deviceId, 'connection_failed', { error: errorMessage });
|
||||
return false;
|
||||
} finally {
|
||||
// Always remove from connecting set when done (success or failure)
|
||||
this.connectingDevices.delete(deviceId);
|
||||
}
|
||||
}
|
||||
|
||||
@ -219,6 +238,7 @@ export class MockBLEManager implements IBLEManager {
|
||||
|
||||
this.connectedDevices.clear();
|
||||
this.connectionStates.clear();
|
||||
this.connectingDevices.clear();
|
||||
this.eventListeners = [];
|
||||
}
|
||||
}
|
||||
|
||||
256
services/ble/__tests__/BLEManager.concurrent.test.ts
Normal file
256
services/ble/__tests__/BLEManager.concurrent.test.ts
Normal file
@ -0,0 +1,256 @@
|
||||
/**
|
||||
* Tests for BLE Concurrent Connection Protection
|
||||
*/
|
||||
|
||||
import { MockBLEManager } from '../MockBLEManager';
|
||||
import { BLEConnectionState } from '../types';
|
||||
|
||||
describe('BLE Concurrent Connection Protection', () => {
|
||||
let manager: MockBLEManager;
|
||||
|
||||
beforeEach(() => {
|
||||
manager = new MockBLEManager();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await manager.cleanup();
|
||||
});
|
||||
|
||||
describe('Concurrent Connection Prevention', () => {
|
||||
it('should prevent concurrent connection attempts to the same device', async () => {
|
||||
const deviceId = 'test-device';
|
||||
|
||||
// Start two connection attempts simultaneously
|
||||
const connection1Promise = manager.connectDevice(deviceId);
|
||||
const connection2Promise = manager.connectDevice(deviceId);
|
||||
|
||||
// Wait for both to complete
|
||||
const [result1, result2] = await Promise.all([connection1Promise, connection2Promise]);
|
||||
|
||||
// One should succeed, one should fail
|
||||
expect(result1 !== result2).toBe(true);
|
||||
|
||||
// The device should end up connected (from the successful attempt)
|
||||
const finalState = manager.getConnectionState(deviceId);
|
||||
expect([BLEConnectionState.READY, BLEConnectionState.ERROR]).toContain(finalState);
|
||||
});
|
||||
|
||||
it('should allow reconnection after first connection completes', async () => {
|
||||
const deviceId = 'test-device';
|
||||
|
||||
// First connection
|
||||
const result1 = await manager.connectDevice(deviceId);
|
||||
expect(result1).toBe(true);
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.READY);
|
||||
|
||||
// Disconnect
|
||||
await manager.disconnectDevice(deviceId);
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.DISCONNECTED);
|
||||
|
||||
// Second connection should succeed
|
||||
const result2 = await manager.connectDevice(deviceId);
|
||||
expect(result2).toBe(true);
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.READY);
|
||||
});
|
||||
|
||||
it('should handle multiple concurrent connections to different devices', async () => {
|
||||
const deviceIds = ['device-1', 'device-2', 'device-3'];
|
||||
|
||||
// Connect to all devices simultaneously
|
||||
const connectionPromises = deviceIds.map(id => manager.connectDevice(id));
|
||||
const results = await Promise.all(connectionPromises);
|
||||
|
||||
// All connections should succeed
|
||||
expect(results).toEqual([true, true, true]);
|
||||
|
||||
// All devices should be in READY state
|
||||
deviceIds.forEach(id => {
|
||||
expect(manager.getConnectionState(id)).toBe(BLEConnectionState.READY);
|
||||
});
|
||||
});
|
||||
|
||||
it('should allow connection if device is already connected', async () => {
|
||||
const deviceId = 'test-device';
|
||||
|
||||
// First connection
|
||||
const result1 = await manager.connectDevice(deviceId);
|
||||
expect(result1).toBe(true);
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.READY);
|
||||
|
||||
// Second connection attempt while already connected should return true
|
||||
const result2 = await manager.connectDevice(deviceId);
|
||||
expect(result2).toBe(true);
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.READY);
|
||||
});
|
||||
|
||||
it('should emit connection_failed event for concurrent connection attempt', async () => {
|
||||
const deviceId = 'test-device';
|
||||
const events: Array<{ event: string; error?: string }> = [];
|
||||
|
||||
const listener = (id: string, event: string, data?: any) => {
|
||||
if (id === deviceId) {
|
||||
events.push({ event, error: data?.error });
|
||||
}
|
||||
};
|
||||
|
||||
manager.addEventListener(listener);
|
||||
|
||||
// Start two connection attempts simultaneously
|
||||
await Promise.all([
|
||||
manager.connectDevice(deviceId),
|
||||
manager.connectDevice(deviceId),
|
||||
]);
|
||||
|
||||
// Should have at least one connection_failed event
|
||||
const failedEvents = events.filter(e => e.event === 'connection_failed');
|
||||
expect(failedEvents.length).toBeGreaterThanOrEqual(1);
|
||||
|
||||
// The error should mention concurrent connection
|
||||
const concurrentError = failedEvents.find(e =>
|
||||
e.error?.includes('Connection already in progress')
|
||||
);
|
||||
expect(concurrentError).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Connection State During Concurrent Attempts', () => {
|
||||
it('should maintain consistent state during concurrent attempts', async () => {
|
||||
const deviceId = 'test-device';
|
||||
const states: BLEConnectionState[] = [];
|
||||
|
||||
const listener = (id: string, event: string, data?: any) => {
|
||||
if (id === deviceId && event === 'state_changed') {
|
||||
states.push(data.state);
|
||||
}
|
||||
};
|
||||
|
||||
manager.addEventListener(listener);
|
||||
|
||||
// Start concurrent connections
|
||||
await Promise.all([
|
||||
manager.connectDevice(deviceId),
|
||||
manager.connectDevice(deviceId),
|
||||
]);
|
||||
|
||||
// States should follow valid transitions
|
||||
// Should not have duplicate CONNECTING states (only one should enter)
|
||||
const connectingCount = states.filter(s => s === BLEConnectionState.CONNECTING).length;
|
||||
expect(connectingCount).toBeLessThanOrEqual(2); // Max 2: one success, one immediate error
|
||||
|
||||
// Final state should be either READY or ERROR
|
||||
const finalState = manager.getConnectionState(deviceId);
|
||||
expect([BLEConnectionState.READY, BLEConnectionState.ERROR]).toContain(finalState);
|
||||
});
|
||||
|
||||
it('should track connection attempts correctly with getAllConnections', async () => {
|
||||
const deviceId = 'test-device';
|
||||
|
||||
// Start concurrent connections
|
||||
await Promise.all([
|
||||
manager.connectDevice(deviceId),
|
||||
manager.connectDevice(deviceId),
|
||||
]);
|
||||
|
||||
const connections = manager.getAllConnections();
|
||||
const deviceConnection = connections.get(deviceId);
|
||||
|
||||
expect(deviceConnection).toBeDefined();
|
||||
expect(deviceConnection?.deviceId).toBe(deviceId);
|
||||
expect([BLEConnectionState.READY, BLEConnectionState.ERROR]).toContain(deviceConnection?.state);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Cleanup During Connection', () => {
|
||||
it('should properly cleanup concurrent connection attempts', async () => {
|
||||
const deviceIds = ['device-1', 'device-2', 'device-3'];
|
||||
|
||||
// Start connections
|
||||
const connectionPromises = deviceIds.map(id => manager.connectDevice(id));
|
||||
|
||||
// Don't wait for connections, cleanup immediately
|
||||
await manager.cleanup();
|
||||
|
||||
// All connections should be cleaned up
|
||||
const connections = manager.getAllConnections();
|
||||
expect(connections.size).toBe(0);
|
||||
|
||||
// All devices should be disconnected
|
||||
deviceIds.forEach(id => {
|
||||
expect(manager.getConnectionState(id)).toBe(BLEConnectionState.DISCONNECTED);
|
||||
});
|
||||
});
|
||||
|
||||
it('should clear connecting devices set on cleanup', async () => {
|
||||
const deviceId = 'test-device';
|
||||
|
||||
// Start connection (but don't await)
|
||||
const connectionPromise = manager.connectDevice(deviceId);
|
||||
|
||||
// Cleanup before connection completes
|
||||
await manager.cleanup();
|
||||
|
||||
// Try to connect again after cleanup - should work
|
||||
const result = await manager.connectDevice(deviceId);
|
||||
expect(result).toBe(true);
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.READY);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Edge Cases', () => {
|
||||
it('should handle rapid connect/disconnect/connect cycles', async () => {
|
||||
const deviceId = 'test-device';
|
||||
|
||||
// Connect
|
||||
await manager.connectDevice(deviceId);
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.READY);
|
||||
|
||||
// Disconnect
|
||||
await manager.disconnectDevice(deviceId);
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.DISCONNECTED);
|
||||
|
||||
// Reconnect immediately
|
||||
const result = await manager.connectDevice(deviceId);
|
||||
expect(result).toBe(true);
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.READY);
|
||||
});
|
||||
|
||||
it('should handle connection attempt during disconnection', async () => {
|
||||
const deviceId = 'test-device';
|
||||
|
||||
// Connect first
|
||||
await manager.connectDevice(deviceId);
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.READY);
|
||||
|
||||
// Start disconnection (don't await)
|
||||
const disconnectPromise = manager.disconnectDevice(deviceId);
|
||||
|
||||
// Try to connect while disconnecting
|
||||
const connectResult = await manager.connectDevice(deviceId);
|
||||
|
||||
// Wait for disconnect to complete
|
||||
await disconnectPromise;
|
||||
|
||||
// Connection attempt should have a defined result
|
||||
expect(typeof connectResult).toBe('boolean');
|
||||
});
|
||||
|
||||
it('should handle multiple rapid concurrent attempts', async () => {
|
||||
const deviceId = 'test-device';
|
||||
|
||||
// Start 5 concurrent connection attempts
|
||||
const attempts = Array(5).fill(null).map(() => manager.connectDevice(deviceId));
|
||||
const results = await Promise.all(attempts);
|
||||
|
||||
// At least one should succeed or all should fail consistently
|
||||
const successCount = results.filter(r => r === true).length;
|
||||
const failureCount = results.filter(r => r === false).length;
|
||||
|
||||
expect(successCount + failureCount).toBe(5);
|
||||
|
||||
// If any succeeded, device should be connected
|
||||
if (successCount > 0) {
|
||||
expect(manager.getConnectionState(deviceId)).toBe(BLEConnectionState.READY);
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
x
Reference in New Issue
Block a user