Skip to content

Commit 0019903

Browse files
ADFA-5035: Fix WebServer occasionally failing to start with EADDRINUSE (#1634)
* ADFA-5035: Fix WebServer occasionally failing to start with EADDRINUSE start() binds serverSocket on a background thread (launched from MainActivity.startWebServer()); stop() (called from onDestroy(), main thread) only closes serverSocket if it's already initialized. If stop() runs before start() reaches bind(), it's a silent no-op -- start() then binds anyway a moment later, orphaned, holding the port until the process dies. The next start() attempt on that port fails with "Address already in use." Synchronize start()'s bind and stop()'s close on a shared lock, and have stop() record that a stop was requested so start() can abort before binding if one arrived first. Closes the race window instead of relying on timing. Also renamed HTTP_INTERNAL_SERVER_ERROR/HTTP_NOT_FOUND to camelCase (pre-existing ktlint property-naming violations, unrelated to this fix but required once this file falls under the Spotless ratchet). * ADFA-5035: Apply Spotless ratchet reformat to WebServer.kt WebServer.kt was space-indented and had several pre-existing ktlint violations (max-line-length, snake_case sql_query). Touching the file in the previous commit pulled it under the Spotless ratchet, so bring it into compliance: tabs, wrapped long lines/comments, and sql_query -> sqlQuery. No behavioral change. * ADFA-5035: Address code review findings - Close database in start()'s finally alongside serverSocket. It was opened before the stopRequested check that can now abort start() early, and was never closed on any other shutdown path either (normal accept-loop exit, exception) -- isInitialized guards the case where opening it failed and this finally still runs. - Correct stop()'s doc comment: it's no longer a full no-op before start() binds -- it still records the stop request so start() can abort before binding, which is the fix itself. Only the socket-close side stays a no-op in that case. (Reverting the behavior back to a literal no-op, as literally suggested, would reopen the exact EADDRINUSE race this ticket fixes.) - Add WebServerTest: deterministic coverage for both lifecycle orderings (stop-before-start aborts the bind; start-then-stop frees the port for reuse), synchronized via the port's own bind/connect behavior rather than fixed sleeps. Skipped: the outputStarted-timing suggestion for realHandleBsEndpoint/ realHandlePrEndpoint is the same CodeRabbit finding already considered and explicitly rejected in an existing code comment ("I disagree... --DS, 23-Feb-2026"); out of scope to unilaterally revisit here. * ADFA-5035: Fix outputStarted timing in handleBsEndpoint/handlePrEndpoint outputStarted was only set after realHandleBsEndpoint/realHandlePrEndpoint returned, so if writeNormalToClient threw partway through (e.g. after the status line but mid-body), the catch block still saw outputStarted=false and sent a second, well-formed response on top of the already-partially-written one. Pass a markOutputStarted callback into both functions and invoke it right before the first write, so the caller's flag reflects reality even when the write itself then fails. Removes the "I disagree with CodeRabbit's message" comment that had left this finding unaddressed.
1 parent a1700fa commit 0019903

2 files changed

Lines changed: 1231 additions & 1008 deletions

File tree

0 commit comments

Comments
 (0)