Skip to content

fix(extension): auto-start must not kill a running server (restart storm, #112) - #113

Open
laulpogan wants to merge 1 commit into
ygncode:mainfrom
laulpogan:fix/auto-start-must-not-kill-live-server
Open

laulpogan wants to merge 1 commit into
ygncode:mainfrom
laulpogan:fix/auto-start-must-not-kill-live-server

Conversation

@laulpogan

Copy link
Copy Markdown

Problem

.pi/extensions/pi-web.ts starts the server opportunistically on every extension load — once per pi process, including the pi --mode rpc workers that pi-web itself spawns. When the health check failed, startPiWeb() ran:

launchctl kickstart -k "gui/$(id -u)/com.pi-web"

-k stops the running instance first. pi-web handles SIGTERM and exits 0, so launchd records a successful exit and the kill is invisible in last exit code — it looks like a voluntary quit, not a kill.

The kill briefly takes the port down, so every other pi process starting in that window fails its own health check and issues its own kickstart -k. Each restart manufactures more failures, and because pi-web's own RPC workers load this extension, the server ends up being restarted by its own children.

Observed on a macOS 15.8 / arm64 host, beta.36, ~200 sessions, load average 40–59:

runs = 582          last exit code = 0        #62 exits in 14 minutes

with intermittent 502s from tailscale serve, which is what made the phone PWA unreachable. Bursts came and went depending on how many pi processes were starting; overnight, with nothing starting, zero restarts.

The confusing part for diagnosis: a manually started pi-web is immune, because kickstart -k addresses the launchd job, not a hand-run process. That reliably sends you hunting through launchd environment, stdin, ThrottleInterval and plist churn instead of here. Full write-up with A/B evidence in #112.

Change

One file, .pi/extensions/pi-web.ts:

  • startPiWeb() can no longer kill. If something is already bound to the port it is not "not running", so it exits without touching the job; otherwise plain launchctl start (no -k).
  • ensurePiWebRunning() confirms before acting. A second check 1.5 s later, because one failed fetch is not proof that a server other processes are using is dead.
  • Health-check timeout 1 s → 4 s. A healthy server answers in ~3 ms measured; since 401/403 already count as up, the only way to fail is a timeout — precisely the transient state a loaded machine produces.

/pi-web restart keeps kickstart -k. An explicit user request to restart should still kill; only the implicit, load-time path needed the kill removed.

Verification

npm run test:extensions   →  Test Files 2 passed (2), Tests 46 passed (46)

All markers asserted by internal/app/extension_static_test.go (pi.registerCommand("pi-web"|"remote"|"refresh"), Usage: /pi-web …, launchctl, systemctl, import("qrcode")) are present. go test ./internal/app/ could not run in my environment — it fails at web/assets_embed.go:8: pattern all:dist: no matching files found because the frontend dist isn't checked in; I verified those markers by inspection instead.

A/B against the live launchd job on the reporting machine (state = running, child pid 6326):

command run runs child pid tailnet
patched start path 1 (unchanged) 6326 (unchanged) 200
launchctl kickstart -k (beta.36 behaviour) 2 (+1) replaced within 2 s recovered

The reporting machine has run this patch since it was written; no further restarts.

Not in this PR

Findings 2 and 3 from #112 are separate and untouched here: the serveRuleConflict false alarm in tailscaleServeRuleState (internal/app/tailscale.go:120, which looks for the port as a JSON key and so matches TCP[port] == {"HTTPS": true} — zero strings — instead of Web["<host>:<port>"].Handlers[*].Proxy), and Main() returning normally after SIGTERM so the shutdown cause is indistinguishable from a clean exit. Happy to send either as a follow-up if useful.

`ensurePiWebRunning()` runs on every extension load — i.e. once per pi
process, including the `pi --mode rpc` workers that pi-web itself spawns.
When its health check failed, it called `startPiWeb()`, which ran
`launchctl kickstart -k`. The `-k` flag stops the running instance first,
and pi-web handles SIGTERM by exiting 0, so launchd recorded a *successful*
exit and the kill was invisible in `last exit code`.

That single kill briefly takes the port down, so every other pi process
starting in that window fails its own check and issues its own kickstart.
Each restart manufactures more failures: a self-sustained restart storm
(observed here at ~6 exits/min, `runs = 582`, with 502s from
`tailscale serve` making the phone PWA unreachable).

- `startPiWeb()` no longer kills: if something is already bound to the
  port it is not "not running", so exit without touching the job;
  otherwise plain `launchctl start`.
- `ensurePiWebRunning()` confirms with a second check before acting,
  because one fetch is not proof that a server other processes are using
  is dead.
- Health-check timeout 1s -> 4s: a healthy server answers in ~3ms, but
  401/403 already count as up, so the only way to fail is a timeout —
  exactly the transient state a loaded machine produces.

`/pi-web restart` keeps `kickstart -k`: an explicit user request to
restart should still kill.

Refs ygncode#112
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