diff --git a/internal/driver/ioscompanion/device.go b/internal/driver/ioscompanion/device.go index 9dd7766..67536a7 100644 --- a/internal/driver/ioscompanion/device.go +++ b/internal/driver/ioscompanion/device.go @@ -109,6 +109,13 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { d.restart = d.respawnDevice d.processContext, d.processCancel = context.WithCancel(ctx) + lock, err := acquireDeviceLock(d.udid) + if err != nil { + d.processCancel() + return nil, err + } + d.deviceLock = lock + if err := d.bringUpDevice(ctx); err != nil { d.Close() return nil, err diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index 373dbbd..78a2ada 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -42,6 +42,17 @@ const runnerStartupTimeout = 120 * time.Second // before it is killed. A variable so the kill-escalation test can shrink it. var shutdownGrace = 15 * time.Second +// launchTimeout bounds a single app lifecycle RPC. The runner serves lifecycle +// inside its XCTest session, and a launch the simulator rejects sends that +// session down a recovery chain (a 120s accessibility wait, a spindump, then an +// idle wait) that answers minutes late or never. Callers reach Launch with an +// undeadlined context, since it runs before the run's duration clock starts, so +// the bound has to come from here or a wedged session hangs the run with no +// trace, no error, and no end. Kept under runnerStartupTimeout: launching an +// app inside a live session must cost less than cold-starting that session. +// A variable so the timeout test can shrink it. +var launchTimeout = 90 * time.Second + // longPressHoldMilliseconds is how long LongPress holds the finger down. const longPressHoldMilliseconds = 600 @@ -141,6 +152,34 @@ type Driver struct { // the moment startup finishes. processContext context.Context processCancel context.CancelFunc + + // deviceLock is the exclusive claim on the target, held for the driver's + // whole life and released by Close. + deviceLock io.Closer +} + +// acquireDeviceLock takes an exclusive advisory lock on the target so only one +// run drives it at a time. Two runs on one device interleave app lifecycle: the +// second run's uninstall and reinstall land under the first's live automation +// session, leaving its app proxies bound to a bundle the simulator no longer +// knows, and every later snapshot and launch on that session stalls. Failing +// fast beats recovering silently, since the other run owns the device and would +// be corrupted either way. The lock lives on the file descriptor, so a crashed +// run's claim is released by the kernel and never strands the device. +func acquireDeviceLock(udid string) (io.Closer, error) { + path := filepath.Join(os.TempDir(), "sanderling-ios-"+udid+".lock") + file, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o644) + if err != nil { + return nil, fmt.Errorf("open device lock %s: %w", path, err) + } + if err := syscall.Flock(int(file.Fd()), syscall.LOCK_EX|syscall.LOCK_NB); err != nil { + file.Close() + return nil, fmt.Errorf( + "ios target %s is already driven by another sanderling run (lock %s); "+ + "wait for that run to finish or point this one at a different device with --ios-device", + udid, path) + } + return file, nil } // New extracts the embedded companion, spawns it against the configured @@ -199,7 +238,15 @@ func New(ctx context.Context, options Options) (*Driver, error) { driverInstance.grantPaste = driverInstance.grantPasteboardAccess driverInstance.processContext, driverInstance.processCancel = context.WithCancel(ctx) + lock, err := acquireDeviceLock(driverInstance.udid) + if err != nil { + driverInstance.processCancel() + return nil, err + } + driverInstance.deviceLock = lock + if err := driverInstance.bringUp(ctx); err != nil { + driverInstance.Close() return nil, err } if driverInstance.hybrid { @@ -408,7 +455,9 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, e // Terminate first so the launch is a clean cold start regardless of the // app's prior state. A not-running app is not an error here. - _ = d.withRecovery(ctx, func() error { return d.lifecycleCompanion().Terminate(ctx, d.bundleID) }) + _ = d.lifecycleCall(ctx, func(callCtx context.Context, companion transport.Companion) error { + return companion.Terminate(callCtx, d.bundleID) + }) if clearState { if err := d.clearAppState(ctx); err != nil { @@ -428,14 +477,26 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, e } } - if err := d.withRecovery(ctx, func() error { - return d.lifecycleCompanion().Launch(ctx, d.bundleID, true) + if err := d.lifecycleCall(ctx, func(callCtx context.Context, companion transport.Companion) error { + return companion.Launch(callCtx, d.bundleID, true) }); err != nil { return fmt.Errorf("launch %s: %w", d.bundleID, err) } return nil } +// lifecycleCall runs an app lifecycle RPC against lifecycleCompanion under a +// launchTimeout-bounded context, with the usual one-restart recovery. The +// companion is resolved inside the retry so a restart's replacement client +// serves the second attempt. +func (d *Driver) lifecycleCall(ctx context.Context, call func(context.Context, transport.Companion) error) error { + boundedCtx, cancel := context.WithTimeout(ctx, launchTimeout) + defer cancel() + return d.withRecovery(boundedCtx, func() error { + return call(boundedCtx, d.lifecycleCompanion()) + }) +} + // lifecycleCompanion is the transport that owns app launch and terminate: the // in-simulator runner when the hybrid is active, otherwise the legacy // companion. Lifecycle performed outside the runner's automation session @@ -528,7 +589,9 @@ func (d *Driver) resetDataContainer(ctx context.Context) error { } func (d *Driver) Terminate(ctx context.Context) error { - return d.withRecovery(ctx, func() error { return d.lifecycleCompanion().Terminate(ctx, d.bundleID) }) + return d.lifecycleCall(ctx, func(callCtx context.Context, companion transport.Companion) error { + return companion.Terminate(callCtx, d.bundleID) + }) } func (d *Driver) Tap(ctx context.Context, x, y int) error { @@ -999,6 +1062,10 @@ func (d *Driver) Close() { if d.processCancel != nil { d.processCancel() } + if d.deviceLock != nil { + _ = d.deviceLock.Close() + d.deviceLock = nil + } } // stopTunnel closes the in-process usbmux forwarder on the device path. Closing diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index 32ff96e..a70ae6a 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -851,3 +851,175 @@ func (s *sequencedDumpCompanion) AccessibilityInfo(context.Context) (string, err *s.reads++ return s.dumps[index], nil } + +// wedgedLifecycleCompanion never answers a lifecycle RPC, standing in for a +// runner whose XCTest session is stuck inside a rejected launch. +type wedgedLifecycleCompanion struct { + fakeCompanion + release chan struct{} +} + +func (w *wedgedLifecycleCompanion) block(ctx context.Context) error { + select { + case <-ctx.Done(): + return ctx.Err() + case <-w.release: + return nil + } +} + +func (w *wedgedLifecycleCompanion) Launch(ctx context.Context, _ string, _ bool) error { + return w.block(ctx) +} + +func (w *wedgedLifecycleCompanion) Terminate(ctx context.Context, _ string) error { + return w.block(ctx) +} + +// TestLaunchBoundsWedgedLifecycleRPC proves the launch path carries its own +// deadline. Callers hand Launch an undeadlined context, so without one a runner +// that never answers hangs the run forever with nothing printed. +func TestLaunchBoundsWedgedLifecycleRPC(t *testing.T) { + previous := launchTimeout + launchTimeout = 100 * time.Millisecond + defer func() { launchTimeout = previous }() + + companion := &wedgedLifecycleCompanion{release: make(chan struct{})} + defer close(companion.release) + d := newTestDriver(companion) + + done := make(chan error, 1) + go func() { done <- d.Launch(context.Background(), "com.example.app", false, nil) }() + + select { + case err := <-done: + if err == nil { + t.Fatal("wedged launch returned nil; a stuck runner must surface an error") + } + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("err = %v, want a deadline-exceeded error", err) + } + case <-time.After(10 * time.Second): + t.Fatal("Launch never returned: the lifecycle RPC is unbounded, so a stuck runner hangs the run forever") + } +} + +// TestTerminateBoundsWedgedLifecycleRPC covers the same bound on the standalone +// terminate, which the runner calls mid-run on an equally stuck session. +func TestTerminateBoundsWedgedLifecycleRPC(t *testing.T) { + previous := launchTimeout + launchTimeout = 100 * time.Millisecond + defer func() { launchTimeout = previous }() + + companion := &wedgedLifecycleCompanion{release: make(chan struct{})} + defer close(companion.release) + d := newTestDriver(companion) + + done := make(chan error, 1) + go func() { done <- d.Terminate(context.Background()) }() + + select { + case err := <-done: + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("err = %v, want a deadline-exceeded error", err) + } + case <-time.After(10 * time.Second): + t.Fatal("Terminate never returned: the lifecycle RPC is unbounded") + } +} + +// TestLaunchLeavesATighterCallerDeadlineAlone confirms the bound narrows the +// caller's context and never widens it, so a caller that wants to give up +// sooner still does. +func TestLaunchLeavesATighterCallerDeadlineAlone(t *testing.T) { + previous := launchTimeout + launchTimeout = 30 * time.Second + defer func() { launchTimeout = previous }() + + companion := &wedgedLifecycleCompanion{release: make(chan struct{})} + defer close(companion.release) + d := newTestDriver(companion) + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + start := time.Now() + if err := d.Launch(ctx, "com.example.app", false, nil); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("err = %v, want a deadline-exceeded error", err) + } + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("Launch took %v; the driver's bound overrode the caller's tighter deadline", elapsed) + } +} + +// newLockTestOptions builds New options that dial a seamed companion, so the +// device-lock tests exercise New without spawning anything. +func newLockTestOptions(t *testing.T, udid string) Options { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { listener.Close() }) + go func() { + for { + connection, acceptErr := listener.Accept() + if acceptErr != nil { + return + } + _ = connection.Close() + } + }() + return Options{ + UniqueDeviceIdentifier: udid, + pickAddress: func() (string, error) { return listener.Addr().String(), nil }, + spawnChild: func(context.Context, string) (*exec.Cmd, error) { return &exec.Cmd{}, nil }, + dialCompanion: func(string) (transport.Companion, error) { + return &fakeCompanion{accessibilityJSON: "[]"}, nil + }, + } +} + +// TestNewRejectsConcurrentRunOnSameDevice proves a second run cannot claim a +// device the first is driving. Two runs interleave app lifecycle on one +// simulator: the second's reinstall lands under the first's automation session +// and wedges it. Failing fast names the contended device; the claim is released +// on Close so the next run is not locked out. +func TestNewRejectsConcurrentRunOnSameDevice(t *testing.T) { + t.Setenv("SANDERLING_SIMULATOR_COMPANION", "legacy") + udid := "LOCK-TEST-" + t.Name() + + first, err := New(context.Background(), newLockTestOptions(t, udid)) + if err != nil { + t.Fatalf("first New: %v", err) + } + + _, err = New(context.Background(), newLockTestOptions(t, udid)) + if err == nil { + t.Fatal("second run claimed a device the first still drives; concurrent runs corrupt each other's session") + } + if !strings.Contains(err.Error(), udid) { + t.Fatalf("err = %v, want it to name the contended device %s", err, udid) + } + + first.Close() + third, err := New(context.Background(), newLockTestOptions(t, udid)) + if err != nil { + t.Fatalf("device stayed locked after Close: %v", err) + } + third.Close() +} + +// TestAcquireDeviceLockKeepsDistinctDevicesIndependent guards against a lock +// path that ignores the udid and serializes unrelated runs. +func TestAcquireDeviceLockKeepsDistinctDevicesIndependent(t *testing.T) { + first, err := acquireDeviceLock("LOCK-TEST-DEVICE-A") + if err != nil { + t.Fatal(err) + } + defer first.Close() + second, err := acquireDeviceLock("LOCK-TEST-DEVICE-B") + if err != nil { + t.Fatalf("a second device was refused while another was locked: %v", err) + } + defer second.Close() +}