From e5574431234fe7e86f5651116bde9bb8cf15a831 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Fri, 29 May 2026 01:20:14 +0200 Subject: [PATCH 1/6] fix(transport): proactively detect peer-closed socket before sending ECR17/Nexi terminals close the TCP socket between transactions. The drop was detected only reactively: ensureConnected() saw isConnected()==true for a half-open socket (the Kotlin reader thread had not observed EOF yet), so a command was sent on a dead socket; the read then failed mid-exchange and the money-safety RetryPolicy (correctly) refused to replay financial commands, surfacing a FALSE "transport disconnected during exchange". Safe/idempotent commands were replayed and succeeded, producing the observed "works once, fails next" alternation. Fix: make Kotlin isConnected() a synchronous non-destructive liveness probe (socket.sendUrgentData) that detects a peer-closed/half-open socket BEFORE the send, so the existing ensureConnected() reconnects proactively and every command starts on a verified-live socket. The probe writes one TCP OOB byte that throws on a closed peer but does not touch the data stream (cannot corrupt an STX/ETX frame, does not race the reader thread). Money-safety is unchanged: RetryPolicy (financial never blindly retried) and sendLastResult ('G') recovery are untouched; only the FALSE drop from a stale pre-send socket is removed. A genuine mid-exchange drop still surfaces. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/LESSON.md | 27 ++++++++++++ .../nitro/ecr17/HybridEcr17Transport.kt | 44 +++++++++++++++++-- package/cpp/Ecr17Client/HybridEcr17Client.cpp | 8 +++- package/ios/HybridEcr17Transport.swift | 7 +++ 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/docs/LESSON.md b/docs/LESSON.md index 5df087e..80572b5 100644 --- a/docs/LESSON.md +++ b/docs/LESSON.md @@ -80,6 +80,33 @@ - **Emit `DISCONNECTED` on a failed connect**, else listeners stay stuck on `CONNECTING`. `connect()` delegates to `ensureConnected()` which emits CONNECTING→(CONNECTED | DISCONNECTED on throw). +- **ECR17/Nexi terminals close the TCP socket BETWEEN transactions → detect the + drop PROACTIVELY (before sending), not reactively (after).** Observed on a real + device: financial commands (verifyCard/pay) INTERMITTENTLY failed with + "transport disconnected during exchange" while safe commands (status/totals) + succeeded — apparent "works once, fails next". Root cause: the terminal closes + the socket after a transaction (and the Kotlin reader thread may not have + observed the EOF `read()<0` yet — a race), so `isConnected()` returned `true` + for a half-open socket. The command was then SENT on a dead socket; the read + failed MID-exchange; `runTransaction`'s catch reconnected and applied the + money-safety RetryPolicy → safe/idempotent ops were replayed (→ ok) but + financial ops were (correctly) NOT replayed → a FALSE error surfaced. The + reactive reconnect left a fresh socket, so the user's NEXT manual attempt landed + on a good socket → the alternation. The money-safety behavior was correct; the + bug was discovering the drop AFTER the send instead of BEFORE. Fix: make Kotlin + `isConnected()` a synchronous NON-DESTRUCTIVE liveness probe — + `socket.sendUrgentData(0xFF)` writes one TCP out-of-band byte that THROWS on a + peer-closed/half-open socket but is harmless on a live one and does NOT touch the + normal data stream (cannot corrupt an STX/ETX frame, does not race the reader + thread which only reads the ordinary input stream). On a thrown probe we mark the + socket dead + fire onDisconnect, so the existing `ensureConnected()` (called at + the start of every command) reconnects BEFORE the send and the financial command + starts on a verified-live socket. Money-safety is UNCHANGED — RetryPolicy and + sendLastResult ('G') recovery are untouched; we only removed the FALSE drop from + a stale pre-send socket; a genuine mid-exchange drop still surfaces and is + recovered via 'G'. iOS (`NWConnection.state == .ready`) reflects peer-close too + but state updates are async (small residual race; no iOS CI → best-effort). Only + reproducible by RUNNING against a real terminal — no build/unit CI catches it. ## Build wiring - **Nitro C++ HybridObject impl header MUST be named after `implementationClassName` diff --git a/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt b/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt index bc7a53c..4c9c7dd 100644 --- a/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt +++ b/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt @@ -93,10 +93,48 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { override fun isConnected(): Boolean { // `Socket.isConnected` stays true after the peer closes the connection, so it // alone can't detect an unexpected drop. The reader thread clears `running` - // when the stream ends (or on an intentional close), making it the source of - // truth for an active, usable connection. + // when the stream ends (or on an intentional close); but there is a RACE: an + // ECR17/Nexi terminal typically closes the TCP socket between transactions, and + // the reader thread may not have observed the EOF (`read() < 0`) yet when the + // NEXT command checks `isConnected()`. If we returned `true` there, the command + // would be SENT on a half-open (peer-closed) socket; the read would then fail + // MID-exchange — and the money-safety RetryPolicy (correctly) refuses to replay + // a financial command, so a FALSE "transport disconnected" error surfaces. + // + // Fix: detect a peer-closed/half-open socket PROACTIVELY here, BEFORE the send, + // with a non-destructive liveness probe. `sendUrgentData` writes a single TCP + // out-of-band byte; on a socket the peer has already closed (FIN/RST received) + // it throws IOException, while on a live socket it is harmless and does not + // touch the normal data stream (so it cannot corrupt an STX/ETX frame and does + // not race the reader thread, which only reads the ordinary input stream). When + // the probe trips, we treat the connection as dead so `ensureConnected()` + // reconnects BEFORE sending — turning the old reactive (post-send) drop + // detection into proactive (pre-send) recovery. Money-safety is unchanged: we + // only remove the FALSE drop caused by a stale pre-send socket; a genuine + // mid-exchange drop still surfaces and is recovered via sendLastResult ('G'). val s = socket - return running && s != null && s.isConnected && !s.isClosed + if (!running || s == null || !s.isConnected || s.isClosed) { + return false + } + return try { + s.sendUrgentData(0xFF) + true + } catch (_: Throwable) { + // Peer has closed the socket (or it is otherwise unusable): mark it dead and + // notify listeners, mirroring the reader thread's EOF path, so the next + // ensureConnected() establishes a fresh socket before any command is sent. + markDropped() + false + } + } + + /** Marks the connection as dropped and fires onDisconnect (unexpected drop). */ + private fun markDropped() { + if (!running) return + running = false + if (!intentionalDisconnect) { + onDisconnectCallback?.invoke() + } } override fun send(bytes: ArrayBuffer) { diff --git a/package/cpp/Ecr17Client/HybridEcr17Client.cpp b/package/cpp/Ecr17Client/HybridEcr17Client.cpp index 86a2cdf..d83d3a8 100644 --- a/package/cpp/Ecr17Client/HybridEcr17Client.cpp +++ b/package/cpp/Ecr17Client/HybridEcr17Client.cpp @@ -318,8 +318,14 @@ void HybridEcr17Client::ensureConnected() { // Run the transport JNI work under the app class loader (Android); inline on iOS. runOnJvmThread([&]() { ensureInit(); + // PROACTIVE reconnect: isConnected() performs a synchronous liveness probe + // (Android: sendUrgentData on the socket) so a peer-closed/half-open socket + // — common because ECR17/Nexi terminals close TCP between transactions — is + // detected HERE, before any command is sent. This is what stops a financial + // command from being sent on a stale socket and then hitting the (correct) + // money-safety "never replay" path with a FALSE "transport disconnected". if (transport_->isConnected()) { - return; // returns from this lambda only — no further JNI needed + return; // verified live — returns from this lambda only, no further JNI } // Auto-connect: block this worker thread until the native transport connects // (or throw on failure). keepAlive leaves the socket open for reuse. diff --git a/package/ios/HybridEcr17Transport.swift b/package/ios/HybridEcr17Transport.swift index c5bf80e..35257ca 100644 --- a/package/ios/HybridEcr17Transport.swift +++ b/package/ios/HybridEcr17Transport.swift @@ -67,6 +67,13 @@ final class HybridEcr17Transport: HybridEcr17TransportSpec { } func isConnected() throws -> Bool { + // Used by the C++ client as a PRE-SEND liveness check so a command starts on + // a verified-live socket (the Android transport probes the socket here). With + // Network.framework the connection transitions to .failed/.cancelled when the + // peer closes, so `.ready` is the closest equivalent. NOTE: state updates are + // delivered asynchronously on `queue`, so a peer close observed between + // transactions may take a moment to flip the state — a small residual race + // not present on Android. Best-effort (no iOS CI); verify on a real build. return connection?.state == .ready } From d6eb6ed1daa771943324f4d5731c34bcdd4b0e58 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Fri, 29 May 2026 01:32:12 +0200 Subject: [PATCH 2/6] fix(transport): use write-free read-probe for liveness (no OOB byte) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address reviewer feedback (Codex P2 + Copilot) on the liveness probe: - Codex P2: socket.sendUrgentData writes a real TCP urgent byte; on a terminal with SO_OOBINLINE that 0xFF lands inline before the next STX frame and could corrupt a financial command. Replace it with a non-destructive, WRITE-FREE probe: a 1-byte peek on a PushbackInputStream (short soTimeout) — read()==-1 => peer closed; SocketTimeoutException => idle but alive; any byte read is unread() so a protocol byte is never consumed or placed on the peer stream. The reader thread and the probe share the input stream under one ioLock (reader uses a short read timeout so it yields the lock between reads), so they never read concurrently. - Copilot: a drop could fire onDisconnect twice (reader finally + probe). Add a single-shot AtomicBoolean so a drop notifies exactly once across both paths; both paths also close the socket so isConnected()'s isClosed check is an immediate, reliable signal. - Copilot: narrow exception handling — SocketTimeoutException means "alive", only genuine I/O failures mark the socket dropped. Money-safety unchanged: RetryPolicy and sendLastResult ('G') untouched; only the FALSE drop from a stale pre-send socket is removed. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/LESSON.md | 34 ++-- .../nitro/ecr17/HybridEcr17Transport.kt | 174 ++++++++++++------ package/cpp/Ecr17Client/HybridEcr17Client.cpp | 14 +- 3 files changed, 146 insertions(+), 76 deletions(-) diff --git a/docs/LESSON.md b/docs/LESSON.md index 80572b5..26eccf7 100644 --- a/docs/LESSON.md +++ b/docs/LESSON.md @@ -94,19 +94,27 @@ reactive reconnect left a fresh socket, so the user's NEXT manual attempt landed on a good socket → the alternation. The money-safety behavior was correct; the bug was discovering the drop AFTER the send instead of BEFORE. Fix: make Kotlin - `isConnected()` a synchronous NON-DESTRUCTIVE liveness probe — - `socket.sendUrgentData(0xFF)` writes one TCP out-of-band byte that THROWS on a - peer-closed/half-open socket but is harmless on a live one and does NOT touch the - normal data stream (cannot corrupt an STX/ETX frame, does not race the reader - thread which only reads the ordinary input stream). On a thrown probe we mark the - socket dead + fire onDisconnect, so the existing `ensureConnected()` (called at - the start of every command) reconnects BEFORE the send and the financial command - starts on a verified-live socket. Money-safety is UNCHANGED — RetryPolicy and - sendLastResult ('G') recovery are untouched; we only removed the FALSE drop from - a stale pre-send socket; a genuine mid-exchange drop still surfaces and is - recovered via 'G'. iOS (`NWConnection.state == .ready`) reflects peer-close too - but state updates are async (small residual race; no iOS CI → best-effort). Only - reproducible by RUNNING against a real terminal — no build/unit CI catches it. + `isConnected()` a synchronous, NON-DESTRUCTIVE, WRITE-FREE liveness probe — a + 1-byte peek on a `PushbackInputStream` (short `soTimeout`): `read()==-1` ⇒ peer + closed (dead); `SocketTimeoutException` ⇒ idle but alive; any read byte is + `unread()` so a protocol byte is NEVER consumed. ⚠️ Do NOT use + `socket.sendUrgentData(0xFF)` for this (the first version did): it WRITES a TCP + out-of-band byte, and on a terminal with `SO_OOBINLINE` that 0xFF lands INLINE + right before the next `STX` frame — corrupting a financial command. The probe + must never put bytes on the peer's protocol stream. The reader thread and the + probe share the input stream under one `ioLock` (reader uses a short read timeout + so it releases the lock between reads); a single-shot `AtomicBoolean` makes a drop + fire onDisconnect exactly once across both paths, and the reader/probe close the + socket on a drop so `isConnected()`'s `isClosed` check is an immediate signal. + When the probe trips it marks the socket dead, so the existing `ensureConnected()` + (called at the start of every command) reconnects BEFORE the send and the + financial command starts on a verified-live socket. Money-safety is UNCHANGED — + RetryPolicy and sendLastResult ('G') recovery are untouched; we only removed the + FALSE drop from a stale pre-send socket; a genuine mid-exchange drop still + surfaces and is recovered via 'G'. iOS (`NWConnection.state == .ready`) reflects + peer-close too but state updates are async (small residual race; no iOS CI → + best-effort). Only reproducible by RUNNING against a real terminal — no build/unit + CI catches it. ## Build wiring - **Nitro C++ HybridObject impl header MUST be named after `implementationClassName` diff --git a/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt b/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt index 4c9c7dd..b89688a 100644 --- a/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt +++ b/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt @@ -2,8 +2,11 @@ package com.margelo.nitro.ecr17 import com.margelo.nitro.core.ArrayBuffer import com.margelo.nitro.core.Promise +import java.io.PushbackInputStream import java.net.InetSocketAddress import java.net.Socket +import java.net.SocketTimeoutException +import java.util.concurrent.atomic.AtomicBoolean import kotlin.concurrent.thread /** @@ -16,12 +19,25 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { private var socket: Socket? = null private var readerThread: Thread? = null + // The reader and the liveness probe ([isConnected]) share one input stream; + // PushbackInputStream lets the probe peek a byte (looking only for EOF) and push + // it back so it is never consumed from the protocol stream. + private var input: PushbackInputStream? = null + @Volatile private var running = false // True while a caller-initiated disconnect is in progress, so the reader's // finally block does not emit a spurious onDisconnect for an intentional close. @Volatile private var intentionalDisconnect = false + // Single-shot guard so a given drop fires onDisconnect exactly once, no matter + // whether the reader thread or the liveness probe observes it first. + private val disconnectEmitted = AtomicBoolean(false) + + // Serializes access to the shared input stream between the reader thread and the + // liveness probe so the two never `read()` concurrently. + private val ioLock = Any() + private var onDataCallback: ((ArrayBuffer) -> Unit)? = null private var onDisconnectCallback: (() -> Unit)? = null @@ -30,40 +46,100 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { // Tear down any previous connection (and join its reader) before reconnecting. closeCurrent() intentionalDisconnect = false + disconnectEmitted.set(false) val s = Socket() s.tcpNoDelay = true s.connect(InetSocketAddress(host, port.toInt()), timeoutMs.toInt()) + // A short read timeout lets the reader loop release `ioLock` periodically so + // the liveness probe ([isConnected]) can run between reads; a timeout is not + // an error, just "no data yet". + s.soTimeout = READ_TIMEOUT_MS socket = s + input = PushbackInputStream(s.getInputStream(), 1) running = true - startReader(s) + startReader() Unit } } - private fun startReader(s: Socket) { + private fun startReader() { readerThread = thread(name = "ecr17-reader", isDaemon = true) { val buffer = ByteArray(4096) try { - val input = s.getInputStream() while (running) { - val read = input.read(buffer) - if (read < 0) break - if (read > 0) { - onDataCallback?.invoke(ArrayBuffer.copy(buffer.copyOfRange(0, read))) + val read = + synchronized(ioLock) { + if (!running) return@synchronized SENTINEL_STOP + try { + input?.read(buffer) ?: -1 // null stream -> treat as EOF + } catch (_: SocketTimeoutException) { + SENTINEL_TIMEOUT // no data within READ_TIMEOUT_MS — keep looping + } + } + when { + read == SENTINEL_STOP -> break + read == SENTINEL_TIMEOUT -> continue + read < 0 -> break // EOF: peer closed the connection + read > 0 -> + onDataCallback?.invoke(ArrayBuffer.copy(buffer.copyOfRange(0, read))) } } } catch (_: Throwable) { // socket closed or read error } finally { - running = false - // Only signal disconnect for unexpected drops, not caller-initiated closes. - if (!intentionalDisconnect) { - onDisconnectCallback?.invoke() - } + // Promptly close the socket so `isConnected()`'s `isClosed` check is an + // immediate, reliable signal of the drop (no reliance on a write probe). + markDropped() + } + } + } + + /** + * Non-destructive liveness probe used to detect a peer-closed / half-open socket + * BEFORE a command is sent. ECR17/Nexi terminals routinely close the TCP socket + * between transactions; `Socket.isConnected` stays `true` for such a half-open + * socket, and the reader thread may not have observed the EOF yet — so without a + * probe a command could be sent on a dead socket, fail MID-exchange, and (because + * the money-safety RetryPolicy correctly refuses to replay a financial command) + * surface a FALSE "transport disconnected" error. + * + * The probe peeks ONE byte with a tiny read timeout and immediately pushes it + * back, so it NEVER consumes a protocol byte and NEVER writes anything to the + * peer (unlike `sendUrgentData`, which can place an inline 0xFF before the next + * STX frame on terminals with SO_OOBINLINE and corrupt a financial command). A + * returned `-1` means the peer closed; a read timeout means the (idle) socket is + * alive. It runs under `ioLock` so it never races the reader's `read()`. + * + * Money-safety is unchanged: this only removes the FALSE drop from a stale + * pre-send socket; a genuine mid-exchange drop still surfaces and is recovered + * via sendLastResult ('G'). + */ + override fun isConnected(): Boolean { + val s = socket + val pin = input + if (!running || s == null || pin == null || !s.isConnected || s.isClosed) { + return false + } + return synchronized(ioLock) { + if (!running || s.isClosed) return@synchronized false + try { + val b = pin.read() // EOF (-1) if peer closed; SocketTimeoutException if idle+alive + if (b < 0) { + markDropped() + false + } else { + pin.unread(b) // push the (unexpected) byte back — never consumed/dropped + true } + } catch (_: SocketTimeoutException) { + true // idle but alive (no FIN received within the timeout) + } catch (_: Throwable) { + markDropped() // I/O error: treat as dropped + false } + } } /** Closes the current socket and joins its reader thread (intentional close). */ @@ -76,6 +152,7 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { // ignore } socket = null + input = null readerThread?.let { t -> try { t.join(1000) @@ -86,57 +163,29 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { readerThread = null } - override fun disconnect() { - closeCurrent() - } - - override fun isConnected(): Boolean { - // `Socket.isConnected` stays true after the peer closes the connection, so it - // alone can't detect an unexpected drop. The reader thread clears `running` - // when the stream ends (or on an intentional close); but there is a RACE: an - // ECR17/Nexi terminal typically closes the TCP socket between transactions, and - // the reader thread may not have observed the EOF (`read() < 0`) yet when the - // NEXT command checks `isConnected()`. If we returned `true` there, the command - // would be SENT on a half-open (peer-closed) socket; the read would then fail - // MID-exchange — and the money-safety RetryPolicy (correctly) refuses to replay - // a financial command, so a FALSE "transport disconnected" error surfaces. - // - // Fix: detect a peer-closed/half-open socket PROACTIVELY here, BEFORE the send, - // with a non-destructive liveness probe. `sendUrgentData` writes a single TCP - // out-of-band byte; on a socket the peer has already closed (FIN/RST received) - // it throws IOException, while on a live socket it is harmless and does not - // touch the normal data stream (so it cannot corrupt an STX/ETX frame and does - // not race the reader thread, which only reads the ordinary input stream). When - // the probe trips, we treat the connection as dead so `ensureConnected()` - // reconnects BEFORE sending — turning the old reactive (post-send) drop - // detection into proactive (pre-send) recovery. Money-safety is unchanged: we - // only remove the FALSE drop caused by a stale pre-send socket; a genuine - // mid-exchange drop still surfaces and is recovered via sendLastResult ('G'). - val s = socket - if (!running || s == null || !s.isConnected || s.isClosed) { - return false - } - return try { - s.sendUrgentData(0xFF) - true - } catch (_: Throwable) { - // Peer has closed the socket (or it is otherwise unusable): mark it dead and - // notify listeners, mirroring the reader thread's EOF path, so the next - // ensureConnected() establishes a fresh socket before any command is sent. - markDropped() - false - } - } - - /** Marks the connection as dropped and fires onDisconnect (unexpected drop). */ + /** + * Marks the connection as dropped, closes the socket so the drop is synchronously + * observable, and fires onDisconnect exactly once. Safe to call from both the + * reader thread and the liveness probe (single-shot via [disconnectEmitted]). + */ private fun markDropped() { - if (!running) return running = false - if (!intentionalDisconnect) { + try { + socket?.close() + } catch (_: Throwable) { + // ignore + } + // Only signal disconnect for unexpected drops, not caller-initiated closes, and + // only once per drop. + if (!intentionalDisconnect && disconnectEmitted.compareAndSet(false, true)) { onDisconnectCallback?.invoke() } } + override fun disconnect() { + closeCurrent() + } + override fun send(bytes: ArrayBuffer) { val s = socket ?: throw IllegalStateException("ECR17 transport is not connected") val out = s.getOutputStream() @@ -151,4 +200,15 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { override fun setOnDisconnect(callback: () -> Unit) { onDisconnectCallback = callback } + + private companion object { + // Reader-loop read timeout: short enough that the liveness probe never waits + // long for `ioLock`, long enough to avoid busy-spinning. + private const val READ_TIMEOUT_MS = 100 + + // Sentinels returned by the synchronized read block; kept below the real EOF + // value (-1) so `read < 0` still catches a genuine EOF after these are handled. + private const val SENTINEL_TIMEOUT = -2 + private const val SENTINEL_STOP = -3 + } } diff --git a/package/cpp/Ecr17Client/HybridEcr17Client.cpp b/package/cpp/Ecr17Client/HybridEcr17Client.cpp index d83d3a8..4a75fa5 100644 --- a/package/cpp/Ecr17Client/HybridEcr17Client.cpp +++ b/package/cpp/Ecr17Client/HybridEcr17Client.cpp @@ -318,12 +318,14 @@ void HybridEcr17Client::ensureConnected() { // Run the transport JNI work under the app class loader (Android); inline on iOS. runOnJvmThread([&]() { ensureInit(); - // PROACTIVE reconnect: isConnected() performs a synchronous liveness probe - // (Android: sendUrgentData on the socket) so a peer-closed/half-open socket - // — common because ECR17/Nexi terminals close TCP between transactions — is - // detected HERE, before any command is sent. This is what stops a financial - // command from being sent on a stale socket and then hitting the (correct) - // money-safety "never replay" path with a FALSE "transport disconnected". + // PROACTIVE reconnect: isConnected() performs a synchronous, non-destructive + // liveness probe (Android: a 1-byte peek-with-pushback that detects a peer + // FIN without writing to or consuming from the stream) so a peer-closed/ + // half-open socket — common because ECR17/Nexi terminals close TCP between + // transactions — is detected HERE, before any command is sent. This is what + // stops a financial command from being sent on a stale socket and then + // hitting the (correct) money-safety "never replay" path with a FALSE + // "transport disconnected". if (transport_->isConnected()) { return; // verified live — returns from this lambda only, no further JNI } From a6d5cee4bd313794a7000975ea8dfeddf22ca0e8 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Fri, 29 May 2026 01:42:35 +0200 Subject: [PATCH 3/6] fix(transport): narrow probe/reader catches to IOException Per Copilot review: catch only IOException (covers SocketException/ SocketTimeoutException) in the reader loop and the liveness probe, so Errors (e.g. OOM) and unexpected RuntimeExceptions propagate and stay visible in development instead of being silently treated as a socket drop. The reader's finally still fires the single-shot drop notification on any unwind. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/margelo/nitro/ecr17/HybridEcr17Transport.kt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt b/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt index b89688a..5824104 100644 --- a/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt +++ b/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt @@ -2,6 +2,7 @@ package com.margelo.nitro.ecr17 import com.margelo.nitro.core.ArrayBuffer import com.margelo.nitro.core.Promise +import java.io.IOException import java.io.PushbackInputStream import java.net.InetSocketAddress import java.net.Socket @@ -86,8 +87,10 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { onDataCallback?.invoke(ArrayBuffer.copy(buffer.copyOfRange(0, read))) } } - } catch (_: Throwable) { - // socket closed or read error + } catch (_: IOException) { + // Expected on socket close / read error. Other throwables (Error, unexpected + // RuntimeException) intentionally propagate so they're visible in dev; the + // finally still fires the drop notification before the thread unwinds. } finally { // Promptly close the socket so `isConnected()`'s `isClosed` check is an // immediate, reliable signal of the drop (no reliance on a write probe). @@ -135,8 +138,8 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { } } catch (_: SocketTimeoutException) { true // idle but alive (no FIN received within the timeout) - } catch (_: Throwable) { - markDropped() // I/O error: treat as dropped + } catch (_: IOException) { + markDropped() // genuine I/O error: treat as dropped (other throwables propagate) false } } From 27ec54bf79a6b46732b4dbf886116ecbe6d98f8c Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Fri, 29 May 2026 02:02:28 +0200 Subject: [PATCH 4/6] =?UTF-8?q?fix(lrc):=20fold=20ETX=20for=20NOEXT=20(not?= =?UTF-8?q?=20STD)=20=E2=80=94=20restores=20correct=20LRC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A prior commit (44f178e) wrongly changed the ETX-fold switch in Lcr.cpp from `case LrcMode::NOEXT` to `case LrcMode::STD`, inverting the LrcMode semantics: - std = payload only (must NOT fold ETX) - noext = payload + ETX (must fold ETX) - stx = folds STX and ETX With the bug, default `std` mode folded ETX, so every frame's LRC was wrong (the terminal NAKs everything) and `test_lrc` failed on main. Revert that one line to `case LrcMode::NOEXT`. Verified locally against the test vectors (STD=0x3E, STX=0x3F, NOEXT=0x3D, STX_NOEXT=0x3C for payload 'A'). Co-Authored-By: Claude Opus 4.7 (1M context) --- package/cpp/Lcr/Lcr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/cpp/Lcr/Lcr.cpp b/package/cpp/Lcr/Lcr.cpp index c2db13d..42466f4 100644 --- a/package/cpp/Lcr/Lcr.cpp +++ b/package/cpp/Lcr/Lcr.cpp @@ -24,7 +24,7 @@ uint8_t Lrc::compute(const std::vector& payload, LrcMode mode) { switch (mode) { case LrcMode::STX: - case LrcMode::STD: + case LrcMode::NOEXT: lrc ^= ETX; break; From fc187008385361096327e3a79a42146a81c7b975 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Fri, 29 May 2026 02:06:29 +0200 Subject: [PATCH 5/6] docs(progress): record proactive-reconnect + LRC regression fix (PR #11) Co-Authored-By: Claude Opus 4.7 (1M context) --- PROGRESS.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/PROGRESS.md b/PROGRESS.md index c8a2dbb..78bc0aa 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -37,6 +37,14 @@ Target PR: to be opened against `main` once Phase 0 lands (or reuse a draft). - [x] Auto-reconnect on mid-session drop — SAFE policy (financial never replayed; RetryPolicy.hpp unit-tested; recover via sendLastResult 'G'). cpp 82/82. - [x] Money-safety reviewed by Copilot (no double-charge path) + README enterprise section. +- [x] PROACTIVE reconnect (PR #11, fix/proactive-reconnect): ECR17/Nexi terminals + close TCP between transactions → detect the peer-closed/half-open socket + BEFORE sending (Kotlin isConnected() = write-free 1-byte peek-with-pushback on + a PushbackInputStream; NOT sendUrgentData — OOB would corrupt a financial frame + under SO_OOBINLINE). Removes the FALSE "transport disconnected during exchange" + on financial commands; money-safety (RetryPolicy / 'G') untouched. Also reverted + the LRC ETX-fold regression in Lcr.cpp (NOEXT, not STD) introduced by 44f178e. + LESSON.md updated. ### Genuinely remaining (need hardware/macOS — documented in README roadmap) - iOS Swift transport verification: NO macOS CI runner. Swift written best-effort. From 1b7b5fc7fe887470ed435cf442292c1781ca8720 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Fri, 29 May 2026 02:24:00 +0200 Subject: [PATCH 6/6] perf(transport): use tiny probe timeout in isConnected so idle sockets don't block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The liveness probe read under READ_TIMEOUT_MS (100ms), so a healthy but idle socket — the normal between-transactions state — made every pre-send check wait the full reader timeout. Set a 1ms soTimeout for the probe (safe under ioLock, restored before releasing), so an alive idle socket returns near-instantly while a peer-closed socket still reports EOF immediately. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../nitro/ecr17/HybridEcr17Transport.kt | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt b/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt index 5824104..e8bb720 100644 --- a/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt +++ b/package/android/src/main/java/com/margelo/nitro/ecr17/HybridEcr17Transport.kt @@ -127,7 +127,13 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { } return synchronized(ioLock) { if (!running || s.isClosed) return@synchronized false + // Use a tiny probe timeout instead of the reader's READ_TIMEOUT_MS so a healthy + // IDLE socket (the normal between-transactions case) returns "alive" almost + // immediately rather than blocking every pre-send check for the full read + // timeout. Safe to retune soTimeout here: we hold ioLock, so the reader thread + // is not mid-read; we restore READ_TIMEOUT_MS before releasing the lock. try { + s.soTimeout = PROBE_TIMEOUT_MS val b = pin.read() // EOF (-1) if peer closed; SocketTimeoutException if idle+alive if (b < 0) { markDropped() @@ -137,10 +143,16 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { true } } catch (_: SocketTimeoutException) { - true // idle but alive (no FIN received within the timeout) + true // idle but alive (no FIN received within the probe timeout) } catch (_: IOException) { markDropped() // genuine I/O error: treat as dropped (other throwables propagate) false + } finally { + try { + s.soTimeout = READ_TIMEOUT_MS // restore the reader loop's timeout + } catch (_: IOException) { + // socket already closed by markDropped(); nothing to restore + } } } } @@ -209,6 +221,10 @@ class HybridEcr17Transport : HybridEcr17TransportSpec() { // long for `ioLock`, long enough to avoid busy-spinning. private const val READ_TIMEOUT_MS = 100 + // Probe read timeout used by isConnected(): tiny so a healthy idle socket reports + // "alive" near-instantly instead of paying READ_TIMEOUT_MS on every pre-send check. + private const val PROBE_TIMEOUT_MS = 1 + // Sentinels returned by the synchronized read block; kept below the real EOF // value (-1) so `read < 0` still catches a genuine EOF after these are handled. private const val SENTINEL_TIMEOUT = -2