Skip to content

fix(wallet): exit when the daemon dies, stop per-call logging, never unlink a successor's socket - #40

Open
TeoSlayer wants to merge 3 commits into
mainfrom
fix/wallet-lifecycle
Open

TeoSlayer wants to merge 3 commits into
mainfrom
fix/wallet-lifecycle

Conversation

@TeoSlayer

Copy link
Copy Markdown
Contributor

Problem

An app-lifecycle sweep of io.pilot.wallet (the catalogue's 0.3.3 bundle, plus source builds of 0.3.3 and main, on darwin/arm64, darwin/amd64 under Rosetta, linux/amd64 and linux/arm64) found three leaks. In the sweep, the app's parent was SIGKILLed and the app was watched for 6 s.

  1. Orphans. The app-store supervisor starts the wallet in its own process group. macOS has no parent-death signal, and on Linux only Pdeathsig covers it. When the daemon dies without stopping its apps (SIGKILL, crash, watchdog exit, launchd restart), the wallet is reparented to launchd/init and keeps serving its socket with the sqlite ledger and cap-state.jsonl open. The next daemon then starts a second wallet next to it.
    • Harness result: ORPHAN: app still alive 6s after parent SIGKILL … serving_after_parent_death=true on all four platforms when Pdeathsig is off.
    • On a live Mac (seen with ps only, not touched), an orphaned wallet with ppid 1 was running beside the daemon's current wallet. Both had byte-identical argv: the same --db, --socket and --cap-state.
  2. Log growth. Every IPC call logged conn open from= + conn closed to stderr, which the supervisor wires into the daemon log: 96.5 bytes per call, 289,417 bytes over 3,000 calls.
  3. Unlinking a live socket (found while fixing 1). UnixListener.Close unlinks the socket path by name. If an old wallet shuts down after a replacement has already bound the same path, the old one deletes the new one's socket, and the live wallet keeps running but can't be reached. This can happen when the respawned daemon starts a new wallet before the orphan notices its parent died: daemons before app-store v1.0.3 have no reapStale. It can also happen when a SIGTERM reaches an old instance late.

Fix

  • watchParent (cmd/wallet/parentwatch.go):
    • Captures the parent pid at startup and checks getppid once a second.
    • When the parent changes, it cancels the run context, so the wallet shuts down through its normal path: stop accepting, drain, close the ledger, remove the socket.
    • A wallet started directly by init/launchd (ppid 1) is not watched.
    • --exit-with-parent=false turns it off.
  • Per-connection logging now sits behind --log-conns, off by default. Serve errors are still logged.
  • ownSocket (cmd/wallet/socketguard.go): the listener remembers the socket file it bound. On Close it leaves the path alone (SetUnlinkOnClose(false)) once the path refers to a different socket.

Tests

  • TestWalletExitsWhenParentDies runs the real main() under a stand-in daemon that uses its own process group and no Pdeathsig, as the supervisor does on macOS. The test SIGKILLs the stand-in, then requires the wallet to exit on its own, log graceful shutdown complete, and remove its socket.
    • With --exit-with-parent defaulting to false, it fails: wallet pid 8981 still running 10.028s after its parent was killed (orphaned).
    • With the fix, the wallet exits after 1.5–2.0 s on macOS arm64, and on linux/arm64 and linux/amd64 (golang:1.25.13-bookworm) with and without --init.
  • TestOwnedListenerCloseKeepsReplacementSocket fails without the ownership check (replacement's socket was removed by the old instance's Close) and passes with it. Two more tests cover normal unlink and an externally removed path.
  • TestServeLogsConnectionsOnlyWhenAsked: with the default, 25 calls give 0 per-connection lines. With --log-conns, they give 50.
  • go vet ./... and go test -race ./... pass on macOS arm64 and linux/arm64. On linux/amd64 the new tests pass.
  • Pre-existing flake: TestRunSmoke on linux/amd64 emulated through Rosetta, under full-suite load. Its 3 s socket deadline is shorter than the observed 4.3 s startup. origin/main fails the same way (run 2 of 2: --- FAIL: TestRunSmoke (5.70s) … did not become reachable). This PR doesn't touch that path.

Harness results (built from this branch)

Signed with a throwaway key; N=200 wallet.evm.chains calls plus the SIGTERM, SIGKILL-stop and orphan phases.

platform calls fd / RSS SIGTERM orphan (no Pdeathsig)
darwin/arm64 200/200 fd 10→10, no RSS trend exit 0 in 4 ms, socket removed SELF-EXIT after 1019 ms (was ORPHAN)
darwin/amd64 (Rosetta) 200/200 fd 10→10 exit 0 in 13 ms SELF-EXIT after 931 ms (was ORPHAN)
linux/arm64 200/200 fd 10→11 (idle 10) exit 0 in 70 ms SELF-EXIT after 1031 ms; Pdeathsig on: died in 11 ms
linux/amd64 200/200 fd 19→19 exit 0 in 87 ms SELF-EXIT after 743 ms; Pdeathsig on: died in 1 ms
  • Log growth, 3,000 calls on darwin/arm64: 1,494 stderr bytes in total and 0 per-call lines. The catalogue's 0.3.3 wrote 289,417 bytes (6,000 lines).
  • Soak, 12,000 calls: fd stays at 10, RSS 24.6 MB → 21.9 MB (flat).

Not in this PR

  • The supervisor still SIGKILLs apps on stop, so the wallet's graceful shutdown never runs on a deliberate stop, upgrade or uninstall. That is fixed on the supervisor side in pilot-protocol/app-store, stacked on build(deps): bump modernc.org/sqlite from 1.57.0 to 1.59.0 #39.
  • wallet.hookPreSendMessage / wallet.hookPostRecvMessage are declared in manifest.json (extends + exposes) but never registered, so IPC returns method not found. Nothing calls them today: neither web4 nor the supervisor wires pkg/extend.
  • The affiliates entry still has the placeholder key ed25519:BBBB….
  • The repo root still contains a committed 15.6 MB darwin/arm64 wallet binary.

🤖 Generated with Claude Code

teovl and others added 3 commits September 24, 2026 11:23
…every IPC connection

Orphan leak: the app-store supervisor starts the wallet in its own process
group. macOS has no parent-death signal, so whenever the daemon exits
without running its shutdown path (watchdog os.Exit, SIGKILL, crash,
launchd restart) the wallet is reparented to launchd and keeps serving its
socket with the sqlite ledger and cap-state log open. Measured with the
apps-sweep harness (mini-supervisor SIGKILLed, app watched 6 s):

  before: ORPHAN, still answering IPC, on darwin/arm64, darwin/amd64
          (Rosetta), and linux/amd64 + linux/arm64 without Pdeathsig
  after:  SELF-EXIT in 915-1025 ms on all four, socket unlinked,
          "graceful shutdown complete" logged

watchParent polls getppid once a second and cancels the run context when
the parent changes, so shutdown goes through the normal SIGTERM path.
A wallet started by init/launchd directly (ppid 1) is not watched;
--exit-with-parent=false opts out for standalone use.

Log growth: every IPC call wrote "conn open" + "conn closed" (96.5 bytes
per call, 289,509 bytes over 3,000 calls) to stderr, which the supervisor
wires straight into the daemon's log. Now behind --log-conns (default
off); serve errors are still logged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Go's UnixListener.Close unlinks the socket path by name. When a wallet
shuts down after a replacement has already taken over the same path, that
Close deletes the replacement's socket and leaves the live wallet running
but unreachable. This happens when:

