fix(data): close sockets when connect() fails after the handshake
All five connect paths opened a socket, completed the TCP/RFCOMM handshake,
and only afterwards stored it in a field. Any exception in between leaked the
socket: the catch block just flipped a boolean, and disconnect() can only close
what already reached the fields.
Leak windows (statements that can throw after the handshake succeeded):
AprsIsClient.connect soTimeout / tcpNoDelay / getOutputStream / getInputStream
Ic705Controller.connect outputStream / inputStream / sendAndWaitAck
Ft817Controller.connect outputStream / inputStream
BluetoothReporter x2 outputStream
AprsIsClient is the worst case because AprsReporter retries on a timer
(intervalMin, minimum 1 minute) and nulls out the client after each failure,
so every failed attempt permanently loses one fd:
failure rate leaked fds/hour time to exhaust 1024 fds
5% 3.0 ~14.2 days
20% 12.0 ~3.6 days
50% 30.0 ~1.4 days
100% 60.0 ~17 hours
Typical trigger is a weak link where the TCP handshake succeeds but the peer
immediately RSTs (overloaded or rate-limiting APRS-IS server). Once fds run out
nothing in the process can open a socket or file any more: TLE updates, AMSAT
status and WaveLog uploads all start failing with no obvious cause.
Each path now keeps a local reference to the socket it opened and closes it in
the catch block, also clearing the stream/socket fields so a half-initialised
connection is not mistaken for a live one.
Verified: :core:data:compileReleaseKotlin BUILD SUCCESSFUL; grep confirms all
five close calls are present.
This commit is contained in:
1 parent
c1895506e0
commit
09e728fa1b
4 files changed
+61
-20
No files matched your search
@@ -35,27 +35,36 @@ class AprsIsClient(
|
||||
fun connect() {
|
||||
disconnect()
|
||||
val s = Socket()
|
||||
s.connect(InetSocketAddress(host, port), 30_000)
|
||||
s.soTimeout = timeoutSec * 1000
|
||||
s.tcpNoDelay = true
|
||||
synchronized(lock) {
|
||||
socket = s
|
||||
writer = PrintWriter(OutputStreamWriter(s.getOutputStream(), Charsets.ISO_8859_1), true)
|
||||
reader = BufferedReader(InputStreamReader(s.getInputStream(), Charsets.ISO_8859_1), 256)
|
||||
}
|
||||
// Login line
|
||||
val login = AprsPacket.formatLogin(callsign, ssid, passcode, version) + filter
|
||||
writer?.println(login)
|
||||
// Read the login response (aprsc replies # logresp ... verified/unverified)
|
||||
runCatching {
|
||||
s.soTimeout = 8000
|
||||
val resp = reader?.readLine()
|
||||
if (resp != null && (resp.contains("Invalid", ignoreCase = true) ||
|
||||
resp.contains("unverified", ignoreCase = true))) {
|
||||
throw IllegalArgumentException(resp.trim())
|
||||
}
|
||||
// Restore timeout
|
||||
try {
|
||||
s.connect(InetSocketAddress(host, port), 30_000)
|
||||
s.soTimeout = timeoutSec * 1000
|
||||
s.tcpNoDelay = true
|
||||
synchronized(lock) {
|
||||
socket = s
|
||||
writer = PrintWriter(OutputStreamWriter(s.getOutputStream(), Charsets.ISO_8859_1), true)
|
||||
reader = BufferedReader(InputStreamReader(s.getInputStream(), Charsets.ISO_8859_1), 256)
|
||||
}
|
||||
// Login line
|
||||
val login = AprsPacket.formatLogin(callsign, ssid, passcode, version) + filter
|
||||
writer?.println(login)
|
||||
// Read the login response (aprsc replies # logresp ... verified/unverified)
|
||||
runCatching {
|
||||
s.soTimeout = 8000
|
||||
val resp = reader?.readLine()
|
||||
if (resp != null && (resp.contains("Invalid", ignoreCase = true) ||
|
||||
resp.contains("unverified", ignoreCase = true))) {
|
||||
throw IllegalArgumentException(resp.trim())
|
||||
}
|
||||
// Restore timeout
|
||||
s.soTimeout = timeoutSec * 1000
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
// Close the local socket before re-throwing, so it does not leak when
|
||||
// an exception is raised after s.connect() but before socket = s.
|
||||
// Otherwise periodic reconnect attempts (AprsReporter every 1–60 min)
|
||||
// accumulate leaked fds until the process cannot open any more files.
|
||||
runCatching { s.close() }
|
||||
throw e
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+14
@@ -76,10 +76,12 @@ class BluetoothReporter(
|
||||
private fun ensureRotatorConnected() {
|
||||
if (rotatorConnected || rotatorConnecting || rotatorDeviceId.isBlank()) return
|
||||
reporterScope.launch {
|
||||
var opened: android.bluetooth.BluetoothSocket? = null
|
||||
try {
|
||||
rotatorConnecting = true
|
||||
val device = bluetoothManager.adapter.getRemoteDevice(rotatorDeviceId)
|
||||
val socket = device.createInsecureRfcommSocketToServiceRecord(sppId)
|
||||
opened = socket
|
||||
socket.connect()
|
||||
rotatorSocket = socket
|
||||
rotatorStream = socket.outputStream
|
||||
@@ -87,6 +89,11 @@ class BluetoothReporter(
|
||||
Log.i(tag, "Rotator connected to $rotatorDeviceId")
|
||||
} catch (e: Exception) {
|
||||
Log.e(tag, "Rotator connect error: ${e.message}")
|
||||
// Close the socket we opened, otherwise a failure after connect()
|
||||
// leaks it: nothing else holds a reference once this returns.
|
||||
runCatching { opened?.close() }
|
||||
rotatorSocket = null
|
||||
rotatorStream = null
|
||||
rotatorConnected = false
|
||||
} finally {
|
||||
rotatorConnecting = false
|
||||
@@ -97,10 +104,12 @@ class BluetoothReporter(
|
||||
private fun ensureFrequencyConnected() {
|
||||
if (frequencyConnected || frequencyConnecting || frequencyDeviceId.isBlank()) return
|
||||
reporterScope.launch {
|
||||
var opened: android.bluetooth.BluetoothSocket? = null
|
||||
try {
|
||||
frequencyConnecting = true
|
||||
val device = bluetoothManager.adapter.getRemoteDevice(frequencyDeviceId)
|
||||
val socket = device.createInsecureRfcommSocketToServiceRecord(sppId)
|
||||
opened = socket
|
||||
socket.connect()
|
||||
frequencySocket = socket
|
||||
frequencyStream = socket.outputStream
|
||||
@@ -108,6 +117,11 @@ class BluetoothReporter(
|
||||
Log.i(tag, "Frequency connected to $frequencyDeviceId")
|
||||
} catch (e: Exception) {
|
||||
Log.e(tag, "Frequency connect error: ${e.message}")
|
||||
// Close the socket we opened, otherwise a failure after connect()
|
||||
// leaks it: nothing else holds a reference once this returns.
|
||||
runCatching { opened?.close() }
|
||||
frequencySocket = null
|
||||
frequencyStream = null
|
||||
frequencyConnected = false
|
||||
} finally {
|
||||
frequencyConnecting = false
|
||||
|
||||
@@ -53,9 +53,11 @@ class Ft817Controller(
|
||||
override suspend fun connect(): Boolean = withContext(Dispatchers.IO) {
|
||||
if (isConnected) return@withContext true
|
||||
if (deviceAddress.isBlank()) return@withContext false
|
||||
var opened: android.bluetooth.BluetoothSocket? = null
|
||||
try {
|
||||
val device = bluetoothManager.adapter.getRemoteDevice(deviceAddress)
|
||||
val btSocket = device.createInsecureRfcommSocketToServiceRecord(sppId)
|
||||
opened = btSocket
|
||||
btSocket.connect()
|
||||
socket = btSocket
|
||||
outputStream = btSocket.outputStream
|
||||
@@ -66,6 +68,13 @@ class Ft817Controller(
|
||||
true
|
||||
} catch (e: Exception) {
|
||||
Log.e(tag, "Connect error: ${e.message}")
|
||||
// Close the socket we opened. Without this a failure after connect()
|
||||
// (e.g. outputStream throwing) leaks the Bluetooth socket, because
|
||||
// disconnect() only closes what already reached the fields.
|
||||
runCatching { opened?.close() }
|
||||
socket = null
|
||||
outputStream = null
|
||||
inputStream = null
|
||||
isConnected = false
|
||||
false
|
||||
}
|
||||
|
||||
@@ -67,9 +67,11 @@ class Ic705Controller(
|
||||
override suspend fun connect(): Boolean = withContext(Dispatchers.IO) {
|
||||
if (isConnected) return@withContext true
|
||||
if (deviceAddress.isBlank()) return@withContext false
|
||||
var opened: android.bluetooth.BluetoothSocket? = null
|
||||
try {
|
||||
val device = bluetoothManager.adapter.getRemoteDevice(deviceAddress)
|
||||
val btSocket = device.createInsecureRfcommSocketToServiceRecord(sppId)
|
||||
opened = btSocket
|
||||
btSocket.connect()
|
||||
socket = btSocket
|
||||
outputStream = btSocket.outputStream
|
||||
@@ -84,6 +86,13 @@ class Ic705Controller(
|
||||
true
|
||||
} catch (e: Exception) {
|
||||
Log.e(tag, "Connect error: ${e.message}")
|
||||
// Close the socket we opened. Without this a failure after connect()
|
||||
// (e.g. outputStream throwing) leaks the Bluetooth socket, because
|
||||
// disconnect() only closes what already reached the fields.
|
||||
runCatching { opened?.close() }
|
||||
socket = null
|
||||
outputStream = null
|
||||
inputStream = null
|
||||
isConnected = false
|
||||
false
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user