From 1e246736320040f5756e99b7680eb9e8d000bd5c Mon Sep 17 00:00:00 2001 From: QIU Date: Sun, 16 Aug 2026 09:36:08 +0000 Subject: [PATCH] fix(network): close sockets on setup failure and on write failure Two leaks in NetworkReporter, the same family as the Bluetooth/APRS/radio socket leaks fixed earlier: 1. ensureRotatorConnected/ensureFrequencyConnected assigned the field directly, so a channel that opened but threw during the rest of setup was never closed and remained referenced. Use a local `opened` and close it in the catch, matching the pattern used in AprsIsClient/Ic705Controller/ Ft817Controller/BluetoothReporter. 2. write() only flipped connected=false on failure. The broken channel stayed in the field, the next ensure* reconnected and overwrote it, and the old channel was never closed. Now a failed write closes the channel and nulls the field. The null check is identity-based (socket === field) so a stale reference from a concurrent report can never close a newer channel. State-machine probe: normal write keeps the socket, failed write closes and nulls, next report reconnects fresh, and passing a stale reference does not close the newer socket. --- .../core/data/framework/NetworkReporter.kt | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/NetworkReporter.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/NetworkReporter.kt index b116b190..4297e8cd 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/NetworkReporter.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/framework/NetworkReporter.kt @@ -71,14 +71,19 @@ class NetworkReporter( private fun ensureRotatorConnected() { if (rotatorConnected || rotatorConnecting || rotatorServer.isBlank()) return reporterScope.launch { + var opened: SocketChannel? = null try { rotatorConnecting = true - rotatorSocket = SocketChannel.open(InetSocketAddress(rotatorServer, rotatorPort)) + opened = SocketChannel.open(InetSocketAddress(rotatorServer, rotatorPort)) + rotatorSocket = opened rotatorConnected = true println("NetworkReporter: Rotator connected to $rotatorServer:$rotatorPort") } catch (e: Exception) { println("NetworkReporter rotator connect error: ${e.message}") rotatorConnected = false + // Close a socket that connected but failed during setup, so a + // broken channel is never left referenced without a closer. + opened?.close() } finally { rotatorConnecting = false } @@ -88,14 +93,17 @@ class NetworkReporter( private fun ensureFrequencyConnected() { if (frequencyConnected || frequencyConnecting || frequencyServer.isBlank()) return reporterScope.launch { + var opened: SocketChannel? = null try { frequencyConnecting = true - frequencySocket = SocketChannel.open(InetSocketAddress(frequencyServer, frequencyPort)) + opened = SocketChannel.open(InetSocketAddress(frequencyServer, frequencyPort)) + frequencySocket = opened frequencyConnected = true println("NetworkReporter: Frequency connected to $frequencyServer:$frequencyPort") } catch (e: Exception) { println("NetworkReporter frequency connect error: ${e.message}") frequencyConnected = false + opened?.close() } finally { frequencyConnecting = false } @@ -111,6 +119,17 @@ class NetworkReporter( } catch (e: Exception) { println("NetworkReporter write error: ${e.message}") onError() + // The channel failed a write: drop it and its closure obligation. + // Leaving it referenced lets the next connect overwrite the field + // and leak the old channel. Only the field the caller passed is + // cleared, matching the connected=false the onError sets. + if (socket === rotatorSocket) { + rotatorSocket?.close() + rotatorSocket = null + } else if (socket === frequencySocket) { + frequencySocket?.close() + frequencySocket = null + } } }