diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index 78a2ada..2db8019 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -53,6 +53,14 @@ var shutdownGrace = 15 * time.Second // A variable so the timeout test can shrink it. var launchTimeout = 90 * time.Second +// launchRecoveryTimeout bounds the whole recovery a blown launch bound +// triggers, the session restart and the second attempt together. It keeps the +// launch path inside the three minutes testrun allows it, so what a user sees +// when the app really cannot be launched stays the driver's error rather than +// that backstop firing over the top of it. A variable so the bound test can +// shrink it. +var launchRecoveryTimeout = 60 * time.Second + // longPressHoldMilliseconds is how long LongPress holds the finger down. const longPressHoldMilliseconds = 600 @@ -477,14 +485,55 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, e } } - if err := d.lifecycleCall(ctx, func(callCtx context.Context, companion transport.Companion) error { - return companion.Launch(callCtx, d.bundleID, true) - }); err != nil { + if err := d.launchWithSessionRecovery(ctx); err != nil { return fmt.Errorf("launch %s: %w", d.bundleID, err) } return nil } +// launchWithSessionRecovery runs the launch RPC and, when it blows its own +// bound, replaces the session and launches again. +// +// A launch the simulator refuses, which is what a clear-state reinstall racing +// FrontBoard's registration produces, never comes back as an error: XCTest +// records the refusal as a test failure the runner cannot observe, then holds +// the session's main thread for about four minutes walking a diagnostic chain +// (a 120s accessibility wait, a spindump, an idle wait). So there is no error +// text to key a retry on, only the expired bound, and every later call queues +// behind the same wedge. Only a session that never served the refused launch +// can serve the retry, which is why this restarts rather than calls again. +func (d *Driver) launchWithSessionRecovery(ctx context.Context) error { + launch := func(callCtx context.Context, companion transport.Companion) error { + return companion.Launch(callCtx, d.bundleID, true) + } + err := d.lifecycleCall(ctx, launch) + // A caller whose own budget ran out gets no restart: the bound that expired + // was the caller's to spend, and the second attempt would inherit it dead. + if err == nil || !errors.Is(err, context.DeadlineExceeded) || ctx.Err() != nil || d.restart == nil { + return err + } + fmt.Fprintf(d.output, "launch %s blew its %v bound (%v); restarting the session and launching once more\n", + d.bundleID, launchTimeout, err) + + // The restart runs under the driver's own lifetime context for the same + // reason withRecovery's does, while the second attempt stays on the + // caller's. Both end at one deadline, so a launch that already spent + // launchTimeout cannot then wait out a session cold start on top of it. + recoveryDeadline := time.Now().Add(launchRecoveryTimeout) + restartCtx := d.processContext + if restartCtx == nil { + restartCtx = ctx + } + restartCtx, cancelRestart := context.WithDeadline(restartCtx, recoveryDeadline) + defer cancelRestart() + if restartErr := d.restart(restartCtx); restartErr != nil { + return fmt.Errorf("session restart failed: %w (original: %v)", restartErr, err) + } + relaunchCtx, cancelRelaunch := context.WithDeadline(ctx, recoveryDeadline) + defer cancelRelaunch() + return d.lifecycleCall(relaunchCtx, launch) +} + // 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 diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index a70ae6a..07316c2 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -951,6 +951,131 @@ func TestLaunchLeavesATighterCallerDeadlineAlone(t *testing.T) { } } +// wedgedUntilRestartCompanion models the session a refused launch leaves +// behind: the refusal is never reported, and no later launch is answered until +// the session itself is replaced. +type wedgedUntilRestartCompanion struct { + fakeCompanion + mutex sync.Mutex + replaced bool + attempted int +} + +func (w *wedgedUntilRestartCompanion) replaceSession() { + w.mutex.Lock() + defer w.mutex.Unlock() + w.replaced = true +} + +func (w *wedgedUntilRestartCompanion) launchAttempts() int { + w.mutex.Lock() + defer w.mutex.Unlock() + return w.attempted +} + +func (w *wedgedUntilRestartCompanion) Launch(ctx context.Context, _ string, _ bool) error { + w.mutex.Lock() + w.attempted++ + replaced := w.replaced + w.mutex.Unlock() + if replaced { + return nil + } + <-ctx.Done() + return ctx.Err() +} + +// TestLaunchReplacesTheSessionAfterALaunchBlowsItsBound covers the FrontBoard +// race: a clear-state reinstall the simulator has not finished registering +// makes the session refuse the launch, and XCTest answers that refusal with +// minutes of diagnostics instead of an error, so the bound expires and every +// later call queues behind the same wedge. Calling launch again on that session +// cannot work; the run only recovers if the session is replaced first. +func TestLaunchReplacesTheSessionAfterALaunchBlowsItsBound(t *testing.T) { + previous := launchTimeout + launchTimeout = 100 * time.Millisecond + defer func() { launchTimeout = previous }() + + companion := &wedgedUntilRestartCompanion{} + output := &bytes.Buffer{} + d := newTestDriver(companion) + d.output = output + restarts := 0 + d.restart = func(context.Context) error { + restarts++ + companion.replaceSession() + return nil + } + + if err := d.Launch(context.Background(), "", false, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + if restarts != 1 { + t.Fatalf("session restarts = %d, want exactly 1", restarts) + } + if attempts := companion.launchAttempts(); attempts != 2 { + t.Fatalf("launch attempts = %d, want 2: one that wedged and one on the replaced session", attempts) + } + if !strings.Contains(output.String(), "restarting the session") { + t.Fatalf("the recovery was silent, so a run that needed it never says so; output was %q", output.String()) + } +} + +// TestLaunchBoundsTheSessionRestartItTriggers keeps the recovery inside a +// budget of its own. The restart deliberately runs on the driver's lifetime +// context rather than the caller's, so without a deadline a session that never +// comes back would hang the launch path exactly the way #73 stopped it hanging. +func TestLaunchBoundsTheSessionRestartItTriggers(t *testing.T) { + previousLaunch, previousRecovery := launchTimeout, launchRecoveryTimeout + launchTimeout = 100 * time.Millisecond + launchRecoveryTimeout = 200 * time.Millisecond + defer func() { launchTimeout, launchRecoveryTimeout = previousLaunch, previousRecovery }() + + d := newTestDriver(&wedgedUntilRestartCompanion{}) + d.restart = func(restartCtx context.Context) error { + <-restartCtx.Done() + return restartCtx.Err() + } + + done := make(chan error, 1) + go func() { done <- d.Launch(context.Background(), "", false, nil) }() + select { + case err := <-done: + if err == nil || !strings.Contains(err.Error(), "session restart failed") { + t.Fatalf("err = %v, want the failed restart named", err) + } + case <-time.After(10 * time.Second): + t.Fatal("Launch never returned: a session that never comes back hangs the launch path") + } +} + +// TestLaunchKeepsTheSessionWhenTheCallersOwnDeadlineExpires holds the recovery +// to the driver's own bound. Spending a session restart on a caller that has +// already run out of budget cannot produce a launch, only a later failure. +func TestLaunchKeepsTheSessionWhenTheCallersOwnDeadlineExpires(t *testing.T) { + previous := launchTimeout + launchTimeout = 30 * time.Second + defer func() { launchTimeout = previous }() + + companion := &wedgedUntilRestartCompanion{} + d := newTestDriver(companion) + restarts := 0 + d.restart = func(context.Context) error { + restarts++ + companion.replaceSession() + return nil + } + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + if err := d.Launch(ctx, "", false, nil); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("err = %v, want a deadline-exceeded error", err) + } + if restarts != 0 { + t.Fatalf("session restarts = %d, want 0", restarts) + } +} + // 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 {