From fda20a70829fc0dd0294bb898ff0f16af6699bd1 Mon Sep 17 00:00:00 2001 From: Daniel Paulus Date: Wed, 5 Aug 2026 11:01:59 -0400 Subject: [PATCH 1/3] fix(tunnel): bound tunnel/RSD TCP dials with a 15s timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01J8eMENxJ1nec9CeHp4tjWk --- ios/connect.go | 45 +++++++++++++++++++++++++++++++++------- ios/connect_test.go | 24 +++++++++++++++++++++ ios/tunnel/tunnel_tcp.go | 3 +-- 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/ios/connect.go b/ios/connect.go index dedf6970..27fb033f 100755 --- a/ios/connect.go +++ b/ios/connect.go @@ -296,6 +296,42 @@ func initializeXpcConnection(h *http.HttpConnection) error { return nil } +// TunnelDialTimeout bounds TCP connects to tunnel/RSD endpoints. Without an +// explicit timeout, a dial to a dead-but-still-routed tunnel address (device +// rebooted or hung while the host-side TUN interface and route stayed up) +// blocks for the kernel's TCP SYN timeout (~135s on Linux) per operation. 15s +// is far above any healthy tunnel connect (sub-second) while still failing +// fast enough for staleness handling to react. +const TunnelDialTimeout = 15 * time.Second + +// ErrDialTimeout marks a tunnel/RSD TCP connect that exceeded go-ios' dial +// timeout rather than failing outright. Callers can use errors.Is to treat the +// endpoint as stale: the route existed but the device never answered, which is +// the signature of a dead tunnel whose interface lingers. +var ErrDialTimeout = errors.New("dial timed out") + +// DialTunnelTCP connects to a tunnel/RSD TCP endpoint (address in the form +// accepted by net.Dial, e.g. "[fd00::1]:1234") with TunnelDialTimeout. +func DialTunnelTCP(address string) (*net.TCPConn, error) { + return DialTunnelTCPWithTimeout(address, TunnelDialTimeout) +} + +// DialTunnelTCPWithTimeout is DialTunnelTCP with a caller-chosen timeout. +// Timeout errors are wrapped in ErrDialTimeout so they stay distinguishable +// from refused/unreachable errors. +func DialTunnelTCPWithTimeout(address string, timeout time.Duration) (*net.TCPConn, error) { + d := net.Dialer{Timeout: timeout} + conn, err := d.Dial("tcp", address) + if err != nil { + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + return nil, fmt.Errorf("%w after %v: %w", ErrDialTimeout, timeout, err) + } + return nil, err + } + return conn.(*net.TCPConn), nil +} + // ConnectTUNDevice creates a *net.TCPConn to the device at the given address and port. // If the device is a userspaceTUN device provided by go-ios agent, it will connect to this // automatically. Otherwise it will try a operating system level TUN device. @@ -310,8 +346,7 @@ func ConnectTUNDevice(remoteIp string, port int, d DeviceEntry) (*net.TCPConn, e return connectTUN(remoteIp, port) } - addr, _ := net.ResolveTCPAddr("tcp4", fmt.Sprintf("%s:%d", d.UserspaceTUNHost, d.UserspaceTUNPort)) - conn, err := net.DialTCP("tcp", nil, addr) + conn, err := DialTunnelTCP(fmt.Sprintf("%s:%d", d.UserspaceTUNHost, d.UserspaceTUNPort)) if err != nil { return nil, fmt.Errorf("ConnectUserSpaceTunnel: failed to dial: %w", err) } @@ -332,11 +367,7 @@ func ConnectTUNDevice(remoteIp string, port int, d DeviceEntry) (*net.TCPConn, e // connect to a operating system level TUN device func connectTUN(address string, port int) (*net.TCPConn, error) { - addr, err := net.ResolveTCPAddr("tcp6", fmt.Sprintf("[%s]:%d", address, port)) - if err != nil { - return nil, fmt.Errorf("ConnectToHttp2WithAddr: failed to resolve address: %w", err) - } - conn, err := net.DialTCP("tcp", nil, addr) + conn, err := DialTunnelTCP(fmt.Sprintf("[%s]:%d", address, port)) if err != nil { return nil, fmt.Errorf("ConnectToHttp2WithAddr: failed to dial: %w", err) } diff --git a/ios/connect_test.go b/ios/connect_test.go index 7edb2b42..c01f6f30 100644 --- a/ios/connect_test.go +++ b/ios/connect_test.go @@ -1,7 +1,9 @@ package ios import ( + "errors" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -57,3 +59,25 @@ func TestConnectToServiceFailsFastWhenServiceIsMissingFromRsd(t *testing.T) { assert.Contains(t, err.Error(), "not available in RSD") }) } + +// A dial to a dead-but-still-routed tunnel address (device rebooted or hung +// while the host-side TUN route stayed up) must fail after go-ios' own dial +// timeout, not the kernel's ~135s SYN timeout, and the error must be +// classifiable as a timeout so staleness handling can key off it (issue #764). +// 192.0.2.1 (TEST-NET-1, RFC 5737) is reserved and never answers, mimicking the +// blackholed-SYN behavior of a dead tunnel. +func TestDialTunnelTCPWithTimeoutFailsFast(t *testing.T) { + const timeout = 250 * time.Millisecond + start := time.Now() + _, err := DialTunnelTCPWithTimeout("192.0.2.1:54321", timeout) + elapsed := time.Since(start) + + require.Error(t, err) + assert.Less(t, elapsed, 5*time.Second, "dial must fail well under the kernel SYN timeout") + if !errors.Is(err, ErrDialTimeout) { + // Some environments answer TEST-NET-1 with a fast ICMP unreachable or a + // sandbox denial instead of blackholing the SYN; then this run cannot + // exercise the timeout classification, only the fast failure above. + t.Skipf("environment did not blackhole 192.0.2.1, got: %v", err) + } +} diff --git a/ios/tunnel/tunnel_tcp.go b/ios/tunnel/tunnel_tcp.go index c4b02169..c4fabdab 100644 --- a/ios/tunnel/tunnel_tcp.go +++ b/ios/tunnel/tunnel_tcp.go @@ -3,7 +3,6 @@ package tunnel import ( "context" "fmt" - "net" "github.com/danielpaulus/go-ios/ios" "github.com/danielpaulus/go-ios/ios/http" @@ -55,7 +54,7 @@ func ManualPairAndConnectToTunnelTCP(ctx context.Context, device ios.DeviceEntry } tunnelAddr := fmt.Sprintf("[%s]:%d", addr, tunnelPort) - tcpConn, err := net.Dial("tcp", tunnelAddr) + tcpConn, err := ios.DialTunnelTCP(tunnelAddr) if err != nil { return Tunnel{}, fmt.Errorf("ManualPairAndConnectToTunnelTCP: failed to dial tunnel port %s: %w", tunnelAddr, err) } From c4bca354541657bbfc4d3410ddb0a110069a7137 Mon Sep 17 00:00:00 2001 From: Daniel Paulus Date: Wed, 5 Aug 2026 11:07:29 -0400 Subject: [PATCH 2/3] fix(tunnel): TunnelManager self-heals dead tunnels, stops retrying unsupported 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=` 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 Claude-Session: https://claude.ai/code/session_01J8eMENxJ1nec9CeHp4tjWk --- ios/tunnel/tunnel_api.go | 122 +++++++- ios/tunnel/tunnel_manager_robustness_test.go | 276 +++++++++++++++++++ 2 files changed, 394 insertions(+), 4 deletions(-) create mode 100644 ios/tunnel/tunnel_manager_robustness_test.go diff --git a/ios/tunnel/tunnel_api.go b/ios/tunnel/tunnel_api.go index 4dfde478..834da1c6 100644 --- a/ios/tunnel/tunnel_api.go +++ b/ios/tunnel/tunnel_api.go @@ -27,6 +27,11 @@ var netClient = &http.Client{ var ErrTunnelNotFound = errors.New("tunnel not found") +// ErrUnsupportedVersion is returned when a device's iOS version can never use +// go-ios managed tunnels (tunnels only exist for iOS 17+). The TunnelManager +// treats it as permanent and stops retrying such devices. +var ErrUnsupportedVersion = errors.New("unsupported iOS version") + func CloseAgent() error { _, err := netClient.Get(fmt.Sprintf("http://%s:%d/shutdown", ios.HttpApiHost(), ios.HttpApiPort())) if err != nil { @@ -322,17 +327,29 @@ type failedDevice struct { type TunnelManager struct { ts tunnelStarter dl deviceLister + pr tunnelProber pm PairRecordManager mux sync.Mutex tunnels map[string]Tunnel // failedDevices tracks devices whose tunnel start failed (keyed by udid) so // UpdateTunnels can back off before retrying them. - failedDevices map[string]failedDevice + failedDevices map[string]failedDevice + // unsupportedDevices tracks devices whose iOS version can never tunnel + // (ErrUnsupportedVersion). They are skipped for the rest of the process so + // the same warning is not logged every update cycle. + unsupportedDevices map[string]bool + // lastProbe tracks when each tunnel was last liveness-probed so probes run + // at most once per probeInterval. + lastProbe map[string]time.Time + probeInterval time.Duration startTunnelTimeout time.Duration firstUpdateCompleted bool userspaceTUN bool closeOnce sync.Once portOffset int + // productVersion resolves a device's iOS version; a seam so tests can run + // without a device. Defaults to ios.GetProductVersion. + productVersion func(ios.DeviceEntry) (*semver.Version, error) // udidFilter, when non-empty, restricts the manager to a single device so // you can run one isolated tunnel agent per device. udidFilter string @@ -364,14 +381,19 @@ func newTunnelManager(pm PairRecordManager, userspaceTUN bool, udidFilter string return &TunnelManager{ ts: manualPairingTunnelStart{}, dl: deviceList{}, + pr: rsdProber{}, pm: pm, tunnels: map[string]Tunnel{}, failedDevices: map[string]failedDevice{}, + unsupportedDevices: map[string]bool{}, + lastProbe: map[string]time.Time{}, + probeInterval: defaultProbeInterval, startTunnelTimeout: 10 * time.Second, userspaceTUN: userspaceTUN, udidFilter: udidFilter, basePort: basePort, portOffset: 1, + productVersion: ios.GetProductVersion, } } @@ -410,6 +432,8 @@ func (m *TunnelManager) UpdateTunnels(ctx context.Context) error { maps.Copy(localTunnels, m.tunnels) localFailed := map[string]failedDevice{} maps.Copy(localFailed, m.failedDevices) + localUnsupported := map[string]bool{} + maps.Copy(localUnsupported, m.unsupportedDevices) m.mux.Unlock() devices, err := m.dl.ListDevices() @@ -433,11 +457,32 @@ func (m *TunnelManager) UpdateTunnels(ctx context.Context) error { currentUDIDs[d.Properties.SerialNumber] = true } + // Liveness-probe existing tunnel records of still-connected devices: a + // tunnel can die without a usbmux disconnect (quick reboot, transport + // death), leaving a stale record that would otherwise be served forever. A + // failed probe tears the record down here so the create loop below rebuilds + // it in the same cycle. Records of vanished devices are handled by the + // disconnect teardown at the end. + now := time.Now() + for udid, tun := range localTunnels { + if !currentUDIDs[udid] || !m.shouldProbe(udid, now) { + continue + } + if err := m.pr.Probe(tun); err != nil { + golog.Warn("tunnel failed liveness probe, restarting it", "module", logModule, "udid", udid, "error", err) + _ = m.stopTunnel(tun) + delete(localTunnels, udid) + } + } + for _, d := range devices.DeviceList { udid := d.Properties.SerialNumber if m.udidFilter != "" && udid != m.udidFilter { continue } + if localUnsupported[udid] { + continue + } if _, exists := localTunnels[udid]; exists { continue } @@ -456,6 +501,16 @@ func (m *TunnelManager) UpdateTunnels(ctx context.Context) error { } t, err := m.startTunnel(ctx, d) if err != nil { + if errors.Is(err, ErrUnsupportedVersion) { + // The device can never tunnel on its current iOS version, so + // retrying would only repeat the same warning every cycle. Log + // once at info and skip the device for the rest of the process. + golog.Info("device iOS version does not support tunnels, skipping it from now on", "module", logModule, "udid", udid, "error", err) + m.mux.Lock() + m.unsupportedDevices[udid] = true + m.mux.Unlock() + continue + } golog.Warn("failed to start tunnel", "module", logModule, "udid", udid, "error", err) m.mux.Lock() m.failedDevices[udid] = failedDevice{lastAttempt: time.Now(), failCount: m.failedDevices[udid].failCount + 1} @@ -464,6 +519,10 @@ func (m *TunnelManager) UpdateTunnels(ctx context.Context) error { } m.mux.Lock() delete(m.failedDevices, udid) + if m.lastProbe != nil { + // A fresh tunnel is known-alive; defer its first probe by a full interval. + m.lastProbe[udid] = time.Now() + } localTunnels[udid] = t m.tunnels[udid] = t m.mux.Unlock() @@ -500,6 +559,21 @@ func shouldSkipDevice(d ios.DeviceEntry, failed map[string]failedDevice, now tim return false } +// shouldProbe reports whether the tunnel for udid is due for a liveness probe +// and, if so, records the attempt so probes run at most once per probeInterval. +func (m *TunnelManager) shouldProbe(udid string, now time.Time) bool { + if m.pr == nil || m.probeInterval <= 0 { + return false + } + m.mux.Lock() + defer m.mux.Unlock() + if last, ok := m.lastProbe[udid]; ok && now.Sub(last) < m.probeInterval { + return false + } + m.lastProbe[udid] = now + return true +} + // failedDeviceBackoff returns how long to wait before retrying a device after // failCount consecutive failures: 30s, 60s, 120s, 240s, capped at 5 minutes. func failedDeviceBackoff(failCount int) time.Duration { @@ -523,6 +597,7 @@ func (m *TunnelManager) RemoveTunnel(ctx context.Context, serialNumber string) e if exists { delete(m.tunnels, serialNumber) } + delete(m.lastProbe, serialNumber) m.mux.Unlock() if !exists { @@ -536,6 +611,7 @@ func (m *TunnelManager) stopTunnel(t Tunnel) error { m.mux.Lock() golog.Info("stopping tunnel", "module", logModule, "udid", t.Udid) delete(m.tunnels, t.Udid) + delete(m.lastProbe, t.Udid) m.mux.Unlock() return t.Close() @@ -545,7 +621,11 @@ func (m *TunnelManager) startTunnel(ctx context.Context, device ios.DeviceEntry) golog.Info("start tunnel", "module", logModule, "udid", device.Properties.SerialNumber) startTunnelCtx, cancel := context.WithTimeout(ctx, m.startTunnelTimeout) defer cancel() - version, err := ios.GetProductVersion(device) + versionOf := m.productVersion + if versionOf == nil { + versionOf = ios.GetProductVersion + } + version, err := versionOf(device) if err != nil { return Tunnel{}, fmt.Errorf("startTunnel: failed to get device version: %w", err) } @@ -586,6 +666,40 @@ type deviceLister interface { ListDevices() (ios.DeviceList, error) } +// tunnelProber checks whether an existing tunnel record still reaches the +// device, so the TunnelManager can tear down and rebuild dead tunnels. +type tunnelProber interface { + Probe(t Tunnel) error +} + +// defaultProbeInterval is how often the TunnelManager liveness-probes each +// tunnel record. Combined with the probe's short dial timeout, a dead tunnel +// is detected and rebuilt well within a minute instead of never. +const defaultProbeInterval = 30 * time.Second + +// probeDialTimeout bounds the liveness probe's TCP connect. A healthy tunnel +// answers in milliseconds; a dead-but-still-routed one swallows the SYN, so a +// short timeout is enough to tell them apart without stalling UpdateTunnels. +const probeDialTimeout = 5 * time.Second + +// rsdProber is the default tunnelProber: a short-timeout TCP dial of the +// tunnel's RSD endpoint over the kernel TUN route. +type rsdProber struct { +} + +func (rsdProber) Probe(t Tunnel) error { + if t.UserspaceTUN { + // The userspace forwarder listens on localhost and accepts connects no + // matter what state the device is in, so a dial proves nothing here. + return nil + } + conn, err := ios.DialTunnelTCPWithTimeout(fmt.Sprintf("[%s]:%d", t.Address, t.RsdPort), probeDialTimeout) + if err != nil { + return err + } + return conn.Close() +} + type manualPairingTunnelStart struct { } @@ -602,11 +716,11 @@ func (m manualPairingTunnelStart) StartTunnel(ctx context.Context, device ios.De } if version.Major() >= 17 { if userspaceTUN { - return Tunnel{}, errors.New("manualPairingTunnelStart: userspaceTUN not supported for iOS >=17 and < 17.4") + return Tunnel{}, fmt.Errorf("manualPairingTunnelStart: userspaceTUN not supported for iOS >=17 and < 17.4: %w %s", ErrUnsupportedVersion, version.String()) } return ManualPairAndConnectToTunnel(ctx, device, p) } - return Tunnel{}, fmt.Errorf("manualPairingTunnelStart: unsupported iOS version %s", version.String()) + return Tunnel{}, fmt.Errorf("manualPairingTunnelStart: %w %s", ErrUnsupportedVersion, version.String()) } type deviceList struct { diff --git a/ios/tunnel/tunnel_manager_robustness_test.go b/ios/tunnel/tunnel_manager_robustness_test.go new file mode 100644 index 00000000..95ef9a49 --- /dev/null +++ b/ios/tunnel/tunnel_manager_robustness_test.go @@ -0,0 +1,276 @@ +package tunnel + +// Unit tests for the TunnelManager robustness behaviors: +// - liveness probing of existing tunnel records (issue #765) +// - permanent skip of devices with unsupported iOS versions (issue #523) +// - the udid filter restricting a manager to one device (issues #607, #479) +// All device-free: the tunnelStarter/deviceLister/tunnelProber/productVersion +// seams are faked. + +import ( + "context" + "errors" + "fmt" + "sync" + "testing" + "time" + + "github.com/Masterminds/semver" + "github.com/danielpaulus/go-ios/ios" +) + +type fakeStarter struct { + mu sync.Mutex + calls []string + err error + // rsdPort is assigned to created tunnels so tests can tell rebuilds apart. + rsdPort int +} + +func (f *fakeStarter) StartTunnel(ctx context.Context, device ios.DeviceEntry, p PairRecordManager, version *semver.Version, userspaceTUN bool) (Tunnel, error) { + f.mu.Lock() + defer f.mu.Unlock() + udid := device.Properties.SerialNumber + f.calls = append(f.calls, udid) + if f.err != nil { + return Tunnel{}, f.err + } + return Tunnel{Udid: udid, Address: "fd00::1", RsdPort: f.rsdPort, closer: func() error { return nil }}, nil +} + +func (f *fakeStarter) callsFor(udid string) int { + f.mu.Lock() + defer f.mu.Unlock() + n := 0 + for _, c := range f.calls { + if c == udid { + n++ + } + } + return n +} + +type fakeProber struct { + err error + probed []string +} + +func (f *fakeProber) Probe(t Tunnel) error { + f.probed = append(f.probed, t.Udid) + return f.err +} + +// robustnessManager builds a fully faked TunnelManager that probes on every +// UpdateTunnels cycle and resolves every device to iOS 18.0.0. +func robustnessManager(ts tunnelStarter, pr tunnelProber, entries ...ios.DeviceEntry) *TunnelManager { + return &TunnelManager{ + ts: ts, + dl: stubDeviceLister{list: ios.DeviceList{DeviceList: entries}}, + pr: pr, + tunnels: map[string]Tunnel{}, + failedDevices: map[string]failedDevice{}, + unsupportedDevices: map[string]bool{}, + lastProbe: map[string]time.Time{}, + probeInterval: time.Nanosecond, + startTunnelTimeout: time.Second, + productVersion: func(ios.DeviceEntry) (*semver.Version, error) { + return semver.MustParse("18.0.0"), nil + }, + } +} + +// A tunnel record whose device is still connected but whose probe fails must be +// torn down and rebuilt in the same update cycle — the agent-side self-heal +// from issue #765. +func TestUpdateTunnelsRebuildsDeadTunnel(t *testing.T) { + starter := &fakeStarter{rsdPort: 4321} + prober := &fakeProber{err: errors.New("connection timed out")} + tm := robustnessManager(starter, prober, devEntry("dead-1", "USB")) + var closed int + tm.tunnels["dead-1"] = Tunnel{Udid: "dead-1", Address: "fd00::1", RsdPort: 1234, closer: func() error { closed++; return nil }} + + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels: %v", err) + } + + if closed != 1 { + t.Fatalf("dead tunnel closer called %d times, want 1", closed) + } + if got := starter.callsFor("dead-1"); got != 1 { + t.Fatalf("tunnel restarted %d times, want 1", got) + } + rebuilt, ok := tm.tunnels["dead-1"] + if !ok || rebuilt.RsdPort != 4321 { + t.Fatalf("expected rebuilt tunnel with RsdPort 4321, got ok=%v tunnel=%+v", ok, rebuilt) + } +} + +// A healthy tunnel record must survive the probe untouched: no teardown, no +// restart. +func TestUpdateTunnelsKeepsHealthyTunnel(t *testing.T) { + starter := &fakeStarter{rsdPort: 4321} + prober := &fakeProber{} + tm := robustnessManager(starter, prober, devEntry("ok-1", "USB")) + var closed int + tm.tunnels["ok-1"] = Tunnel{Udid: "ok-1", Address: "fd00::1", RsdPort: 1234, closer: func() error { closed++; return nil }} + + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels: %v", err) + } + + if len(prober.probed) != 1 || prober.probed[0] != "ok-1" { + t.Fatalf("probed = %v, want [ok-1]", prober.probed) + } + if closed != 0 { + t.Fatalf("healthy tunnel closer called %d times, want 0", closed) + } + if got := starter.callsFor("ok-1"); got != 0 { + t.Fatalf("healthy tunnel restarted %d times, want 0", got) + } + if tun := tm.tunnels["ok-1"]; tun.RsdPort != 1234 { + t.Fatalf("healthy tunnel record changed: %+v", tun) + } +} + +// A record whose device already vanished from usbmux is not probed — the +// existing disconnect teardown owns that case. +func TestUpdateTunnelsDoesNotProbeDisconnectedDevice(t *testing.T) { + starter := &fakeStarter{} + prober := &fakeProber{err: errors.New("dead")} + tm := robustnessManager(starter, prober) // no devices connected + tm.tunnels["gone-1"] = Tunnel{Udid: "gone-1", closer: func() error { return nil }} + + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels: %v", err) + } + + if len(prober.probed) != 0 { + t.Fatalf("probed = %v, want none for a disconnected device", prober.probed) + } + if _, ok := tm.tunnels["gone-1"]; ok { + t.Fatal("disconnected device's tunnel should have been torn down") + } +} + +// shouldProbe rate-limits probes to one per probeInterval and records attempts. +func TestShouldProbeRespectsInterval(t *testing.T) { + tm := &TunnelManager{ + pr: &fakeProber{}, + probeInterval: time.Hour, + lastProbe: map[string]time.Time{}, + } + now := time.Now() + if !tm.shouldProbe("a", now) { + t.Fatal("first probe must be due") + } + if tm.shouldProbe("a", now.Add(time.Minute)) { + t.Fatal("probe inside the interval must not be due") + } + if !tm.shouldProbe("a", now.Add(2*time.Hour)) { + t.Fatal("probe after the interval must be due") + } + // Managers without a prober (zero value, as older tests construct) never probe. + if (&TunnelManager{}).shouldProbe("a", now) { + t.Fatal("manager without prober/interval must not probe") + } +} + +// The default prober cannot judge a userspace tunnel (the local forwarder +// accepts connects regardless of device state), so it must report healthy +// instead of dialing. +func TestRsdProberSkipsUserspaceTunnels(t *testing.T) { + if err := (rsdProber{}).Probe(Tunnel{Udid: "u-1", UserspaceTUN: true}); err != nil { + t.Fatalf("userspace tunnel probe = %v, want nil", err) + } +} + +// manualPairingTunnelStart classifies version-based failures with +// ErrUnsupportedVersion so the manager can stop retrying them (issue #523). +func TestManualPairingTunnelStartClassifiesUnsupportedVersions(t *testing.T) { + starter := manualPairingTunnelStart{} + _, err := starter.StartTunnel(context.Background(), ios.DeviceEntry{}, PairRecordManager{}, semver.MustParse("16.6.0"), false) + if !errors.Is(err, ErrUnsupportedVersion) { + t.Fatalf("iOS 16 error = %v, want ErrUnsupportedVersion", err) + } + _, err = starter.StartTunnel(context.Background(), ios.DeviceEntry{}, PairRecordManager{}, semver.MustParse("17.2.0"), true) + if !errors.Is(err, ErrUnsupportedVersion) { + t.Fatalf("iOS 17.2 userspace error = %v, want ErrUnsupportedVersion", err) + } +} + +// An unsupported-version failure marks the device permanently skipped: exactly +// one start attempt, no backoff entry, and no further attempts on later cycles +// (previously this warned every single update cycle, issue #523). +func TestUpdateTunnelsSkipsUnsupportedDevicePermanently(t *testing.T) { + starter := &fakeStarter{err: fmt.Errorf("manualPairingTunnelStart: %w 16.6.0", ErrUnsupportedVersion)} + tm := robustnessManager(starter, &fakeProber{}, devEntry("old-1", "USB")) + + for i := 0; i < 3; i++ { + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels cycle %d: %v", i, err) + } + } + + if got := starter.callsFor("old-1"); got != 1 { + t.Fatalf("unsupported device attempted %d times, want exactly 1", got) + } + if !tm.unsupportedDevices["old-1"] { + t.Fatal("device should be marked unsupported") + } + if _, ok := tm.failedDevices["old-1"]; ok { + t.Fatal("unsupported device must not enter the transient-failure backoff") + } +} + +// A transient failure keeps the existing backoff behavior and never lands in +// the permanent unsupported set. +func TestUpdateTunnelsKeepsBackoffForTransientFailures(t *testing.T) { + starter := &fakeStarter{err: errors.New("pairing handshake failed")} + tm := robustnessManager(starter, &fakeProber{}, devEntry("flaky-1", "USB")) + + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels: %v", err) + } + + if got, ok := tm.failedDevices["flaky-1"]; !ok || got.failCount != 1 { + t.Fatalf("transient failure should be backed off, got ok=%v entry=%+v", ok, got) + } + if tm.unsupportedDevices["flaky-1"] { + t.Fatal("transient failure must not mark the device unsupported") + } +} + +// With a udid filter set, only the matching device gets a tunnel; all others +// are ignored entirely (issues #607, #479). An empty filter manages everything. +func TestUpdateTunnelsUdidFilter(t *testing.T) { + starter := &fakeStarter{rsdPort: 1111} + tm := robustnessManager(starter, &fakeProber{}, devEntry("match-1", "USB"), devEntry("other-1", "USB")) + tm.udidFilter = "match-1" + + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels: %v", err) + } + + if got := starter.callsFor("match-1"); got != 1 { + t.Fatalf("filtered device attempted %d times, want 1", got) + } + if got := starter.callsFor("other-1"); got != 0 { + t.Fatalf("non-matching device attempted %d times, want 0", got) + } + if _, ok := tm.tunnels["match-1"]; !ok { + t.Fatal("filtered device should have a tunnel") + } + if _, ok := tm.tunnels["other-1"]; ok { + t.Fatal("non-matching device must not have a tunnel") + } + + // Empty filter (the default) manages all devices. + all := &fakeStarter{rsdPort: 2222} + tmAll := robustnessManager(all, &fakeProber{}, devEntry("match-1", "USB"), devEntry("other-1", "USB")) + if err := tmAll.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels: %v", err) + } + if len(tmAll.tunnels) != 2 { + t.Fatalf("expected tunnels for both devices, got %v", tmAll.tunnels) + } +} From 0352b30d568f4d16b3ff1bc6adf22e0776b811f3 Mon Sep 17 00:00:00 2001 From: Daniel Paulus Date: Fri, 7 Aug 2026 09:59:45 -0400 Subject: [PATCH 3/3] fix(tunnel): concurrent bounded liveness probes + failure hysteresis 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 Claude-Session: https://claude.ai/code/session_01J8eMENxJ1nec9CeHp4tjWk --- ios/tunnel/tunnel_api.go | 121 ++++++++++++++--- ios/tunnel/tunnel_manager_robustness_test.go | 131 +++++++++++++++++-- 2 files changed, 229 insertions(+), 23 deletions(-) diff --git a/ios/tunnel/tunnel_api.go b/ios/tunnel/tunnel_api.go index 834da1c6..a341d3b3 100644 --- a/ios/tunnel/tunnel_api.go +++ b/ios/tunnel/tunnel_api.go @@ -340,7 +340,12 @@ type TunnelManager struct { unsupportedDevices map[string]bool // lastProbe tracks when each tunnel was last liveness-probed so probes run // at most once per probeInterval. - lastProbe map[string]time.Time + lastProbe map[string]time.Time + // probeFailures counts consecutive failed liveness probes per udid. A tunnel + // is only torn down after probeFailureThreshold consecutive failures so a + // single transient probe error (a momentary route hiccup or load spike) + // doesn't destroy an otherwise healthy tunnel. A successful probe resets it. + probeFailures map[string]int probeInterval time.Duration startTunnelTimeout time.Duration firstUpdateCompleted bool @@ -387,6 +392,7 @@ func newTunnelManager(pm PairRecordManager, userspaceTUN bool, udidFilter string failedDevices: map[string]failedDevice{}, unsupportedDevices: map[string]bool{}, lastProbe: map[string]time.Time{}, + probeFailures: map[string]int{}, probeInterval: defaultProbeInterval, startTunnelTimeout: 10 * time.Second, userspaceTUN: userspaceTUN, @@ -460,19 +466,18 @@ func (m *TunnelManager) UpdateTunnels(ctx context.Context) error { // Liveness-probe existing tunnel records of still-connected devices: a // tunnel can die without a usbmux disconnect (quick reboot, transport // death), leaving a stale record that would otherwise be served forever. A - // failed probe tears the record down here so the create loop below rebuilds - // it in the same cycle. Records of vanished devices are handled by the - // disconnect teardown at the end. + // tunnel that fails probeFailureThreshold consecutive probes is torn down + // here so the create loop below rebuilds it in the same cycle. Records of + // vanished devices are handled by the disconnect teardown at the end. + // + // Probes run concurrently (bounded) because a dead-but-still-routed tunnel + // blackholes the SYN and burns the full probeDialTimeout; probing serially + // would let N dead tunnels delay tunnel creation for newly connected devices + // by up to probeDialTimeout*N. now := time.Now() - for udid, tun := range localTunnels { - if !currentUDIDs[udid] || !m.shouldProbe(udid, now) { - continue - } - if err := m.pr.Probe(tun); err != nil { - golog.Warn("tunnel failed liveness probe, restarting it", "module", logModule, "udid", udid, "error", err) - _ = m.stopTunnel(tun) - delete(localTunnels, udid) - } + for udid := range m.probeTunnels(localTunnels, currentUDIDs, now) { + _ = m.stopTunnel(localTunnels[udid]) + delete(localTunnels, udid) } for _, d := range devices.DeviceList { @@ -541,6 +546,21 @@ func (m *TunnelManager) UpdateTunnels(ctx context.Context) error { delete(m.failedDevices, udid) } } + // Prune per-device bookkeeping for devices that are no longer connected so + // these maps stay bounded by the current device count instead of growing + // with every udid ever seen. Pruning unsupportedDevices on disconnect also + // lets a device that was unsupported, then upgraded to a tunnel-capable iOS + // and reconnected, be retried instead of skipped for the rest of the process. + for udid := range m.unsupportedDevices { + if !currentUDIDs[udid] { + delete(m.unsupportedDevices, udid) + } + } + for udid := range m.probeFailures { + if !currentUDIDs[udid] { + delete(m.probeFailures, udid) + } + } m.firstUpdateCompleted = true m.mux.Unlock() return nil @@ -574,6 +594,61 @@ func (m *TunnelManager) shouldProbe(udid string, now time.Time) bool { return true } +// probeTunnels liveness-probes every due tunnel of a still-connected device and +// returns the set of udids that should be torn down. Probes run concurrently +// with at most maxConcurrentProbes in flight so a batch of dead-but-routed +// tunnels (each of which blackholes the SYN for the full probeDialTimeout) +// can't serialize into a multi-second stall of the update loop. A tunnel is only +// reported dead after probeFailureThreshold consecutive failures; a success +// resets its counter, so a single transient probe error is absorbed. +func (m *TunnelManager) probeTunnels(localTunnels map[string]Tunnel, currentUDIDs map[string]bool, now time.Time) map[string]bool { + type probeResult struct { + udid string + err error + } + results := make(chan probeResult) + sem := make(chan struct{}, maxConcurrentProbes) + var wg sync.WaitGroup + for udid, tun := range localTunnels { + if !currentUDIDs[udid] || !m.shouldProbe(udid, now) { + continue + } + wg.Add(1) + go func(udid string, tun Tunnel) { + defer wg.Done() + sem <- struct{}{} + defer func() { <-sem }() + results <- probeResult{udid: udid, err: m.pr.Probe(tun)} + }(udid, tun) + } + go func() { + wg.Wait() + close(results) + }() + + dead := map[string]bool{} + m.mux.Lock() + defer m.mux.Unlock() + for r := range results { + if r.err == nil { + delete(m.probeFailures, r.udid) + continue + } + m.probeFailures[r.udid]++ + fails := m.probeFailures[r.udid] + if fails < probeFailureThreshold { + golog.Warn("tunnel failed liveness probe, will retry before restarting it", + "module", logModule, "udid", r.udid, "consecutiveFailures", fails, "threshold", probeFailureThreshold, "error", r.err) + continue + } + golog.Warn("tunnel failed liveness probe repeatedly, restarting it", + "module", logModule, "udid", r.udid, "consecutiveFailures", fails, "error", r.err) + delete(m.probeFailures, r.udid) + dead[r.udid] = true + } + return dead +} + // failedDeviceBackoff returns how long to wait before retrying a device after // failCount consecutive failures: 30s, 60s, 120s, 240s, capped at 5 minutes. func failedDeviceBackoff(failCount int) time.Duration { @@ -598,6 +673,7 @@ func (m *TunnelManager) RemoveTunnel(ctx context.Context, serialNumber string) e delete(m.tunnels, serialNumber) } delete(m.lastProbe, serialNumber) + delete(m.probeFailures, serialNumber) m.mux.Unlock() if !exists { @@ -612,6 +688,7 @@ func (m *TunnelManager) stopTunnel(t Tunnel) error { golog.Info("stopping tunnel", "module", logModule, "udid", t.Udid) delete(m.tunnels, t.Udid) delete(m.lastProbe, t.Udid) + delete(m.probeFailures, t.Udid) m.mux.Unlock() return t.Close() @@ -673,8 +750,10 @@ type tunnelProber interface { } // defaultProbeInterval is how often the TunnelManager liveness-probes each -// tunnel record. Combined with the probe's short dial timeout, a dead tunnel -// is detected and rebuilt well within a minute instead of never. +// tunnel record. Combined with the probe's short dial timeout and the +// probeFailureThreshold consecutive-failure requirement, a persistently dead +// tunnel is detected and rebuilt within roughly a minute instead of never, +// while a single transient probe error never tears down a healthy tunnel. const defaultProbeInterval = 30 * time.Second // probeDialTimeout bounds the liveness probe's TCP connect. A healthy tunnel @@ -682,6 +761,18 @@ const defaultProbeInterval = 30 * time.Second // short timeout is enough to tell them apart without stalling UpdateTunnels. const probeDialTimeout = 5 * time.Second +// maxConcurrentProbes caps how many liveness probes run at once. Probes are +// concurrent so a batch of dead-but-routed tunnels can't serialize into a +// probeDialTimeout*N stall, but bounded so a large fleet can't open an +// unbounded number of dial sockets in one cycle. +const maxConcurrentProbes = 8 + +// probeFailureThreshold is the number of consecutive failed liveness probes +// required before a tunnel is torn down and rebuilt. Requiring two failures +// keeps a single transient probe error (a momentary route hiccup) from +// destroying a healthy tunnel. +const probeFailureThreshold = 2 + // rsdProber is the default tunnelProber: a short-timeout TCP dial of the // tunnel's RSD endpoint over the kernel TUN route. type rsdProber struct { diff --git a/ios/tunnel/tunnel_manager_robustness_test.go b/ios/tunnel/tunnel_manager_robustness_test.go index 95ef9a49..0bbfd4b5 100644 --- a/ios/tunnel/tunnel_manager_robustness_test.go +++ b/ios/tunnel/tunnel_manager_robustness_test.go @@ -51,15 +51,24 @@ func (f *fakeStarter) callsFor(udid string) int { } type fakeProber struct { + mu sync.Mutex err error probed []string } func (f *fakeProber) Probe(t Tunnel) error { + f.mu.Lock() f.probed = append(f.probed, t.Udid) + f.mu.Unlock() return f.err } +func (f *fakeProber) probedUDIDs() []string { + f.mu.Lock() + defer f.mu.Unlock() + return append([]string(nil), f.probed...) +} + // robustnessManager builds a fully faked TunnelManager that probes on every // UpdateTunnels cycle and resolves every device to iOS 18.0.0. func robustnessManager(ts tunnelStarter, pr tunnelProber, entries ...ios.DeviceEntry) *TunnelManager { @@ -71,6 +80,7 @@ func robustnessManager(ts tunnelStarter, pr tunnelProber, entries ...ios.DeviceE failedDevices: map[string]failedDevice{}, unsupportedDevices: map[string]bool{}, lastProbe: map[string]time.Time{}, + probeFailures: map[string]int{}, probeInterval: time.Nanosecond, startTunnelTimeout: time.Second, productVersion: func(ios.DeviceEntry) (*semver.Version, error) { @@ -79,9 +89,10 @@ func robustnessManager(ts tunnelStarter, pr tunnelProber, entries ...ios.DeviceE } } -// A tunnel record whose device is still connected but whose probe fails must be -// torn down and rebuilt in the same update cycle — the agent-side self-heal -// from issue #765. +// A tunnel record whose device is still connected but whose probe keeps failing +// must be torn down and rebuilt — the agent-side self-heal from issue #765. +// Teardown requires probeFailureThreshold consecutive failures, so it happens on +// the second failing cycle, not the first. func TestUpdateTunnelsRebuildsDeadTunnel(t *testing.T) { starter := &fakeStarter{rsdPort: 4321} prober := &fakeProber{err: errors.New("connection timed out")} @@ -89,10 +100,21 @@ func TestUpdateTunnelsRebuildsDeadTunnel(t *testing.T) { var closed int tm.tunnels["dead-1"] = Tunnel{Udid: "dead-1", Address: "fd00::1", RsdPort: 1234, closer: func() error { closed++; return nil }} + // First failing probe: below the threshold, so the tunnel survives. if err := tm.UpdateTunnels(context.Background()); err != nil { - t.Fatalf("UpdateTunnels: %v", err) + t.Fatalf("UpdateTunnels cycle 1: %v", err) + } + if closed != 0 { + t.Fatalf("after one failed probe closer called %d times, want 0 (needs %d consecutive)", closed, probeFailureThreshold) + } + if got := starter.callsFor("dead-1"); got != 0 { + t.Fatalf("after one failed probe tunnel restarted %d times, want 0", got) } + // Second consecutive failing probe: threshold reached, tear down and rebuild. + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels cycle 2: %v", err) + } if closed != 1 { t.Fatalf("dead tunnel closer called %d times, want 1", closed) } @@ -105,6 +127,38 @@ func TestUpdateTunnelsRebuildsDeadTunnel(t *testing.T) { } } +// A single transient probe failure followed by a success must NOT tear down the +// tunnel: the consecutive-failure counter resets on success (issue #765 — +// avoiding false-positive teardown of healthy tunnels under momentary load). +func TestUpdateTunnelsAbsorbsTransientProbeFailure(t *testing.T) { + starter := &fakeStarter{rsdPort: 4321} + prober := &fakeProber{err: errors.New("temporary hiccup")} + tm := robustnessManager(starter, prober, devEntry("flap-1", "USB")) + var closed int + tm.tunnels["flap-1"] = Tunnel{Udid: "flap-1", Address: "fd00::1", RsdPort: 1234, closer: func() error { closed++; return nil }} + + // One failing cycle (count=1), then the probe recovers. + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels cycle 1: %v", err) + } + prober.mu.Lock() + prober.err = nil + prober.mu.Unlock() + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels cycle 2: %v", err) + } + + if closed != 0 { + t.Fatalf("tunnel torn down after a single transient failure, closer called %d times", closed) + } + if got := starter.callsFor("flap-1"); got != 0 { + t.Fatalf("tunnel restarted %d times after transient failure, want 0", got) + } + if n := tm.probeFailures["flap-1"]; n != 0 { + t.Fatalf("probeFailures[flap-1] = %d after a success, want 0 (counter must reset)", n) + } +} + // A healthy tunnel record must survive the probe untouched: no teardown, no // restart. func TestUpdateTunnelsKeepsHealthyTunnel(t *testing.T) { @@ -118,8 +172,8 @@ func TestUpdateTunnelsKeepsHealthyTunnel(t *testing.T) { t.Fatalf("UpdateTunnels: %v", err) } - if len(prober.probed) != 1 || prober.probed[0] != "ok-1" { - t.Fatalf("probed = %v, want [ok-1]", prober.probed) + if probed := prober.probedUDIDs(); len(probed) != 1 || probed[0] != "ok-1" { + t.Fatalf("probed = %v, want [ok-1]", probed) } if closed != 0 { t.Fatalf("healthy tunnel closer called %d times, want 0", closed) @@ -144,8 +198,8 @@ func TestUpdateTunnelsDoesNotProbeDisconnectedDevice(t *testing.T) { t.Fatalf("UpdateTunnels: %v", err) } - if len(prober.probed) != 0 { - t.Fatalf("probed = %v, want none for a disconnected device", prober.probed) + if probed := prober.probedUDIDs(); len(probed) != 0 { + t.Fatalf("probed = %v, want none for a disconnected device", probed) } if _, ok := tm.tunnels["gone-1"]; ok { t.Fatal("disconnected device's tunnel should have been torn down") @@ -274,3 +328,64 @@ func TestUpdateTunnelsUdidFilter(t *testing.T) { t.Fatalf("expected tunnels for both devices, got %v", tmAll.tunnels) } } + +// mutableLister lets a test change the connected-device list between cycles. +type mutableLister struct { + mu sync.Mutex + entries []ios.DeviceEntry +} + +func (l *mutableLister) ListDevices() (ios.DeviceList, error) { + l.mu.Lock() + defer l.mu.Unlock() + return ios.DeviceList{DeviceList: append([]ios.DeviceEntry(nil), l.entries...)}, nil +} + +func (l *mutableLister) set(entries ...ios.DeviceEntry) { + l.mu.Lock() + defer l.mu.Unlock() + l.entries = entries +} + +// An unsupported classification must not survive a disconnect: once the device +// leaves usbmux the entry is pruned, so reconnecting (e.g. after an iOS upgrade +// to a tunnel-capable version) is retried instead of skipped forever. This also +// bounds the unsupportedDevices map by the current device count rather than by +// every udid ever seen (issue #523 growth follow-up). +func TestUpdateTunnelsPrunesUnsupportedOnDisconnect(t *testing.T) { + lister := &mutableLister{} + lister.set(devEntry("up-1", "USB")) + starter := &fakeStarter{err: fmt.Errorf("manualPairingTunnelStart: %w 16.6.0", ErrUnsupportedVersion)} + tm := robustnessManager(starter, &fakeProber{}) + tm.dl = lister + + // Cycle 1: device is unsupported, gets marked and skipped. + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels cycle 1: %v", err) + } + if !tm.unsupportedDevices["up-1"] { + t.Fatal("device should be marked unsupported after cycle 1") + } + + // Cycle 2: device disconnects; the unsupported entry must be pruned. + lister.set() + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels cycle 2: %v", err) + } + if tm.unsupportedDevices["up-1"] { + t.Fatal("unsupported entry must be pruned once the device disconnects") + } + + // Cycle 3: device reconnects on a supported version; it must be retried. + lister.set(devEntry("up-1", "USB")) + starter.mu.Lock() + starter.err = nil + starter.rsdPort = 9999 + starter.mu.Unlock() + if err := tm.UpdateTunnels(context.Background()); err != nil { + t.Fatalf("UpdateTunnels cycle 3: %v", err) + } + if _, ok := tm.tunnels["up-1"]; !ok { + t.Fatal("reconnected (now-supported) device should get a tunnel, not stay skipped") + } +}