From 657f7ef28924adc26c3935af9d461b78b4536290 Mon Sep 17 00:00:00 2001 From: Gregor MacLennan Date: Thu, 25 Jun 2026 14:40:43 +0100 Subject: [PATCH 1/2] fix(android): shutdown IPC socket on JS reload so backend cleans up subscriptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On an RN JS-thread reload the main process stays alive, so the NodeJSIPC LocalSocket is only closed if ComapeoCoreModule.OnDestroy closes it. OnDestroy called disconnect(), which routes socket.close() through connectJob.cancelAndJoin() — but the receive coroutine is parked in a blocking readFully that only unblocks once the socket closes, so the join deadlocks and the fd never closes. The backend then never observes EOF, so its per-connection rpc-reflector cleanup never runs and every prior session's subscriptions stay attached to the singleton MapeoManager, emitting events into dead peers. Verified on device: the connection and manager-listener counts climb 1→2→3→4→5, one per reload. Add NodeJSIPC.close(): a synchronous terminal teardown that shutdown(2)s the socket (shutdownInput wakes the blocked read, shutdownOutput sends FIN to the peer) before close(2), then cancels the scope. OnDestroy now calls close() instead of disconnect(). shutdown-before-close is the same order iOS's disconnect() already uses, so iOS needs no change (comment only). Plain close() without shutdown does NOT fix the leak — the blocked readFully keeps the connection alive (verified: still climbs 1→5). Verified on Pixel_7a_API_34 and the iOS simulator: reloads now show a clean socket close + reopen with connection and listener counts bounded at 1. closeReleasesSocketSynchronously instrumented test added; full NodeJSIPCTest suite passes. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../java/com/comapeo/core/NodeJSIPCTest.kt | 39 +++++++++++++++++++ .../com/comapeo/core/ComapeoCoreModule.kt | 9 ++++- .../main/java/com/comapeo/core/NodeJSIPC.kt | 24 ++++++++++++ ios/ComapeoCoreModule.swift | 5 +++ 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/android/src/androidTest/java/com/comapeo/core/NodeJSIPCTest.kt b/android/src/androidTest/java/com/comapeo/core/NodeJSIPCTest.kt index 65ef8886..74f2f530 100644 --- a/android/src/androidTest/java/com/comapeo/core/NodeJSIPCTest.kt +++ b/android/src/androidTest/java/com/comapeo/core/NodeJSIPCTest.kt @@ -288,6 +288,45 @@ class NodeJSIPCTest { Thread.sleep(500) } + /** + * Pins the synchronous-close contract: after [NodeJSIPC.close] returns, the + * peer's blocking read has already observed EOF. A regression to the + * `disconnect()` teardown — which closes the socket behind + * `cancelAndJoin()` of a receive loop parked in a blocking `readFully` — + * would deadlock and leave the read blocked, leaking the FD across reloads. + */ + @Test + fun closeReleasesSocketSynchronously() { + val connected = CountDownLatch(1) + val readResult = java.util.concurrent.atomic.AtomicInteger(Int.MIN_VALUE) + val readReturned = CountDownLatch(1) + + startMockServer { input, _ -> + connected.countDown() + try { + readResult.set(input.read()) + } catch (e: IOException) { + // Some kernels surface peer-close as IOException; treat as EOF. + readResult.set(-1) + } + readReturned.countDown() + } + + val ipc = NodeJSIPC(socketFile) { msg -> receivedMessages.add(msg) } + assertTrue("Should connect within 10s", connected.await(10, TimeUnit.SECONDS)) + // Let the receive loop settle so we test steady-state close, not a + // connect-cancel race. + Thread.sleep(200) + + ipc.close() + + assertTrue( + "Server-side read must unblock with EOF within 1s of close() returning", + readReturned.await(1, TimeUnit.SECONDS) + ) + assertEquals(-1, readResult.get()) + } + @Test fun handlesServerDisconnect() { val connected = CountDownLatch(1) diff --git a/android/src/main/java/com/comapeo/core/ComapeoCoreModule.kt b/android/src/main/java/com/comapeo/core/ComapeoCoreModule.kt index 502997b2..f38b2a58 100644 --- a/android/src/main/java/com/comapeo/core/ComapeoCoreModule.kt +++ b/android/src/main/java/com/comapeo/core/ComapeoCoreModule.kt @@ -139,8 +139,13 @@ class ComapeoCoreModule : Module() { } OnDestroy { - ipc.disconnect() - controlIpc.disconnect() + // OnCreate/OnDestroy bind to the Expo AppContext (JS-runtime + // lifetime), so they fire on every JS reload while this process + // stays alive. Close synchronously: the backend must see EOF on the + // old socket before the next OnCreate connects, otherwise the FD + // lingers and rpc-reflector listeners leak onto MapeoManager. + ipc.close() + controlIpc.close() } OnActivityEntersForeground { diff --git a/android/src/main/java/com/comapeo/core/NodeJSIPC.kt b/android/src/main/java/com/comapeo/core/NodeJSIPC.kt index 52efc197..c90b2bc3 100644 --- a/android/src/main/java/com/comapeo/core/NodeJSIPC.kt +++ b/android/src/main/java/com/comapeo/core/NodeJSIPC.kt @@ -8,6 +8,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay @@ -217,6 +218,29 @@ class NodeJSIPC( connect() sendChannel.trySend(message) } + + /** + * Synchronous, terminal teardown for JS reload (process stays alive, so the + * fd must be closed here or it leaks until process death). `shutdown` must + * precede `close`: the receive loop is parked in a blocking `readFully` that + * holds the socket open until it returns, so `close()` alone never reaches + * the peer — `shutdownInput` wakes the read, `shutdownOutput` sends FIN. + * Not reusable after close; construct a new instance. + */ + fun close() { + scope.cancel() + sendChannel.close() + if (::socket.isInitialized) { + try { socket.shutdownInput() } catch (_: Exception) {} + try { socket.shutdownOutput() } catch (_: Exception) {} + } + try { dataOutputStream?.close() } catch (_: Exception) {} + try { dataInputStream?.close() } catch (_: Exception) {} + if (::socket.isInitialized) { + try { socket.close() } catch (_: Exception) {} + } + state.value = State.Disconnected + } } /** diff --git a/ios/ComapeoCoreModule.swift b/ios/ComapeoCoreModule.swift index c90db167..39d3f52d 100644 --- a/ios/ComapeoCoreModule.swift +++ b/ios/ComapeoCoreModule.swift @@ -50,6 +50,11 @@ public class ComapeoCoreModule: Module { } OnDestroy { + // Fires on every JS reload as of expo-modules-core SDK 53+ (weak + // MainValueConverter.appContext lets AppContext deinit; verified on + // 56). No iOS equivalent of the Android close() is needed: iOS + // disconnect() shutdown(2)s before joining the receive loop, so the + // backend sees EOF synchronously without the cancelAndJoin deadlock. self.ipc?.disconnect() self.ipc = nil } From f05170a1e2ac619af995d3c2d87a665248b18e44 Mon Sep 17 00:00:00 2001 From: Gregor MacLennan Date: Thu, 25 Jun 2026 16:40:48 +0100 Subject: [PATCH 2/2] fix(android): wake blocked read in disconnect() and tidy close() teardown Generalize the shutdown-before-close fix into disconnect(): shutdown the socket to wake the receive loop parked in a blocking readFully before cancelAndJoin, so the join can't deadlock on a live but idle node backend. The deadlock is unreachable on today's callers (they run after the backend has exited), so this is hardening that keeps disconnect() correct in isolation and symmetric with close(). In close(), set the terminal Disconnected state right after scope.cancel() so the woken receive loop's disconnect() short-circuits on its state guard instead of relaunching teardown on the cancelled scope. Extract shared shutdownSocket()/closeStreamsAndSocket() helpers used by both paths so the two teardowns can't drift. --- .../main/java/com/comapeo/core/NodeJSIPC.kt | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/android/src/main/java/com/comapeo/core/NodeJSIPC.kt b/android/src/main/java/com/comapeo/core/NodeJSIPC.kt index c90b2bc3..aa06b5fc 100644 --- a/android/src/main/java/com/comapeo/core/NodeJSIPC.kt +++ b/android/src/main/java/com/comapeo/core/NodeJSIPC.kt @@ -189,11 +189,15 @@ class NodeJSIPC( } } } + // `shutdown` before `cancelAndJoin`: the receive loop is parked in a + // blocking `readFully` that `cancelAndJoin` cannot interrupt, so without + // first waking it the join blocks until the node backend sends a message + // or closes the socket — a deadlock when node is connected but idle. + // Same fix as close(). + shutdownSocket() connectJob?.cancelAndJoin() connectJob = null - try { dataOutputStream?.close() } catch (_: Exception) {} - try { dataInputStream?.close() } catch (_: Exception) {} - try { socket.close() } catch (_: Exception) {} + closeStreamsAndSocket() } disconnectJob.invokeOnCompletion { cause -> state.value = when (cause) { @@ -224,22 +228,38 @@ class NodeJSIPC( * fd must be closed here or it leaks until process death). `shutdown` must * precede `close`: the receive loop is parked in a blocking `readFully` that * holds the socket open until it returns, so `close()` alone never reaches - * the peer — `shutdownInput` wakes the read, `shutdownOutput` sends FIN. + * the node backend — `shutdownInput` wakes the read, `shutdownOutput` sends FIN. * Not reusable after close; construct a new instance. */ fun close() { scope.cancel() + // Mark terminal before shutdownSocket() wakes the receive loop's blocking + // readFully: its IOException handler calls disconnect(), which then + // short-circuits on this state guard instead of relaunching teardown on + // the now-cancelled scope. Set after scope.cancel() so the (already + // cancelled) state collector still doesn't forward this transition, + // matching the prior terminal-close semantics. + state.value = State.Disconnected sendChannel.close() + shutdownSocket() + closeStreamsAndSocket() + } + + // `shutdown` (not `close`) wakes the receive loop parked in a blocking + // readFully and sends FIN; it must precede closeStreamsAndSocket(). + private fun shutdownSocket() { if (::socket.isInitialized) { try { socket.shutdownInput() } catch (_: Exception) {} try { socket.shutdownOutput() } catch (_: Exception) {} } + } + + private fun closeStreamsAndSocket() { try { dataOutputStream?.close() } catch (_: Exception) {} try { dataInputStream?.close() } catch (_: Exception) {} if (::socket.isInitialized) { try { socket.close() } catch (_: Exception) {} } - state.value = State.Disconnected } }