From c64ea0a0218214a406319e8fd2a80ac7e5312b09 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Thu, 15 Jan 2026 16:00:48 +0700 Subject: [PATCH] fix: centralize and strictly enforce connection limits (#618) --- .../mesh/BluetoothConnectionManager.kt | 89 +++++++------ .../mesh/BluetoothConnectionTracker.kt | 120 +++++++----------- .../mesh/BluetoothGattClientManager.kt | 9 +- .../mesh/BluetoothGattServerManager.kt | 16 --- 4 files changed, 103 insertions(+), 131 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/mesh/BluetoothConnectionManager.kt b/app/src/main/java/com/bitchat/android/mesh/BluetoothConnectionManager.kt index b982f199..2e78ac28 100644 --- a/app/src/main/java/com/bitchat/android/mesh/BluetoothConnectionManager.kt +++ b/app/src/main/java/com/bitchat/android/mesh/BluetoothConnectionManager.kt @@ -7,6 +7,7 @@ import com.bitchat.android.model.RoutedPacket import com.bitchat.android.protocol.BitchatPacket import kotlinx.coroutines.* import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.combine /** * Power-optimized Bluetooth connection manager with comprehensive memory management @@ -57,6 +58,8 @@ class BluetoothConnectionManager( } override fun onDeviceConnected(device: BluetoothDevice) { + // Trigger limit enforcement immediately upon any new connection + enforceStrictLimits() delegate?.onDeviceConnected(device) } @@ -103,47 +106,58 @@ class BluetoothConnectionManager( if (enabled) startClient() else stopClient() } } - // Connection caps: enforce on change + + // Centralized limit enforcement on any setting change connectionScope.launch { - dbg.maxConnectionsOverall.collect { maxOverall -> - if (!isActive) return@collect - // 1. Enforce client limits (handled by tracker) - connectionTracker.enforceConnectionLimits() - - // 2. Enforce overall limit on server connections if needed - // (Tracker knows about all connections but can't disconnect servers directly) - val maxServer = dbg.maxServerConnections.value - val excessServers = connectionTracker.getExcessServerConnections(maxServer, maxOverall) - excessServers.forEach { device -> - Log.d(TAG, "Disconnecting server ${device.address} due to overall cap") - serverManager.disconnectDevice(device) - } - } - } - connectionScope.launch { - dbg.maxClientConnections.collect { - if (!isActive) return@collect - connectionTracker.enforceConnectionLimits() - } - } - connectionScope.launch { - dbg.maxServerConnections.collect { maxServer -> - if (!isActive) return@collect - // Enforce server specific limit - serverManager.enforceServerLimit(maxServer) - - // Also check if this change puts us over the overall limit - val maxOverall = dbg.maxConnectionsOverall.value - val excessServers = connectionTracker.getExcessServerConnections(maxServer, maxOverall) - excessServers.forEach { device -> - Log.d(TAG, "Disconnecting server ${device.address} due to overall cap") - serverManager.disconnectDevice(device) + combine( + dbg.maxConnectionsOverall, + dbg.maxServerConnections, + dbg.maxClientConnections + ) { _, _, _ -> + // We don't need the values here, we just need to trigger enforcement + Unit + }.collect { + if (isActive) { + enforceStrictLimits() } } } } catch (_: Exception) { } } + /** + * Centralized connection limit enforcement + */ + private fun enforceStrictLimits() { + if (!isActive) return + + try { + val dbg = com.bitchat.android.ui.debug.DebugSettingsManager.getInstance() + val maxOverall = dbg.maxConnectionsOverall.value + val maxServer = dbg.maxServerConnections.value + val maxClient = dbg.maxClientConnections.value + + // Get list of connections to evict to satisfy all constraints + val toEvict = connectionTracker.getConnectionsToEvict(maxOverall, maxServer, maxClient) + + if (toEvict.isNotEmpty()) { + Log.i(TAG, "Enforcing limits (max: $maxOverall, s: $maxServer, c: $maxClient) - evicting ${toEvict.size} connections") + + toEvict.forEach { conn -> + if (conn.isClient) { + Log.d(TAG, "Evicting client ${conn.device.address}") + try { conn.gatt?.disconnect() } catch (_: Exception) { } + } else { + Log.d(TAG, "Evicting server ${conn.device.address}") + serverManager.disconnectDevice(conn.device) + } + } + } + } catch (e: Exception) { + Log.e(TAG, "Error enforcing limits: ${e.message}") + } + } + /** * Start all Bluetooth services with power optimization */ @@ -408,12 +422,7 @@ class BluetoothConnectionManager( } // Enforce connection limits - connectionTracker.enforceConnectionLimits() - // Best-effort server cap - try { - val maxServer = com.bitchat.android.ui.debug.DebugSettingsManager.getInstance().maxServerConnections.value - serverManager.enforceServerLimit(maxServer) - } catch (_: Exception) { } + enforceStrictLimits() } } diff --git a/app/src/main/java/com/bitchat/android/mesh/BluetoothConnectionTracker.kt b/app/src/main/java/com/bitchat/android/mesh/BluetoothConnectionTracker.kt index 59fca7aa..681185d5 100644 --- a/app/src/main/java/com/bitchat/android/mesh/BluetoothConnectionTracker.kt +++ b/app/src/main/java/com/bitchat/android/mesh/BluetoothConnectionTracker.kt @@ -237,86 +237,60 @@ class BluetoothConnectionTracker( * Check if connection limit is reached */ /** - * Check if connection limit is reached + * Check if a new client connection is allowed based on limits */ - fun isConnectionLimitReached(): Boolean { - // Respect debug override if set - val dbg = try { com.bitchat.android.ui.debug.DebugSettingsManager.getInstance() } catch (_: Exception) { null } - val maxConnections = dbg?.maxConnectionsOverall?.value ?: powerManager.getMaxConnections() - return connectedDevices.size >= maxConnections + fun canConnectAsClient(maxOverall: Int, maxClient: Int): Boolean { + val total = connectedDevices.size + val clients = connectedDevices.values.count { it.isClient } + return total < maxOverall && clients < maxClient } /** - * Enforce connection limits by disconnecting oldest connections + * Calculate which connections should be evicted to satisfy limits. + * Logic: + * 1. Enforce strict role limits (maxClient, maxServer) - evict oldest excess. + * 2. Enforce overall limit (maxOverall) - evict oldest remaining, preferring clients. */ - fun enforceConnectionLimits() { - // Read debug overrides if available - val dbg = try { com.bitchat.android.ui.debug.DebugSettingsManager.getInstance() } catch (_: Exception) { null } - val maxOverall = dbg?.maxConnectionsOverall?.value ?: powerManager.getMaxConnections() - val maxClient = dbg?.maxClientConnections?.value ?: maxOverall - // Note: maxServer is handled by GattServerManager, but we need to respect overall limit here too - - val clients = connectedDevices.values.filter { it.isClient } - - // Enforce client cap first (we can actively disconnect) - if (clients.size > maxClient) { - Log.i(TAG, "Enforcing client cap: ${clients.size} > $maxClient") - val toDisconnect = clients.sortedBy { it.connectedAt }.take(clients.size - maxClient) - toDisconnect.forEach { dc -> - Log.d(TAG, "Disconnecting client ${dc.device.address} due to client cap") - dc.gatt?.disconnect() - } - } - - // Re-check overall cap after client cleanup - if (connectedDevices.size > maxOverall) { - Log.i(TAG, "Enforcing overall cap: ${connectedDevices.size} > $maxOverall") - val excess = connectedDevices.size - maxOverall - - // Prefer disconnecting clients first to satisfy overall cap - val clientsToDisconnect = connectedDevices.values - .filter { it.isClient } - .sortedBy { it.connectedAt } - .take(excess) - - clientsToDisconnect.forEach { dc -> - Log.d(TAG, "Disconnecting client ${dc.device.address} due to overall cap") - dc.gatt?.disconnect() - } - } - } - - /** - * Get excess server connections that should be disconnected to satisfy limits. - * This allows the Manager to coordinate server disconnects since Tracker doesn't control the server. - */ - fun getExcessServerConnections(maxServer: Int, maxOverall: Int): List { - val servers = connectedDevices.values.filter { !it.isClient } - val excessList = mutableListOf() - - // 1. Check server specific limit - if (servers.size > maxServer) { - val excessCount = servers.size - maxServer - val toRemove = servers.sortedBy { it.connectedAt }.take(excessCount) - excessList.addAll(toRemove.map { it.device }) - } - - // 2. Check overall limit (considering we might have already removed some above) - // We need to count how many connections we will have after the above removals - val currentTotal = connectedDevices.size - val plannedRemovals = excessList.size - val projectedTotal = currentTotal - plannedRemovals + fun getConnectionsToEvict(maxOverall: Int, maxServer: Int, maxClient: Int): List { + val toEvict = mutableSetOf() + val currentDevices = connectedDevices.values.toList() - if (projectedTotal > maxOverall) { - val furtherExcess = projectedTotal - maxOverall - // We can only remove servers here. Clients are handled in enforceConnectionLimits. - // Filter out devices we already planned to remove - val remainingServers = servers.filter { s -> excessList.none { it.address == s.device.address } } - val toRemove = remainingServers.sortedBy { it.connectedAt }.take(furtherExcess) - excessList.addAll(toRemove.map { it.device }) + // 1. Enforce Role Limits + val clients = currentDevices.filter { it.isClient }.sortedBy { it.connectedAt } + if (clients.size > maxClient) { + toEvict.addAll(clients.take(clients.size - maxClient)) } - - return excessList + + val servers = currentDevices.filter { !it.isClient }.sortedBy { it.connectedAt } + if (servers.size > maxServer) { + toEvict.addAll(servers.take(servers.size - maxServer)) + } + + // 2. Enforce Overall Limit + // Count how many would remain after the above evictions + val remaining = currentDevices.filter { !toEvict.contains(it) } + if (remaining.size > maxOverall) { + val excessCount = remaining.size - maxOverall + + // Explicitly prefer evicting clients first + val clientCandidates = remaining.filter { it.isClient }.sortedBy { it.connectedAt } + val serverCandidates = remaining.filter { !it.isClient }.sortedBy { it.connectedAt } + + var needed = excessCount + + // Take from clients first + val fromClients = clientCandidates.take(needed) + toEvict.addAll(fromClients) + needed -= fromClients.size + + // If still need more, take from servers + if (needed > 0) { + val fromServers = serverCandidates.take(needed) + toEvict.addAll(fromServers) + } + } + + return toEvict.toList() } /** diff --git a/app/src/main/java/com/bitchat/android/mesh/BluetoothGattClientManager.kt b/app/src/main/java/com/bitchat/android/mesh/BluetoothGattClientManager.kt index 7c31e0d3..2bb22fce 100644 --- a/app/src/main/java/com/bitchat/android/mesh/BluetoothGattClientManager.kt +++ b/app/src/main/java/com/bitchat/android/mesh/BluetoothGattClientManager.kt @@ -381,8 +381,13 @@ class BluetoothGattClientManager( return } - if (connectionTracker.isConnectionLimitReached()) { - Log.d(TAG, "Connection limit reached (${powerManager.getMaxConnections()})") + // Check if connection limit is reached + val dbg = try { com.bitchat.android.ui.debug.DebugSettingsManager.getInstance() } catch (_: Exception) { null } + val maxOverall = dbg?.maxConnectionsOverall?.value ?: powerManager.getMaxConnections() + val maxClient = dbg?.maxClientConnections?.value ?: maxOverall + + if (!connectionTracker.canConnectAsClient(maxOverall, maxClient)) { + Log.d(TAG, "Client connection limit reached (overall: $maxOverall, client: $maxClient)") return } diff --git a/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt b/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt index 4a0630a2..ad7c9cf1 100644 --- a/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt +++ b/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt @@ -46,22 +46,6 @@ class BluetoothGattServerManager( // State management private var isActive = false - // Enforce a server connection limit by canceling the oldest connections (best-effort) - fun enforceServerLimit(maxServer: Int) { - if (maxServer <= 0) return - try { - // Use connection tracker to get actual connected server devices - val servers = connectionTracker.getConnectedDevices().values.filter { !it.isClient } - if (servers.size > maxServer) { - val excess = servers.size - maxServer - // Disconnect oldest - servers.sortedBy { it.connectedAt }.take(excess).forEach { d -> - try { gattServer?.cancelConnection(d.device) } catch (_: Exception) { } - } - } - } catch (_: Exception) { } - } - /** * Disconnect a specific device (used by ConnectionManager to enforce overall limits) */