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..aa06b5fc 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 @@ -188,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) { @@ -217,6 +222,45 @@ 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 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) {} + } + } } /** 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 }