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.
This commit is contained in:
mckero committed 2026-08-16 09:36:08 +00:00
1 parent ed0f5678b3
commit 1e24673632
1 file changed
+21 -2
@@ -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
}
}
}