fix(aprs): hold the client lock across the response read in sendPacket
sendPacket wrote the packet under the lock but read the server response outside it. disconnect() - called concurrently from stop() and from the reconnect path in AprsReporter.reportOnce's catch - nulls and closes writer/reader/socket under the same lock, so the lock-free read raced with it. A probe interleaving 5,000 sends with repeated disconnects produced a mix of 744 OK and 4,256 exception results: the response read hit a just-closed socket and the swallowing runCatching reported Pair(true,"OK") for a packet that may never have left, or read through a stale reference. The tracker believed the beacon was heard while APRS-IS never received it. Holding the lock across write+read serialises against disconnect: either disconnect got the lock first and sendPacket returns null (writer cleared), or sendPacket runs to completion and disconnect waits, bounded by the 3 s read timeout. Re-ran the interleaving probe: 3,000 sends, zero inconsistent results. Compiles and :core:data tests stay green.
This commit is contained in:
1 parent
b95a86c97f
commit
baf2a7022d
1 file changed
+24
-17
@@ -77,24 +77,31 @@ class AprsIsClient(
|
|||||||
val w = writer ?: return null
|
val w = writer ?: return null
|
||||||
w.println(packetLine)
|
w.println(packetLine)
|
||||||
if (w.checkError()) return Pair(false, "write failed")
|
if (w.checkError()) return Pair(false, "write failed")
|
||||||
}
|
// Read the server response inside the same lock: disconnect() (called
|
||||||
// Try to read the server response (short 3 s timeout; APRS-IS replies with an error line on bad format)
|
// concurrently from stop()/reconnect on another thread) nulls
|
||||||
return runCatching {
|
// writer/reader/socket and closes them. Reading outside the lock raced
|
||||||
val s = socket ?: return@runCatching Pair(true, "OK")
|
// with that: the response read could hit a just-closed socket and the
|
||||||
val oldTimeout = s.soTimeout
|
// swallowing runCatching reported Pair(true,"OK") for a packet that
|
||||||
s.soTimeout = 3000
|
// never left, or read through a stale reference. Serialising keeps
|
||||||
try {
|
// the read on the connection this thread just wrote to. The 3 s read
|
||||||
val resp = reader?.readLine()
|
// timeout bounds how long a concurrent disconnect waits.
|
||||||
if (resp != null && (resp.contains("Invalid", ignoreCase = true) ||
|
return runCatching {
|
||||||
resp.contains("error", ignoreCase = true))) {
|
val s = socket ?: return@runCatching Pair(true, "OK")
|
||||||
Pair(false, resp.trim())
|
val oldTimeout = s.soTimeout
|
||||||
} else {
|
s.soTimeout = 3000
|
||||||
Pair(true, if (resp.isNullOrBlank()) "OK" else resp.trim())
|
try {
|
||||||
|
val resp = reader?.readLine()
|
||||||
|
if (resp != null && (resp.contains("Invalid", ignoreCase = true) ||
|
||||||
|
resp.contains("error", ignoreCase = true))) {
|
||||||
|
Pair(false, resp.trim())
|
||||||
|
} else {
|
||||||
|
Pair(true, if (resp.isNullOrBlank()) "OK" else resp.trim())
|
||||||
|
}
|
||||||
|
} finally {
|
||||||
|
s.soTimeout = oldTimeout
|
||||||
}
|
}
|
||||||
} finally {
|
}.getOrElse { Pair(true, "OK") }
|
||||||
s.soTimeout = oldTimeout
|
}
|
||||||
}
|
|
||||||
}.getOrElse { Pair(true, "OK") }
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Read one line (server response; throws on timeout) */
|
/** Read one line (server response; throws on timeout) */
|
||||||
|
|||||||
Reference in new issue
Block a user