wap: keep the unowned-box LAN setup listeners in step with the interfaces - #248
Merged
Conversation
…aces startLANSetupListeners binds once, at startup. But wap starts with the box and the network arrives later — DHCP, or a cable the user plugs in after powering it on — so LAN setup only ever worked on a box that happened to be wired before boot. Everywhere else it silently offered nothing. The watcher re-evaluates lanListenAddrs each tick and binds whatever is new. Deliberately NOT the retry-until-success shape of watchForHotspot: the addresses are not known in advance, there can be several, and a second interface can come up long after the first one bound. It tracks what it already holds, so a steady state costs one interface enumeration and no bind attempts — otherwise every tick would retry an address it owns and fill the log with EADDRINUSE. It stops the moment the box gains an owner, and refuses to start on a box that already has one. lanSetupGuard would refuse those requests anyway, since it re-checks ownership per request, but a listener opened after setup is a socket that should not exist; leaning on the guard would be relying on the second line of defence to cover for the first. lanListenAddrs is now indirected through a variable so a test can drive "a cable was plugged in" without reconfiguring the host's interfaces. startAuxListeners also switched from `mc.listeners = append(...)` to `mc.add()`: the watcher adds listeners concurrently, and add() takes the mutex and refuses during shutdown. Tests: an address appearing later is bound; a steady state does not rebind; the watcher stops once the Blox is claimed (an address appearing after setup is NOT bound); and it never starts on an already-owned box. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QyQw3WtVXDTtvJKN7ykczw
The map needs no lock only because exactly one goroutine touches it after the synchronous seed. That is invisible at the call site, so a reviewer reading the code flagged it as a possible race -- which it would become the moment a caller kept using its copy. Say so at both ends. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJyqpFReP1wbz86nsqsoXq
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
An unconfigured Blox serves the setup API on its hotspot address and on whatever LAN addresses it had at boot. A box that gets a LAN address later — ethernet plugged in after power-on, a slow DHCP lease, Wi-Fi joined by a previous owner — never binds to it, so a browser on that network cannot reach setup even though the box is sitting right there on the LAN.
startLANSetupWatchre-enumerates the interfaces every 15 s and binds any address it has not bound yet, serving the same guarded, CORS-wrapped mux. It stops as soon as the box has an owner, and onmultiCloser.Close().Why now
The web setup app (fxblox-web) is adding a "my Blox is already on my home network — here is its address" step as a last resort after Bluetooth fails and before asking the user to give up their internet for the hotspot. That step is only useful if the box is actually listening on the address the user types, which today it often is not.
Shape
boundis seeded synchronously by the boot-time bind, so addresses already listening are never retried andEADDRINUSEdoes not fill the log.context.Contexton purpose:Serve()creates a ctx withdefer cancel(), so handing that to a background goroutine gives it a context already cancelled by the time the first tick arrives — the watcher would return immediately and the bug would look fixed while doing nothing. The stop signal is achan struct{}onmultiCloserrather than acontext.CancelFunc, whichgo vet's lostcancel analyzer flags when stored in a struct.mc.add(ln)is the thread-safe path and closes the listener itself ifClose()already ran, so a listener cannot outlive the server.bloxHasOwner()at both entry and every tick: a box that has been claimed never exposes setup on the LAN.Tests
Four new tests in
lanbind_test.go(interval and address enumeration are indirected for tests): a late address gets bound and served, an already-bound address is not retried,Close()stops the watcher, and an owned box never starts one.go test ./wap/pkg/server/passes 32/32;go vet ./..., the shadow analyzer and gofmt are clean.Rollout
This only reaches devices through a GitHub Release (
publish.ymlison: release) and then an OTA — merging to main publishes nothing. Until that release ships, the web app's LAN step will report "no Blox answered at that address" on boxes that acquired their address after boot, and users fall through to the hotspot as they do today.🤖 Generated with Claude Code
https://claude.ai/code/session_01YJyqpFReP1wbz86nsqsoXq