fix(bindings): a closed hello-mapped link gives the peer back its live address, and each link keeps its own hello

bindings/react-native/android/src/main/java/com/offlineprotocol/ble/BleTransportFacade.kt

@@ -4546,6 +4546,22 @@ class BleTransportFacade(
                 flushPeerMtu(address, peerId)
             }
         }
+        if (isCleanDisconnect && connections.getGatt(address) == null) {
+            connections.deviceIdForAddress(address)?.let { peerId ->
+                if (connections.hasOtherLiveLink(peerId, address)) {
+                    // A hello mapped this peer at the central-role address its
+                    // link to us came from, and that became the address we send
+                    // to. With that link gone and no client link of ours there,
+                    // every fragment would queue and the drain would dial an
+                    // address that does not advertise, while our own link to the
+                    // peer sat idle. Drop the address: the registry points the
+                    // peer back at its live link, and a new link from the peer
+                    // brings a new hello.
+                    connections.removeIdentifiersForAddress(address)
+                    centralClient.clearResolutionAttempt(address)
+                }
+            }
+        }
         if (!isCleanDisconnect) {
             lastSeenRssi.remove(address)
             connections.deviceIdForAddress(address)?.let { peerId ->

bindings/react-native/android/src/main/java/com/offlineprotocol/ble/CentralGattClient.kt

@@ -261,8 +261,13 @@ internal class CentralGattClient(
     private val negotiatedPayload = ConcurrentHashMap<String, Int>()
 
     // Hello writes awaiting their callback, keyed by address, holding the
-    // watchdog that goes ready without it. BLE-thread only.
-    private val helloInFlight = HashMap<String, Runnable>()
+    // link the write went out on and the watchdog that goes ready without the
+    // callback. Keyed to the link as well as the address: a link that closes
+    // mid-hello and a new one at the same address within the watchdog window
+    // must not share an entry, or the old watchdog marks the new link ready
+    // mid-handshake and a late callback from the old link does the same.
+    // Concurrent because a disconnect arrives on a binder thread.
+    private val helloInFlight = ConcurrentHashMap<String, Pair<BluetoothGatt, Runnable>>()
 
     private val deviceIdResolutionAttempts = ConcurrentHashMap<String, Long>()
 
@@ -346,6 +351,7 @@ internal class CentralGattClient(
         deviceIdResolutionAttempts.remove(address)
         advertisedDeviceIds.remove(address)
         cancelMtuWatchdog(address)
+        cancelHelloWatchdog(address, null)
         clearServiceInstanceSelection(address)
     }
 
@@ -361,9 +367,21 @@ internal class CentralGattClient(
         serviceInstanceProbes.clear()
         appTagReadWatchdogs.values.forEach { bleHandler.removeCallbacks(it) }
         appTagReadWatchdogs.clear()
+        helloInFlight.values.forEach { bleHandler.removeCallbacks(it.second) }
+        helloInFlight.clear()
         boundServiceInstanceIds.clear()
     }
 
+    /**
+     * Drops the hello pending at [address], only if it belongs to [gatt] when
+     * one is given, and cancels its watchdog.
+     */
+    private fun cancelHelloWatchdog(address: String, gatt: BluetoothGatt?) {
+        val entry = helloInFlight[address] ?: return
+        if (gatt != null && entry.first !== gatt) return
+        if (helloInFlight.remove(address, entry)) bleHandler.removeCallbacks(entry.second)
+    }
+
     /**
      * Cancels any pending MTU watchdog for [address] and drops the
      * associated `mtuInFlight` slot. Called from every handshake
@@ -866,8 +884,11 @@ internal class CentralGattClient(
                 val address = gatt.device.address
                 bleHandler.post {
                     if (host.isShuttingDown()) return@post
-                    val watchdog = helloInFlight.remove(address) ?: return@post
-                    bleHandler.removeCallbacks(watchdog)
+                    // Only for the hello this link wrote: a link that closed
+                    // and reopened at the same address holds its own entry.
+                    val entry = helloInFlight[address] ?: return@post
+                    if (entry.first !== gatt || !helloInFlight.remove(address, entry)) return@post
+                    bleHandler.removeCallbacks(entry.second)
                     if (status != BluetoothGatt.GATT_SUCCESS) {
                         diagnosticEmitter("info", "BLE hello refused", mapOf("address" to address, "status" to status))
                     }
@@ -976,6 +997,7 @@ internal class CentralGattClient(
         val address = gatt.device.address
         val wasConnected = host.connections.getGatt(address) != null
         host.connections.removeGatt(address)
+        cancelHelloWatchdog(address, gatt)
         linkReady.remove(address)
 
         // Don't remove from discovered list - keep trying to reconnect.
@@ -1438,12 +1460,12 @@ internal class CentralGattClient(
             override fun run() {
                 // Only for this write: a later link at the same address
                 // holds its own entry.
-                if (helloInFlight[address] !== this) return
-                helloInFlight.remove(address)
+                val entry = helloInFlight[address] ?: return
+                if (entry.second !== this || !helloInFlight.remove(address, entry)) return
                 if (!host.isShuttingDown()) markLinkReady(address)
             }
         }
-        helloInFlight[address] = watchdog
+        helloInFlight[address] = gatt to watchdog
         bleHandler.postDelayed(watchdog, HELLO_WATCHDOG_MS)
     }
 
@@ -1467,6 +1489,7 @@ internal class CentralGattClient(
             Log.w(TAG, "Error closing gatt for $address ($reason)", e)
         }
         host.connections.removeGatt(address)
+        cancelHelloWatchdog(address, gatt)
         linkReady.remove(address)
         // Drop the unproven claim with the link, so a reconnect cannot pair
         // this connection's device id with the next one's identity.

bindings/react-native/android/src/test/java/com/offlineprotocol/ble/CentralGattClientHelloTest.kt

@@ -163,6 +163,37 @@ class CentralGattClientHelloTest {
         assertEquals(1, host.drains)
     }
 
+    @Test
+    fun `a link reopened at the same address keeps its own hello`() {
+        // Link one sends its hello and closes before the callback. Link two
+        // opens at the same address within the watchdog window and sends its
+        // own. Neither link one's watchdog nor a late callback from link one
+        // may mark link two ready while its hello is still in flight: the
+        // drain would then write Message fragments beside the handshake.
+        val first = link(withHello = true)
+        cccdAcked(first)
+        shadowOf(Looper.getMainLooper()).idleFor(Duration.ofSeconds(2))
+        client.callback.onConnectionStateChange(first.gatt, 19, android.bluetooth.BluetoothProfile.STATE_DISCONNECTED)
+        idle()
+        host.drains = 0
+
+        val second = link(withHello = true)
+        cccdAcked(second)
+        assertEquals("link two sent its own hello", listOf(helloUuid), second.shadow.writes.map { it.first })
+
+        client.callback.onCharacteristicWrite(first.gatt, first.hello!!, BluetoothGatt.GATT_SUCCESS)
+        idle()
+        assertEquals("a late callback from link one", 0, host.drains)
+
+        // Past link one's watchdog (3 s after its hello), short of link two's.
+        shadowOf(Looper.getMainLooper()).idleFor(Duration.ofMillis(1500))
+        assertEquals("link one's watchdog", 0, host.drains)
+
+        client.callback.onCharacteristicWrite(second.gatt, second.hello!!, BluetoothGatt.GATT_SUCCESS)
+        idle()
+        assertEquals("link two's own callback", 1, host.drains)
+    }
+
     private class FakeHost(private val hello: ByteArray?) : CentralGattClient.Host {
         var drains = 0
         override val protocol: OfflineProtocol

bindings/react-native/android/src/test/java/com/offlineprotocol/ble/StaleAddressRegistryTest.kt

@@ -106,4 +106,28 @@ class StaleAddressRegistryTest {
         assertFalse(registry.hasEstablishedLink("peerA"))
         assertEquals(setOf("server-side", "client-side"), registry.addressesForDevice("peerA").toSet())
     }
+
+    @Test
+    fun `a hello-mapped link that closes gives the peer back its live address`() {
+        // We hold a client link to peerA at its advertising address. Its
+        // central then opened a link to our server from its central-role
+        // address and wrote a hello, which made that address peerA's
+        // send address. When that link closes, sending must go back to the
+        // client link rather than to an address that does not advertise.
+        val registry = MeshConnectionRegistry()
+        registry.setDeviceIdentifier("advertising", "peerA")
+        registry.trackServerConnection("central-role")
+        registry.setDeviceIdentifier("central-role", "peerA")
+        assertEquals("central-role", registry.addressForDevice("peerA"))
+
+        registry.untrackServerConnection("central-role")
+        // A live link at the advertising address. The registry's client map
+        // needs a BluetoothGatt; a server link counts the same for liveness.
+        registry.trackServerConnection("advertising")
+        assertTrue(registry.hasOtherLiveLink("peerA", excluding = "central-role"))
+        registry.removeIdentifiersForAddress("central-role")
+
+        assertEquals("advertising", registry.addressForDevice("peerA"))
+        assertEquals(null, registry.deviceIdForAddress("central-role"))
+    }
 }

crates/offline-protocol-uniffi/src/lib.rs

@@ -14450,6 +14450,39 @@ mod tests {
         );
     }
 
+    /// A hello makes the central-role address a peer's send address. When the
+    /// link from that address closes cleanly while the peer is still up on
+    /// another link, the address must be dropped so sending goes back to the
+    /// live link. Kept, every fragment queued and the drain dialled an address
+    /// that does not advertise, while our own link to the peer sat idle.
+    #[test]
+    fn react_native_android_clean_drop_of_a_hello_mapped_link_repoints_the_peer() {
+        let facade = rn_source_code_only(
+            "android/src/main/java/com/offlineprotocol/ble/BleTransportFacade.kt",
+        );
+        let decl = "private fun handleCentralDisconnectedOnBleThread(";
+        let start = facade
+            .find(decl)
+            .expect("the facade handles a server-side disconnect")
+            + decl.len();
+        let body = &facade[start..];
+        let body = &body[..rn_kotlin_body_end(body)];
+        let rule = body
+            .find("if (isCleanDisconnect && connections.getGatt(address) == null) {")
+            .expect("a clean server-side disconnect must check for a client link at the address");
+        let rest = &body[rule..];
+        let live = rest
+            .find("connections.hasOtherLiveLink(peerId, address)")
+            .expect("the peer must still be up elsewhere before its address is dropped");
+        let drop = rest
+            .find("connections.removeIdentifiersForAddress(address)")
+            .expect("the dead central-role address must be dropped");
+        assert!(
+            live < drop,
+            "check the live link before dropping the address"
+        );
+    }
+
     /// Android hears Bluetooth go off and come back from the adapter's state
     /// broadcast, not only from a failed scan start. Polled once a minute, the
     /// transport missed a Bluetooth stack crash entirely (the stack restarts