- the wallet exits because its parent died (previous commit) and the
  respawned daemon has already started a new wallet, which removed the
  stale socket file and bound a fresh one. Daemons before app-store
  v1.0.3 have no reapStale to stop the old copy first;
- a SIGTERM reaches an old instance after its successor is up.

The listener now records the socket file it bound, and Close keeps the
path when it no longer refers to that socket (SetUnlinkOnClose(false)).
Normal shutdown still removes the wallet's own socket.

TestOwnedListenerCloseKeepsReplacementSocket fails without the check
("replacement's socket was removed by the old instance's Close") and
passes with it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
TestWalletExitsWhenParentDies runs the real main() under a stand-in
daemon (own process group, no Pdeathsig, as the supervisor sets it up on
macOS), SIGKILLs the stand-in, and requires the wallet to exit on its
own, log "graceful shutdown complete" and remove its socket. With
--exit-with-parent defaulting to false it fails: "wallet pid N still
running 10.028s after its parent was killed (orphaned)". With the fix
the wallet exits within about 2 s (macOS, linux/arm64, linux/amd64).

TestServeLogsConnectionsOnlyWhenAsked checks that serve writes no
per-connection lines by default and one open/close pair per call with
--log-conns.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.22034% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cmd/wallet/socketguard.go 75.00% 2 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

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.

2 participants