Skip to content

fix(tunnel): TunnelManager self-heals dead tunnels, stops retrying unsupported devices, tested udid filter - #809

Open
danielpaulus wants to merge 3 commits into
mainfrom
fix/issue-765-tunnel-manager-robustness
Open

fix(tunnel): TunnelManager self-heals dead tunnels, stops retrying unsupported devices, tested udid filter#809
danielpaulus wants to merge 3 commits into
mainfrom
fix/issue-765-tunnel-manager-robustness

Conversation

@danielpaulus

Copy link
Copy Markdown
Owner

Stacked PR — based on #808. This branch includes #808's commit (the shared dial-timeout helper its liveness probe reuses) and should merge after #808; once #808 lands, this diff reduces to the TunnelManager changes only.

Problem

Three long-standing tunnel-agent problems, all in TunnelManager.UpdateTunnels:

  1. TunnelManager agent doesn't auto-recover a dead-but-still-present tunnel (reboot test only covers explicit refresh) #765 — no self-heal for dead-but-still-present tunnels. A tunnel that dies without a usbmux disconnect (quick reboot the mux never observes, transport death while the TUN route stays up) remains a stale record forever: the create path skips devices that already have a record, and teardown only fires for devices that vanish from ListDevices(). Every consumer then dials the dead endpoint (135s each before fix(tunnel): bound tunnel/RSD TCP dials with a 15s timeout #808/Kernel tunnel: device-unreachable mid-run causes 135s no-timeout RSD dials → e2e job timeout #764). Observed live on the Linux CI runner across two runs.
  2. Using ios <17 and >=17 together is generating a huge amount of logging #523 — sub-iOS17 devices spam warnings forever. With mixed iOS <17 / 17+ fleets, every update cycle logs failed to start tunnel ... unsupported iOS version for each old device, escalating with device count.
  3. How to tunnel start for multiple iOS devices? #607 / Is it possible to remove the output of the tunnel start command or to start or stop the tunnel by udid with ios commands? #479 — per-device tunnel management. Users asked for ios tunnel start --udid to manage a single device's tunnel. The TunnelManager.udidFilter plumbing exists (NewTunnelManagerForDevice, wired from cmd_tunnel.go with help text in main.go) but had zero unit-test coverage guarding it.

Root cause

UpdateTunnels had exactly two behaviors: create-if-absent and teardown-if-vanished. There was no liveness notion for existing records, and no classification of permanent vs transient start failures — everything took the retry/backoff path.

