Control server: no socket timeout and unlimited connections allow thread exhaustion #1

New Issue

benjamin · 2026-08-26T01:52:30+02:00

benjamin commented

2026-08-26 01:52:30 +02:00

Problem

ControlServer.startServer() calls start(0, true) (ControlServer.kt:74). NanoHTTPD only calls Socket.setSoTimeout() when the timeout argument is > 0 — verified in the 2.3.1 bytecode (ServerRunnable.run: if (timeout <= 0) skip setSoTimeout). With 0 no read deadline is ever set, so reads block forever.

NanoHTTPD spawns one unbounded thread per accepted connection. RemoteSocket.onOpen() (ControlServer.kt:175) adds every socket to clients unconditionally, and unauthorized non-hello messages are silently dropped while the connection stays open — there is no handshake timeout and no connection cap.

Failure scenario

Any host on the LAN opens connections to :8765 without ever sending hello. Each pins a thread that is never reaped. A few thousand connections exhaust threads/FDs and the control server (and possibly the app) stops responding for legitimate users. No authentication is needed to trigger this.

The same leak happens accidentally: a phone that drops Wi-Fi mid-handshake leaves a thread pinned forever.

Suggested fix

start(NanoHTTPD.SOCKET_READ_TIMEOUT, true) so accepted sockets get a read deadline
Drop connections that have not authenticated within a few seconds
Cap concurrent clients (a handful is plenty for a TV remote)

Found by multi-agent code review; verified against nanohttpd 2.3.1 bytecode and current ControlServer.kt.

## Problem `ControlServer.startServer()` calls `start(0, true)` (`ControlServer.kt:74`). NanoHTTPD only calls `Socket.setSoTimeout()` when the timeout argument is `> 0` — verified in the 2.3.1 bytecode (`ServerRunnable.run`: `if (timeout <= 0) skip setSoTimeout`). With `0` no read deadline is ever set, so reads block forever. NanoHTTPD spawns one unbounded thread per accepted connection. `RemoteSocket.onOpen()` (`ControlServer.kt:175`) adds every socket to `clients` unconditionally, and unauthorized non-`hello` messages are silently dropped while the connection stays open — there is no handshake timeout and no connection cap. ## Failure scenario Any host on the LAN opens connections to `:8765` without ever sending `hello`. Each pins a thread that is never reaped. A few thousand connections exhaust threads/FDs and the control server (and possibly the app) stops responding for legitimate users. No authentication is needed to trigger this. The same leak happens accidentally: a phone that drops Wi-Fi mid-handshake leaves a thread pinned forever. ## Suggested fix - `start(NanoHTTPD.SOCKET_READ_TIMEOUT, true)` so accepted sockets get a read deadline - Drop connections that have not authenticated within a few seconds - Cap concurrent clients (a handful is plenty for a TV remote) Found by multi-agent code review; verified against nanohttpd 2.3.1 bytecode and current `ControlServer.kt`.

benjamin referenced this issue from a commit

2026-08-26 02:14:49 +02:00

Resolve the full review backlog (issues #1-#13)

benjamin commented

2026-08-26 02:15:43 +02:00

Fixed in dd612fc, shipped in v0.9.0.

startServer() now uses start(NanoHTTPD.SOCKET_READ_TIMEOUT, true), so accepted sockets get a read deadline instead of blocking forever.
The ping timer became maintainClients(): it drops any socket that has not authenticated within 10 s, alongside the existing ping/reap.
onOpen() refuses connections beyond MAX_CLIENTS = 8 and closes them with a policy-violation frame.

A stalled or hostile connection can therefore no longer pin a thread indefinitely.

Fixed in dd612fc, shipped in v0.9.0. - `startServer()` now uses `start(NanoHTTPD.SOCKET_READ_TIMEOUT, true)`, so accepted sockets get a read deadline instead of blocking forever. - The ping timer became `maintainClients()`: it drops any socket that has not authenticated within 10 s, alongside the existing ping/reap. - `onOpen()` refuses connections beyond `MAX_CLIENTS = 8` and closes them with a policy-violation frame. A stalled or hostile connection can therefore no longer pin a thread indefinitely.

benjamin closed this issue

2026-08-26 02:15:44 +02:00

benjamin referenced this issue from a commit

2026-08-26 02:35:25 +02:00

Rename app to Castarr (dev.castarr.tv), add JDK toolchain resolution

benjamin referenced this issue from a commit

2026-08-26 02:35:26 +02:00

Resolve the full review backlog (issues #1-#13)

benjamin referenced this issue from a commit

2026-08-26 10:33:40 +02:00

Stop the socket timeout from killing the remote mid-session

Sign in to join this conversation.