fix: centralize and strictly enforce connection limits (#618)

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
-            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
+            // Centralized limit enforcement on any setting change
-                    // (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 {
+                combine(
-                    if (!isActive) return@collect
+                    dbg.maxConnectionsOverall,
-                    connectionTracker.enforceConnectionLimits()
+                    dbg.maxServerConnections,
-                }
+                    dbg.maxClientConnections
-            }
+                ) { _, _, _ -> 
-            connectionScope.launch {
+                    // We don't need the values here, we just need to trigger enforcement
-                dbg.maxServerConnections.collect { maxServer ->
+                    Unit 
-                    if (!isActive) return@collect
+                }.collect {
-                    // Enforce server specific limit
+                    if (isActive) {
-                    serverManager.enforceServerLimit(maxServer)
+                        enforceStrictLimits()
-                    
-                    // 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)
                     }
                 }
             }
         } 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()
+            enforceStrictLimits()
-            // Best-effort server cap
-            try {
-                val maxServer = com.bitchat.android.ui.debug.DebugSettingsManager.getInstance().maxServerConnections.value
-                serverManager.enforceServerLimit(maxServer)
-            } catch (_: Exception) { }
         }
     }
     

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 {
+    fun canConnectAsClient(maxOverall: Int, maxClient: Int): Boolean {
-        // Respect debug override if set
+        val total = connectedDevices.size
-        val dbg = try { com.bitchat.android.ui.debug.DebugSettingsManager.getInstance() } catch (_: Exception) { null }
+        val clients = connectedDevices.values.count { it.isClient }
-        val maxConnections = dbg?.maxConnectionsOverall?.value ?: powerManager.getMaxConnections()
+        return total < maxOverall && clients < maxClient
-        return connectedDevices.size >= maxConnections
     }
     
     /**
-     * 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() {
+    fun getConnectionsToEvict(maxOverall: Int, maxServer: Int, maxClient: Int): List<DeviceConnection> {
-        // Read debug overrides if available
+        val toEvict = mutableSetOf<DeviceConnection>()
-        val dbg = try { com.bitchat.android.ui.debug.DebugSettingsManager.getInstance() } catch (_: Exception) { null }
+        val currentDevices = connectedDevices.values.toList()
-        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 }
+        // 1. Enforce Role Limits
-
+        val clients = currentDevices.filter { it.isClient }.sortedBy { it.connectedAt }
-        // Enforce client cap first (we can actively disconnect)
         if (clients.size > maxClient) {
-            Log.i(TAG, "Enforcing client cap: ${clients.size} > $maxClient")
+            toEvict.addAll(clients.take(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
+        val servers = currentDevices.filter { !it.isClient }.sortedBy { it.connectedAt }
-        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<BluetoothDevice> {
-        val servers = connectedDevices.values.filter { !it.isClient }
-        val excessList = mutableListOf<BluetoothDevice>()
-
-        // 1. Check server specific limit
         if (servers.size > maxServer) {
-            val excessCount = servers.size - maxServer
+            toEvict.addAll(servers.take(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)
+        // 2. Enforce Overall Limit
-        // We need to count how many connections we will have after the above removals
+        // Count how many would remain after the above evictions
-        val currentTotal = connectedDevices.size
+        val remaining = currentDevices.filter { !toEvict.contains(it) }
-        val plannedRemovals = excessList.size
+        if (remaining.size > maxOverall) {
-        val projectedTotal = currentTotal - plannedRemovals
+            val excessCount = remaining.size - maxOverall
             
-        if (projectedTotal > maxOverall) {
+            // Explicitly prefer evicting clients first
-            val furtherExcess = projectedTotal - maxOverall
+            val clientCandidates = remaining.filter { it.isClient }.sortedBy { it.connectedAt }
-            // We can only remove servers here. Clients are handled in enforceConnectionLimits.
+            val serverCandidates = remaining.filter { !it.isClient }.sortedBy { it.connectedAt }
-            // Filter out devices we already planned to remove
+            
-            val remainingServers = servers.filter { s -> excessList.none { it.address == s.device.address } }
+            var needed = excessCount
-            val toRemove = remainingServers.sortedBy { it.connectedAt }.take(furtherExcess)
+            
-            excessList.addAll(toRemove.map { it.device })
+            // 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 excessList
+        return toEvict.toList()
     }
     
     /**

app/src/main/java/com/bitchat/android/mesh/BluetoothGattClientManager.kt

@@ -381,8 +381,13 @@ class BluetoothGattClientManager(
             return
         }
         
-        if (connectionTracker.isConnectionLimitReached()) {
+        // Check if connection limit is reached
-            Log.d(TAG, "Connection limit reached (${powerManager.getMaxConnections()})")
+        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
         }
         

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)
      */