Fix

  1. Liveness probing (TunnelManager agent doesn't auto-recover a dead-but-still-present tunnel (reboot test only covers explicit refresh) #765). Existing records of still-connected devices are probed behind a new tunnelProber seam, at most once per 30s per device (defaultProbeInterval). The default rsdProber is a 5s-timeout TCP dial of the tunnel's RSD endpoint using fix(tunnel): bound tunnel/RSD TCP dials with a 15s timeout #808's ios.DialTunnelTCPWithTimeout; userspace tunnels are skipped (the localhost forwarder accepts connects regardless of device state, so a dial proves nothing). On probe failure the record is torn down via the per-device stop machinery from Add per-device tunnel stop and refresh #738 and the create loop rebuilds it in the same cycle. Fresh tunnels defer their first probe by a full interval.
  2. Permanent-failure classification (Using ios <17 and >=17 together is generating a huge amount of logging #523). manualPairingTunnelStart wraps its version-based failures (iOS <17, and userspace on 17.0–17.3) in a new ErrUnsupportedVersion sentinel. UpdateTunnels logs those once at info and marks the udid permanently skipped for the process lifetime (restart the agent after upgrading a device's OS). Transient errors keep the existing failedDevice exponential backoff untouched.
  3. udid filter (How to tunnel start for multiple iOS devices? #607/Is it possible to remove the output of the tunnel start command or to start or stop the tunnel by udid with ios commands? #479). The --udid filter path is now locked in by unit tests: only the matching device is attempted/tunneled, empty filter keeps managing all devices. No behavior change needed — the plumbing and ios tunnel start help text are already on main; this closes the issues with the behavior now guaranteed by tests.

Also adds a productVersion seam (defaults to ios.GetProductVersion) so the start path is unit-testable without a device.

Options considered

  • Probe = short-timeout RSD TCP dial (chosen) vs a full RSD/RemoteXPC handshake: the incident signature is a dead route (SYN blackhole), which a plain connect detects; a handshake adds protocol weight and failure modes for little extra signal. The tunnelProber interface leaves room to upgrade later.
  • Probe cadence: every-cycle probing (1s) would dial each device every second and stall the loop up to 5s per dead device; a 30s per-device interval bounds overhead while detecting death well under a minute.
  • Using ios <17 and >=17 together is generating a huge amount of logging #523 blacklist on any first failure (patch suggested in the issue thread): too aggressive — a transient pairing hiccup would permanently disable a healthy device. Classifying only version errors as permanent keeps backoff for everything genuinely retryable.
  • Pruning the unsupported set on disconnect (device might be OS-upgraded): rejected for now — issues ask for silence; process-lifetime skip is the simplest contract and an agent restart re-detects.

Test plan

New device-free tests in ios/tunnel/tunnel_manager_robustness_test.go (fake starter/lister/prober/version seams), each failing without its change:

  • TestUpdateTunnelsRebuildsDeadTunnel — device still listed, prober says dead → old record closed exactly once, tunnel restarted, new record served.
  • TestUpdateTunnelsKeepsHealthyTunnel — healthy record probed but untouched.
  • TestUpdateTunnelsDoesNotProbeDisconnectedDevice — vanished devices stay with the disconnect teardown.
  • TestShouldProbeRespectsInterval, TestRsdProberSkipsUserspaceTunnels.
  • TestManualPairingTunnelStartClassifiesUnsupportedVersions — iOS 16.x and userspace-17.2 both classify as ErrUnsupportedVersion.
  • TestUpdateTunnelsSkipsUnsupportedDevicePermanently — one attempt across three cycles, no backoff entry; TestUpdateTunnelsKeepsBackoffForTransientFailures — transient errors still back off, never marked unsupported.
  • TestUpdateTunnelsUdidFilter — filter attempts only the matching device; empty filter manages all.

go build ./..., go test ./... and go test -race ./ios/tunnel/ green; gofmt -l clean. Real-device e2e via /test-devices.

Fixes #765, fixes #523, fixes #607, fixes #479

🤖 Generated with Claude Code

https://claude.ai/code/session_01J8eMENxJ1nec9CeHp4tjWk

danielpaulus and others added 2 commits August 5, 2026 11:01
A dial to a dead-but-still-routed kernel tunnel address (device rebooted
or hung while the host-side TUN interface and route stayed up) inherited
the kernel's TCP SYN timeout (~135s on Linux), burning 135s per operation
and timing out entire e2e runs before anything could recover.

Introduce ios.DialTunnelTCP / DialTunnelTCPWithTimeout, a shared dial
helper using net.Dialer{Timeout: 15s}, and use it at all three tunnel
dial sites:

- ios/connect.go connectTUN (kernel TUN RSD dial — the 135s case)
- ios/connect.go ConnectTUNDevice userspace forwarder dial
- ios/tunnel/tunnel_tcp.go TLS-PSK tunnel listener dial (iOS 18.2+)

Timeout errors are wrapped in the exported ios.ErrDialTimeout sentinel so
staleness logic can distinguish "device unreachable over a live route"
from refused/unreachable failures.

Fixes #764

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8eMENxJ1nec9CeHp4tjWk
…supported devices

Three robustness fixes to the tunnel agent's UpdateTunnels loop:

1. Liveness probing (#765): a tunnel that dies without a usbmux
   disconnect (quick reboot, transport death while the TUN route stays
   up) used to remain a stale record forever, because UpdateTunnels only
   creates tunnels for devices without a record and only tears down
   records of vanished devices. Existing records of still-connected
   devices are now probed every 30s behind a tunnelProber seam; the
   default prober is a short-timeout (5s) TCP dial of the RSD endpoint
   reusing the #764 dial helper. A failed probe tears the record down via
   the per-device stop machinery from #738 so the create path rebuilds it
   in the same cycle.

2. Unsupported-iOS-version classification (#523): sub-iOS17 devices used
   to produce a "failed to start tunnel ... unsupported iOS version"
   warning on every retry, forever, when mixed with iOS 17+ devices.
   manualPairingTunnelStart now wraps version-based failures in the
   ErrUnsupportedVersion sentinel; UpdateTunnels logs those once at info
   and permanently skips the device (restart the agent after an OS
   update). Transient errors keep the existing failedDevice backoff.

3. udid filter coverage (#607, #479): `ios tunnel start --udid=<udid>`
   restricts the agent to that one device via TunnelManager.udidFilter;
   add unit tests proving only the matching device is attempted and that
   an empty filter manages all devices.

Also adds a productVersion seam (defaulting to ios.GetProductVersion) so
the manager's start path is unit-testable without a device.

Fixes #765, fixes #523, fixes #607, fixes #479

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8eMENxJ1nec9CeHp4tjWk
@danielpaulus

Copy link
Copy Markdown
Owner Author

/test-devices

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🧪 Running real-device tests on PR #809run.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

❌ Real-device tests failed — see run.

Address two robustness defects in the TunnelManager self-heal path (#765):

- Probes were run serially inside UpdateTunnels; each dead-but-still-routed
  tunnel blackholes the SYN for the full probeDialTimeout, so N dead tunnels
  could stall tunnel creation for newly connected devices by up to
  probeDialTimeout*N. Probes now run concurrently, bounded by
  maxConcurrentProbes, so a batch of dead tunnels can't serialize into a
  multi-second stall.

- A single failed probe immediately tore down an otherwise healthy tunnel. A
  tunnel is now only rebuilt after probeFailureThreshold (2) consecutive
  failures, and the counter resets on any successful probe, so a momentary
  route hiccup or load spike no longer destroys a healthy tunnel.

Also prune unsupportedDevices and probeFailures for disconnected devices so
these maps stay bounded by the current device count instead of every udid
ever seen, and so a device that was unsupported, upgraded to a tunnel-capable
iOS, and reconnected is retried instead of skipped for the process lifetime.

Tests cover the two-failure threshold, transient-failure absorption, and
unsupported-on-disconnect pruning; all pass under -race.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8eMENxJ1nec9CeHp4tjWk
@danielpaulus

Copy link
Copy Markdown
Owner Author

Adversarial review + fixes (pushed to this branch as `0352b30`).

Verdict: logic was sound (probe lives outside the mux, map access is consistently mutex-guarded, `%w` classification and `errors.Is` work, teardown/`lastProbe` cleanup was complete, no orphaned utun). Fixed two confirmed defects and one bounded-growth issue in the self-heal path:

  1. Probe starvation (HIGH). The liveness-probe loop ran serially inside `UpdateTunnels`; each dead-but-still-routed tunnel blackholes the SYN for the full `probeDialTimeout` (5s), so N dead tunnels could delay tunnel creation for newly connected devices by up to 5s×N. Probes now run concurrently, bounded by `maxConcurrentProbes` (8) via a semaphore + WaitGroup, collected under the mux.

  2. False-positive teardown (MEDIUM). A single failed probe immediately tore down a healthy tunnel. Teardown now requires `probeFailureThreshold` (2) consecutive failures, reset on any success (new `probeFailures` map, mutex-guarded), so a momentary route hiccup/load spike can't destroy a healthy tunnel. Trade-off: a persistently dead tunnel is now rebuilt in ~2 probe intervals (~60s) instead of ~30s — documented on `defaultProbeInterval`.

  3. Bounded-growth / stale-after-upgrade (MEDIUM). `unsupportedDevices` (and the new `probeFailures`) are now pruned for disconnected devices in the disconnect teardown, so they stay bounded by current device count, and a device that was unsupported → upgraded to a tunnel-capable iOS → reconnected is retried instead of skipped for the process lifetime.

New tests: two-failure threshold, transient-failure absorption (counter reset), unsupported-on-disconnect pruning + reconnect-retry. Made `fakeProber` thread-safe (probes are concurrent now).

Dismissed: the exact-`17.4.0` userspace-TUN boundary (`GreaterThan(17.4.0)`) is pre-existing and unchanged by this PR (only the error string was reworded). No transient-version misclassification risk: `GetProductVersion` either returns the real version or errors (→ transient-failure backoff, not the permanent set).

Verification (both branches): `go build ./...`, `go test ./...`, `go test -race ./ios/tunnel/` all green; `gofmt -l` clean.

Note: #808 (base of this stack) was reviewed and confirmed clean — no changes needed there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment