refactor(android): retire the control socket for a bound Messenger between the FGS and main process
I maintainer di solito rispondono entro 1 giorno
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Idoneità per principianti
- 35/100
- Tipo di issue
- Refactoring
- Chiarezza
- Abbastanza chiara
- Stato di attività
- Tranquilla
- Stack tecnologico
- javascript, kotlin, node.js, swift
- Ambito
- backend, distributed-systems, mobile, testing
Direzione di ricerca
Inizia da ARCHITECTURE §§3, 4, 5.4 e 8.1, quindi segui ComapeoCoreService.onBind, NodeJSService.swift:199/646-660 e ComapeoCoreModule.kt:106-137. Esegui IPCLifecycleTests.swift e il flow esistente maestro/fgs-restart.yaml; il lavoro è completato quando sono verificati i callback del bridge, il comportamento snapshot/push/death di Messenger, gli scenari del ciclo di vita preservati, la documentazione aggiornata e la suite e2e invariata.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
After #243 and #244, control.sock carries four frames — started, ready, stopping, error — plus shutdown and error-native inbound. On iOS every one of those endpoints is in the same process as Node. On Android all of them are too, except one: the main app process, which needs lifecycle visibility into state that physically lives in the :ComapeoCore FGS.
This issue finishes the job: the in-process endpoints become addon calls, and the one genuinely cross-process channel moves from a second socket connection to a bound Messenger.
Lowest value of the three. Do it if the disconnect-inference rules are causing grief; skip it otherwise.
Why the remaining socket is the wrong shape
ARCHITECTURE §8.1 already scored this and picked the second NodeJSIPC connection. Two of its rejection reasons were premised on the current architecture and no longer hold once the addon exists:
Messenger/ bound service — Reject: moves state into Kotlin, duplicates Node's state machine, breaks "Node owns the truth"
With the addon, Node reports state to Kotlin in-process. State is already in Kotlin, delivered by Node rather than re-derived, so there is nothing to duplicate and Node still owns the truth. The objection evaporates. So does "replay logic lives in JS where the rest of the lifecycle lives" — there is no JS-side client list left to replay to.
Re-scored, the channel needs exactly three things:
- Current state on attach — the main process reloads constantly under Fast Refresh.
- Push on change.
- FGS-death detection.
A bound service gives all three: onServiceConnected → snapshot query, a reply Messenger for updates, and linkToDeath / onServiceDisconnected for liveness. Intent broadcasts give only (2) — sticky broadcasts are deprecated so (1) needs a separate query path, and there is no death signal at all. §8.1 was right to reject them and stays right. ContentProvider + ContentObserver, §8.1's runner-up, gives (1) and (2) but not (3).
Binder death notification is the real prize: it is the OS's own liveness signal and distinguishes "the process died" from "a socket closed", which lets the disconnect-inference block in ComapeoCoreModule.kt:106-137 be replaced by an explicit callback.
Two things make this cheaper than §8.1's ~100 LOC estimate: ComapeoCoreService is already a Service with an onBind stub returning null (line 482), and the state is already a MutableStateFlow (NodeJSService.kt:199).
What this removes
backend/lib/simple-rpc.jsentirely, and with itServerHelper's control-socket use, the readiness-phase replay, the terminal-frame cache, and the method table.control.sockfrom the backend: the bind, the path argv positional, the delete-before-bind cleanup, and the ordering constraint that it bind beforecomapeo.sock.ios/ControlFrame.swift,android/.../ControlFrame.kt, and the secondNodeJSIPCinstance on both platforms.MockBackend.swiftandMockNodeServer.swift, which exist to speak the control protocol over a socket during macOS swift-test runs.- The disconnect-inference rules in
ComapeoCoreModule.kt, replaced by an explicit death callback.
NodeJSIPC itself stays — comapeo.sock still needs it on both platforms. This does not eliminate the socket dependency, only this instance of it. That is the honest limit of the win.
Not a performance change
Stated explicitly so nobody reaches for it as justification: this channel carries roughly six messages per process lifetime. There is no throughput or allocation win here, and the copy-minimisation discipline in #244 does not apply. The case rests entirely on deleting simple-rpc.js, the second NodeJSIPC, and the disconnect-inference rules — and on linkToDeath being a better liveness signal than socket EOF.
Cost
The test harness is the bill. Roughly 1,100 lines assume a socket seam: MockBackend.swift (175), MockNodeServer.swift (146), IPCLifecycleTests.swift (271), ControlFrameTests.swift (164), ControlFrameTest.kt (165), simple-rpc.test.mjs (253). Production code deleted is smaller than test code rewritten. The replacement seam is a fake bridge injected into NodeJSService, which MockNodeService.swift shows is already an established pattern.
Hard-crash detection changes on iOS. Today a SIGSEGV or OOM in the runtime is observed as control.sock closing. Without the socket, the signal is nodeEntryPoint(args) returning — which runNode() already classifies (NodeJSService.swift:646-660), so the path exists, but it becomes the only one and needs to be verified rather than assumed.
Plan
1. Addon surface
Node → native, fire-and-forget:
notifyState(phase) // "started" | "ready" | "stopping"
notifyError(phase, message) // terminal
Native → Node needs a threadsafe function, since shutdown and error-native originate on arbitrary native threads:
requestShutdown()
reportNativeError(phase, message)
Both must work mid-boot, before the backend has finished initialising — the equivalent of today's sendMessageSync queueing into the IPC's pending buffer when still connecting (NodeJSService.swift:574). Register the TSFN as early as the addon is loaded and buffer until the backend installs its handlers.
2. Backend
Replace controlIpcServer.setReadinessPhase("started" | "ready") and the two broadcast({type:"stopping"|"error"}) sites with the corresponding notifyState / notifyError calls, at exactly the same points. Register the requestShutdown and reportNativeError handlers where the SimpleRpcServer method table sits today, preserving the existing semantics — handleFatal still broadcasts, flushes and exits; shutdown still closes servers in the same order.
Delete simple-rpc.js and its test. Drop the control-socket argv positional and renumber the remaining ones, or better, switch the backend to named flags while touching this — the positional list is up to six entries and a renumber is exactly the kind of change that silently misassigns.
3. iOS
handleControlMessage becomes a set of callbacks registered on the bridge. backendState transitions stay identical. sendShutdownFrame / sendErrorNativeFrame become bridge calls. Delete controlIPC, ControlFrame.swift, and the control-socket path plumbing.
The started callback keeps its two side effects: closing the node-spawn boot span and setting backendState = .controlBound. Consider renaming that state now that nothing is bound.
4. Android FGS
Same as iOS for the Node-facing side. Additionally, fill in ComapeoCoreService.onBind to return a Messenger backed by a Handler, with a small protocol:
MSG_REGISTER— client sends its replyMessenger; service responds immediately with the currentstateFlow.valuesnapshot. This is the free late-join.MSG_STATE— service → client on everystateFlowchange, carrying state pluserrorPhase/errorMessage.- Service holds client
Messengers weakly and drops them onlinkToDeath.
Collect stateFlow in serviceScope and fan out. No new state derivation — deriveLifecycleState is unchanged and stays the single source.
5. Android main process
ComapeoCoreModule binds to the service in OnCreate (bindService with BIND_AUTO_CREATE — check the interaction with the dataSync FGS type, see below) and unbinds in OnDestroy. It consumes the published state directly instead of re-deriving it from frames, which deletes both the frame when block and the disconnect-inference rules.
onServiceDisconnected / binderDied becomes the explicit "FGS process died" signal, mapping to ERROR with node-runtime-unexpected only when the pre-death state was STARTING/STARTED — the same rule as today, but triggered by a real death notification rather than inferred from a socket close.
6. Docs
Rewrite ARCHITECTURE §3 (one socket, not two), §4 (no init handshake), §5.4, and §8.1 — the last should keep the original table and add why the scoring changed, rather than being replaced. §3.2's three justifications for two sockets are all gone by this point and the section should say so.
Testing
- Replace
MockBackend/MockNodeServerwith a fake bridge conforming to the same protocolNodeJSServiceconsumes.DeriveStateTests.swiftshould not need to change at all — if it does, the derivation was coupled to the transport and that is worth knowing. - New Android instrumented test for the bind protocol: bind → receive snapshot, state change → receive push, kill the FGS →
binderDiedfires and the main process lands inERROR. The last one pairs with the existingmaestro/fgs-restart.yamlflow. - Keep
IPCLifecycleTests.swift's scenarios and re-point them at the bridge; they encode real orderings that are worth preserving even though the transport changed. - The e2e suite should be unchanged. If
apps/e2eneeds edits, the JS-facing contract moved, which it must not.
Risks
bindService from the main process to a dataSync foreground service interacts with the FGS lifecycle — a bound service is kept alive by the binding, which could mask a stop the user requested via the notification action. Verify against ForegroundService.md's user-stop handling and the fgs-restart Maestro flow before committing to BIND_AUTO_CREATE; BIND_NOT_FOREGROUND or an unbound Messenger handshake may be the safer shape.
TSFN lifetime versus NodeMobileStartNode's once-per-process constraint on iOS: a botched acquire/release is unrecoverable without an app restart. Release on backend shutdown only, and never on JS reload — the module reloads, the Node thread does not.
Ordering between notifyState and the native stateFlow fan-out. Node is authoritative; make the FGS apply state synchronously on receipt so the main process cannot observe ready before the FGS's own backendState reflects it.
- Lingua principale
- Kotlin
- Stelle
- 1
- Fork
- 0
- Merge medio
- 1g 6h
- PR unite (30g)
- 5
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di digidem/comapeo-core-react-native
-
area: ios priority: P1
Difficoltà 2/5 Mezza giornata Idoneità per principianti 70/100
digidem/comapeo-core-react-native#21 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 52/100
digidem/comapeo-core-react-native#254 ·
I maintainer di solito rispondono entro 1 giorno
-
chore: recover from backend restarts via backend-owned project lifecycle (tracking)Forse di nuovo libera @RangerMauve l’ha presa 67 giorni fa e non c’è nessuna pull request aperta. Aperta
digidem/comapeo-core-react-native#247 · 2 commenti · 2 assegnatari ·
I maintainer di solito rispondono entro 1 giorno
-
refactor(sentry): deliver Node telemetry to the native SDK through the addon, not the control socketAperta
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
digidem/comapeo-core-react-native#244 ·
I maintainer di solito rispondono entro 1 giorno
-
refactor: read the rootkey and master key through a native addon instead of the control socketAperta
Difficoltà 5/5 Più di una settimana Idoneità per principianti 45/100
digidem/comapeo-core-react-native#243 ·
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di digidem/comapeo-core-react-native
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 74/100
I maintainer di solito rispondono entro 2 giorni
-
unconfirmed
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
thunderbird/thunderbird-android#11893 · 1 commento ·
I maintainer di solito rispondono entro 2 giorni
-
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 65/100
Droid-Deck/DroidDeck#2198 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 77/100
world-federation-of-advertisers/cross-media-measurement#4846 ·
I maintainer di solito rispondono entro 1 giorno
-
bug
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
I maintainer di solito rispondono entro 1 giorno