From 09e728fa1b032ba304bf2fb539e9b0bd49827f04 Mon Sep 17 00:00:00 2001 From: QIU Date: Fri, 14 Aug 2026 14:09:42 +0000 Subject: [PATCH] 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. --- .../look4sat/core/data/aprs/AprsIsClient.kt | 49 +++++++++++-------- .../core/data/framework/BluetoothReporter.kt | 14 ++++++ .../core/data/framework/Ft817Controller.kt | 9 ++++ .../core/data/framework/Ic705Controller.kt | 9 ++++ 4 files changed, 61 insertions(+), 20 deletions(-) diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClient.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClient.kt index 1893f105..27770496 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClient.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClient.kt @@ -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 } } diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/BluetoothReporter.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/BluetoothReporter.kt index 17721c81..9d1b5f18 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/BluetoothReporter.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/BluetoothReporter.kt @@ -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 diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/Ft817Controller.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/Ft817Controller.kt index 42dec0ac..6affe684 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/Ft817Controller.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/Ft817Controller.kt @@ -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 } diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/Ic705Controller.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/Ic705Controller.kt index 3a890583..ce17b086 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/Ic705Controller.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/Ic705Controller.kt @@ -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 }