Skip to content

refactor(kbhttp): one reconciling goroutine for both local servers - #29652

Draft
chrisnojima wants to merge 4 commits into
nojima/HOTPOT-as-03-appstatefrom
nojima/HOTPOT-as-04-kbhttp
Draft

chrisnojima wants to merge 4 commits into
nojima/HOTPOT-as-03-appstatefrom
nojima/HOTPOT-as-04-kbhttp

Conversation

@chrisnojima

@chrisnojima chrisnojima commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Stack 4/9, splitting #29637. Parent: #29651.

Why

The local http server serves chat attachments, avatars and maps to the app's own webviews and image components. When it is down, images silently fail. On master there are two separate implementations of "keep a local server alive across app-state changes" — the service's and KBFS's — each with its own monitor, lock and restart path, and neither recovers from the server dying on its own.

Master bugs this fixes

  • A dead server stays dead. startHTTPSrv starts the server and returns; nothing watches for Serve exiting. If it exits unexpectedly, the server stays down until the next app-state transition happens to restart it. Until then every attachment, avatar and map fails to load.
  • The server is torn down on INACTIVE, so a transient state like Control Center or a system alert kills image loading. With feat(lifecycle): derive the app state from a lifecycle controller #29651 the app reports INACTIVE far more often (including every cold launch and resume), which would make this much worse.
  • Two duplicate implementations. KBFS's libhttpserver has its own restart(), monitorAppState(), lock and cancel func, so a fix has to be made twice and the two have already drifted — KBFS's monitor only ever restarted, never stopped.
  • config.go polls Addr() for up to 2 seconds to learn the address instead of asking once.
  • The server does not keep its last bound address while stopped, so callers that need a URL while backgrounded get nothing.

What this changes

One reconciling goroutine owns both servers. manager.New takes appState/nextAppState as plain function parameters rather than an interface — that is what lets KBFS pass its own AppStateUpdater and the service pass MobileAppState without a shared type. reconcile tears the server down only in BACKGROUND, and only where stopInBackground. The server restarts after an unexpected Serve exit, keeps its last bound address while stopped, and rebinds when leaving the background. libhttpserver deletes its own machinery and delegates. config.go makes one Info() call.

httpSrv moves from NewService to SetupCriticalSubServices because the new Srv reads the notify router once at construction, and the router does not exist yet in NewService.

Also includes the localhost-src slice: a retried image src is repointed at the current server address and token, so an image baked with an old address recovers instead of failing forever.

One fix beyond a straight port of #29637

#29637 repoints any http://127.0.0.1:PORT src. KBFS serves /files/ from a second local server on its own port with its own independently-rotated token, so that rewrote KBFS URLs to the chat server's address and token — turning a transient image failure into a permanent wrong-port/403. Before this PR the retry only appended a cache-buster, so the regression would have been introduced here. Repointing is now gated on the service's own endpoints (/at, /av, /map); everything else still gets the cache-buster. Reachable from the KBFS file preview, which passes these URLs to ZoomableImage. Regression test added.

Verification

go build, go vet, gofmt, golangci-lint --new-from-rev master 0 issues. kbhttp, libhttpserver and attachment tests pass, including under -race and repeated runs. yarn lint:all clean, unit suite green.

Worth a decision during review: stopInBackground means KBFS's server is now torn down in BACKGROUND and no longer binds at New() on mobile, where the initial state is BACKGROUND — so Server.Address() and GetGUIFileContext fail until the first foreground. That is a real behaviour change from master, where it was never stopped.

@chrisnojima
chrisnojima added this pull request to stack #29658 September 21, 2026 18:56
@chrisnojima chrisnojima changed the title nojima/HOTPOT as 04 kbhttp refactor(kbhttp): one reconciling goroutine for both local servers Sep 21, 2026
StartWithHandlers populates the ServeMux under the same lock that binds the
listener, so no request can reach a freshly started server before its
handlers exist.
manager.Srv now owns its kbhttp.Srv from a single goroutine: run reads the
app state it started in, then reconciles on every app state change, handler
registration and shutdown. Handlers are registered into the mux as the
listener binds, so a restart can't answer a 404, and the last bound address
is kept while the server is stopped so URLs built then point at where it
comes back.

kbfs's libhttpserver drops its own restart/monitorAppState/serverLock copy of
this logic and takes a manager.Srv, passing its env.AppStateUpdater methods
as the app state functions.

The server now stays up in INACTIVE, so chat's getURL no longer refuses to
build a URL while it is down, and GetBootstrapStatus reads the address once
instead of polling Addr for two seconds.

The service builds its Srv in SetupCriticalSubServices rather than
NewService: the new Srv reads NotifyRouter once at construction, so the
router has to exist first.
A retry of a 127.0.0.1 src rewrote nothing but a cache-buster, so an
attachment url baked with an old port or an old per-process token kept
failing. localhost-src rewrites both from the httpSrv info the service
announced. The server now stays up while the app is inactive, so only
BACKGROUND holds off the retry.
kbfs runs a second local server on 127.0.0.1 with its own port and its own
token, and its /files/ urls reach the same native Image. Matching on the
127.0.0.1 prefix alone rewrote those to the service's address and token, so
a transient failure became a permanent wrong-port 403. Repoint only the
service's own endpoints; everything else still gets the cache-buster.
@chrisnojima
chrisnojima force-pushed the nojima/HOTPOT-as-04-kbhttp branch from bb3a6e2 to 8d1d7d0 Compare September 22, 2026 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant