Skip to content

Commit e25fef0

Browse files
committed
fix(apple): scope snapshot circuits to runner sessions
1 parent 4e78ccc commit e25fef0

11 files changed

Lines changed: 302 additions & 82 deletions

‎apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/AgentDeviceRunnerUITests-Bridging-Header.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,7 @@
1515
#if __has_include("UnitTests/RunnerEdgeContactTesting.h")
1616
#import "UnitTests/RunnerEdgeContactTesting.h"
1717
#endif
18+
19+
#if __has_include("UnitTests/RunnerSnapshotBridgeTesting.h")
20+
#import "UnitTests/RunnerSnapshotBridgeTesting.h"
21+
#endif

‎apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSnapshotBridge.m‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -209,13 +209,14 @@ @implementation RunnerSnapshotBridgeAnswer
209209
/// is abandoned, not stopped: its answer is dropped when it returns.
210210
static NSDictionary *RunnerSnapshotBridgeRead(NSDictionary *request,
211211
NSUInteger budgetMs,
212-
NSDictionary * (^read)(void))
212+
NSDictionary * (^read)(void),
213+
NSTimeInterval (^clock)(void))
213214
{
214215
NSUInteger sequence;
215216
@synchronized (RunnerSnapshotBridgeAnswer.class) {
216217
if (RunnerSnapshotBridgeReadsInFlight) {
217218
BOOL wedged = RunnerSnapshotBridgeAbandonedAt > 0 &&
218-
NSProcessInfo.processInfo.systemUptime - RunnerSnapshotBridgeAbandonedAt >= RunnerSnapshotBridgeWedgeSeconds;
219+
clock() - RunnerSnapshotBridgeAbandonedAt >= RunnerSnapshotBridgeWedgeSeconds;
219220
return RunnerSnapshotBridgeFailure(request, @"application_not_responding",
220221
wedged ? @"bridge-reader-wedged" : @"bridge-read-in-flight",
221222
@"another accessibility read has not returned yet");
@@ -242,7 +243,7 @@ @implementation RunnerSnapshotBridgeAnswer
242243
if (dispatch_semaphore_wait(done, dispatch_time(DISPATCH_TIME_NOW, (int64_t)budgetMs * NSEC_PER_MSEC)) != 0) {
243244
@synchronized (RunnerSnapshotBridgeAnswer.class) {
244245
if (RunnerSnapshotBridgeReadsInFlight && RunnerSnapshotBridgeReadSequence == sequence) {
245-
RunnerSnapshotBridgeAbandonedAt = NSProcessInfo.processInfo.systemUptime;
246+
RunnerSnapshotBridgeAbandonedAt = clock();
246247
}
247248
}
248249
return RunnerSnapshotBridgeFailure(request, @"application_not_responding", @"bridge-read-abandoned",
@@ -251,6 +252,18 @@ @implementation RunnerSnapshotBridgeAnswer
251252
return answer.value;
252253
}
253254

255+
#if defined(AGENT_DEVICE_RUNNER_UNIT_TESTS)
256+
NSDictionary *RunnerSnapshotBridgeReadForTest(NSDictionary *request, NSUInteger budgetMs,
257+
NSDictionary *(^read)(void), NSTimeInterval now)
258+
{
259+
return RunnerSnapshotBridgeRead(request, budgetMs, read, ^{ return now; });
260+
}
261+
void RunnerSnapshotBridgeDrainForTest(void)
262+
{
263+
dispatch_sync(RunnerSnapshotBridgeReaderQueue(), ^{});
264+
}
265+
#endif
266+
254267
uint32_t RunnerSnapshotBridgeMaximumFrameBytes(void)
255268
{
256269
return kMaximumFrameBytes;
@@ -266,12 +279,12 @@ uint32_t RunnerSnapshotBridgeMaximumFrameBytes(void)
266279
if ([verb isEqualToString:@"target"]) {
267280
return RunnerSnapshotBridgeRead(request, RunnerSnapshotBridgeBudgetMs(request), ^{
268281
return RunnerSnapshotBridgeTarget(bundleId, request);
269-
});
282+
}, ^{ return NSProcessInfo.processInfo.systemUptime; });
270283
}
271284
if ([verb isEqualToString:@"describe"]) {
272285
return RunnerSnapshotBridgeRead(request, RunnerSnapshotBridgeBudgetMs(request), ^{
273286
return RunnerSnapshotBridgeDescribe(bundleId, request);
274-
});
287+
}, ^{ return NSProcessInfo.processInfo.systemUptime; });
275288
}
276289
return RunnerSnapshotBridgeFailure(request, @"bad_request", @"verb-not-supported",
277290
@"the runner snapshot bridge accepts target and describe requests only");
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#import <Foundation/Foundation.h>
2+
#import <TargetConditionals.h>
3+
NS_ASSUME_NONNULL_BEGIN
4+
#if TARGET_OS_IOS && TARGET_OS_SIMULATOR
5+
NSDictionary *RunnerSnapshotBridgeReadForTest(NSDictionary *request, NSUInteger budgetMs,
6+
NSDictionary *(^read)(void), NSTimeInterval now);
7+
void RunnerSnapshotBridgeDrainForTest(void);
8+
#endif
9+
NS_ASSUME_NONNULL_END

‎apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotBridgeTests.swift‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,24 @@ extension RunnerTests {
6666
let named = Self.snapshotBridgeReaderRequest(payload, simulatorUdid: "SIM")
6767
XCTAssertEqual(named["simulatorUdid"] as? String, "SIM")
6868
}
69+
func testAbandonedReadBecomesWedgedUntilItReturns() {
70+
let release = DispatchSemaphore(value: 0)
71+
let request = ["requestId": "wedge"]
72+
let first = RunnerSnapshotBridgeReadForTest(request, 100, {
73+
release.wait()
74+
return ["ok": true]
75+
}, 10)
76+
XCTAssertEqual(first["error_code"] as? String, "bridge-read-abandoned")
77+
let busy = RunnerSnapshotBridgeReadForTest(request, 100, { ["ok": true] }, 14.999)
78+
XCTAssertEqual(busy["error_code"] as? String, "bridge-read-in-flight")
79+
let wedged = RunnerSnapshotBridgeReadForTest(request, 100, { ["ok": true] }, 15)
80+
XCTAssertEqual(wedged["error_code"] as? String, "bridge-reader-wedged")
81+
release.signal()
82+
RunnerSnapshotBridgeDrainForTest()
83+
let admitted = RunnerSnapshotBridgeReadForTest(request, 1000, { ["ok": true] }, 16)
84+
XCTAssertEqual(admitted["ok"] as? Bool, true)
85+
}
86+
6987
func testReaderDoesNotRefuseTheBoundsOfAFirstRead() throws {
7088
var request = Self.snapshotBridgeReaderRequest(try firstReadPayload())
7189
request["pid"] = 1

‎packages/platform-apple/src/runner-runtime.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ export function createAppleRunnerRuntime(options: {
6060
// the foreground read starts a runner that is not up yet, under the capture's own budget, instead
6161
// of giving up the proof that keeps the capture on the bridge.
6262
let snapshotRoute: SnapshotBridgeRoute | undefined;
63-
let runnerSessionId: string | undefined;
6463
const readForeground = createSystemSurfaceForegroundProbe(async (device, input) => {
6564
if (!options.provider.hasLiveSession(device)) {
6665
try {
@@ -76,11 +75,6 @@ export function createAppleRunnerRuntime(options: {
7675
}
7776
const session = readRunnerSessionLiveness(device.id);
7877
if (session?.liveness !== 'ready') return undefined;
79-
if (runnerSessionId !== undefined && runnerSessionId !== session.sessionId) {
80-
bridge.resetTargets();
81-
snapshotRoute?.resetRunnerSession();
82-
}
83-
runnerSessionId = session.sessionId;
8478
return { sessionId: session.sessionId };
8579
});
8680
/**
@@ -102,6 +96,7 @@ export function createAppleRunnerRuntime(options: {
10296
// The runner answers the target read itself; there is no detached discovery to wait on.
10397
isDiscoveryPending: () => false,
10498
targetFreshness: 'cached',
99+
sessionChanged: bridge.resetTargets,
105100
},
106101
async (device, input) =>
107102
await withAppleRunnerProvider(

‎packages/platform-apple/src/runner-snapshot-bridge.test.ts‎

Lines changed: 108 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -495,15 +495,20 @@ test('a wedged runner retires the bridge for the session across captures and app
495495
const { snapshot, fallbacks } = runtimeSnapshots(withHostStates(provider, 'notRunning'));
496496
await snapshot();
497497
state.refuseTarget = 'bridge-reader-wedged';
498-
await snapshot();
498+
const wedged = await snapshot();
499499
state.process = { pid: 77, processStartTime: '1700000100.000002' };
500-
await snapshot();
501-
await snapshot();
500+
const later = await snapshot();
501+
const last = await snapshot();
502+
for (const result of [wedged, later, last]) {
503+
expect(result.warnings).toEqual([
504+
'Simulator AX snapshot unavailable (bridge-reader-wedged); used XCTest for this runner session.',
505+
]);
506+
}
502507
expect(verbs(sent)).toEqual(['target', 'describe', 'target']);
503508
expect(fallbacks).toHaveLength(4);
504509
});
505510

506-
test('a never-returning read costs one wedged refusal after the runner bound', async () => {
511+
test('a wedged refusal retires the session after a held read', async () => {
507512
const state: {
508513
process: RunnerProcess;
509514
holdNextDescribe?: Promise<void>;
@@ -545,3 +550,102 @@ test('a replacement runner retries a generation retired by the previous runner o
545550
expect(verbs(sent)).toEqual(['target', 'describe', 'describe']);
546551
expect(presented).toHaveLength(2);
547552
});
553+
554+
test('a late presentation failure cannot retire the replacement session generation', async () => {
555+
const state = { process: { pid: 41, processStartTime: '1700000000.000001' } };
556+
const { provider, sent } = bridgeRunner(state);
557+
const bridge = createRunnerSnapshotBridge({ device: IOS_SIMULATOR, provider });
558+
let sessionId = 'first';
559+
let release!: () => void;
560+
let entered!: () => void;
561+
const blocked = new Promise<void>((resolve) => {
562+
release = resolve;
563+
});
564+
const presenting = new Promise<void>((resolve) => {
565+
entered = resolve;
566+
});
567+
let presentationCount = 0;
568+
const route = createSnapshotBridgeRoute(
569+
{
570+
snapshot: {
571+
presentIosAcquisition: async (result) => {
572+
if (++presentationCount === 1) {
573+
entered();
574+
await blocked;
575+
throw new Error('old presentation failed');
576+
}
577+
return { ...runnerResult(), producer: result.acquisition.producer };
578+
},
579+
},
580+
appleApplications: { hasLiveRunnerSession: async () => true },
581+
},
582+
{
583+
source: bridge.source,
584+
resolveTarget: bridge.resolveTarget,
585+
isDiscoveryPending: () => false,
586+
targetFreshness: 'cached',
587+
sessionChanged: bridge.resetTargets,
588+
},
589+
async () => ({ foreground: 'background', sessionId }),
590+
);
591+
const snapshot = () =>
592+
route.capture(
593+
IOS_SIMULATOR,
594+
{ options: { appBundleId: APP, interactiveOnly: true } },
595+
new AbortController().signal,
596+
async () => runnerResult(),
597+
);
598+
const old = snapshot();
599+
await presenting;
600+
sessionId = 'second';
601+
expect((await snapshot()).producer).toBe('simulator-ax-bridge');
602+
release();
603+
expect((await old).warnings).toContain(
604+
'Simulator AX snapshot unavailable (presentation-invariant); used XCTest for this app generation.',
605+
);
606+
expect((await snapshot()).producer).toBe('simulator-ax-bridge');
607+
expect(verbs(sent)).toEqual(['target', 'describe', 'target', 'describe', 'describe']);
608+
});
609+
610+
test('a wedge before the runner id is known does not disable a newly identified session', async () => {
611+
const state: { process: RunnerProcess; refuseTarget?: string } = {
612+
process: { pid: 41, processStartTime: '1700000000.000001' },
613+
refuseTarget: 'bridge-reader-wedged',
614+
};
615+
const { provider, sent } = bridgeRunner(state);
616+
const bridge = createRunnerSnapshotBridge({ device: IOS_SIMULATOR, provider });
617+
let sessionId: string | undefined = undefined;
618+
const route = createSnapshotBridgeRoute(
619+
{
620+
snapshot: {
621+
presentIosAcquisition: (result) => ({
622+
...runnerResult(),
623+
producer: result.acquisition.producer,
624+
}),
625+
},
626+
appleApplications: { hasLiveRunnerSession: async () => true },
627+
},
628+
{
629+
source: bridge.source,
630+
resolveTarget: bridge.resolveTarget,
631+
isDiscoveryPending: () => false,
632+
targetFreshness: 'cached',
633+
sessionChanged: bridge.resetTargets,
634+
},
635+
async () => (sessionId ? { foreground: 'background', sessionId } : 'background'),
636+
);
637+
const snapshot = () =>
638+
route.capture(
639+
IOS_SIMULATOR,
640+
{ options: { appBundleId: APP, interactiveOnly: true } },
641+
new AbortController().signal,
642+
async () => runnerResult(),
643+
);
644+
expect((await snapshot()).warnings).toEqual([
645+
'Simulator AX snapshot unavailable (bridge-reader-wedged); used XCTest for this runner session.',
646+
]);
647+
sessionId = 'new';
648+
state.refuseTarget = undefined;
649+
expect((await snapshot()).producer).toBe('simulator-ax-bridge');
650+
expect(verbs(sent)).toEqual(['target', 'target', 'describe']);
651+
});

‎packages/platform-apple/src/runner-snapshot-bridge.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ export type RunnerSnapshotBridge = Readonly<{
6060
export function createRunnerSnapshotBridge(
6161
options: Readonly<{ device: DeviceInfo; provider: AppleRunnerProvider }>,
6262
): RunnerSnapshotBridge {
63-
const targets = new Map<string, RunnerSnapshotBridgeTarget>();
64-
let targetEpoch = 0;
63+
let targets = new Map<string, RunnerSnapshotBridgeTarget>();
6564
// The runner admits one bridge read at a time and refuses an overlapping one at once, so it alone
6665
// owns that order: a read a cancelled capture left running costs the next capture its bridge read,
6766
// and the route answers that refusal for that capture only.
@@ -95,7 +94,7 @@ export function createRunnerSnapshotBridge(
9594
if (cached && refresh !== 'refresh') return cached;
9695
// Only a resolved target replaces the cached one: a refresh that fails leaves the generation the
9796
// route planned against.
98-
const epoch = targetEpoch;
97+
const captureTargets = targets;
9998
const requestId = randomUUID();
10099
const envelope = await send(
101100
appBundleId,
@@ -107,7 +106,7 @@ export function createRunnerSnapshotBridge(
107106
AbortSignal.any([signal, AbortSignal.timeout(TARGET_READ_TIMEOUT_MS)]),
108107
);
109108
const target = readTarget(envelope, { requestId, appBundleId, targetId });
110-
if (epoch === targetEpoch) targets.set(targetId, target);
109+
captureTargets.set(targetId, target);
111110
return target;
112111
};
113112

@@ -165,8 +164,7 @@ export function createRunnerSnapshotBridge(
165164
source,
166165
resolveTarget,
167166
resetTargets: () => {
168-
targetEpoch++;
169-
targets.clear();
167+
targets = new Map();
170168
},
171169
};
172170
}

‎packages/platform-apple/src/snapshot-route-failure-policy.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export type SurfaceHostRefusal = IosSystemSurfaceHost | 'unverified';
2727

2828
/**
2929
* The one classification a bridge failure receives before the route acts on it. `code` travels
30-
* unchanged for diagnostics; the scope says whether the generation is implicated, and a
30+
* unchanged for diagnostics; the scope identifies the capture, generation, or runner session, and a
3131
* capture-scoped verdict must name what it saw instead. A surface host refusal names the host, and
3232
* the route serves it from the runner before any fallback warning applies.
3333
*/
@@ -68,15 +68,17 @@ const CAPTURE_SCOPED_REFUSALS: ReadonlyMap<string, CaptureScopedConsequence> = n
6868
['snapshot-tree-changed', 'tree-changed'],
6969
]);
7070

71+
const RUNNER_SESSION_SCOPED: ReadonlySet<string> = new Set(['bridge-reader-wedged']);
72+
7173
/**
7274
* Whether a bridge failure is evidence that this app generation cannot be served by the bridge.
73-
* The preparation and the table's screen refusals speak for this capture only; every other failure
74-
* retires the generation. That default is conservative on purpose: `code` is an open string from
75+
* Preparation and screen refusals speak for this capture; a wedged reader retires its runner
76+
* session. Other failures retire the generation. That default is conservative on purpose: `code` is an open string from
7577
* the guest, and an unknown code must not quietly keep a generation on a path that just refused
7678
* it. Do not widen the default; classify a new known refusal in the table.
7779
*/
7880
export function bridgeFailureDisposition(failure: SnapshotSourceFailure): BridgeFailureDisposition {
79-
if (failure.code === 'bridge-reader-wedged') {
81+
if (RUNNER_SESSION_SCOPED.has(failure.code)) {
8082
return { scope: 'runner-session', code: failure.code };
8183
}
8284
if (failure.kind === 'preparing') {

0 commit comments

Comments
 (0)