Skip to content

Commit 0f58d95

Browse files
committed
fix(provider-webdriver): allocate one WebDriver session per lease
Two concurrent lease_allocate calls for the same lease id each rented a billed cloud session: the reuse check ran after an await, so neither call could see the other's session, and only the last one published was ever releasable. A release or shutdown that landed while a create was in flight also found nothing to dispose. Publish the lease's entry before the first await so a duplicate allocation joins the in-flight create instead of starting its own. Model the entry as a discriminated union: allocating until the hub answers, then ready. Release and shutdown take the entry out of the map in one synchronous step, so whoever holds it disposes the session exactly once, including a session that finishes renting after the lease is already gone.
1 parent f7879cf commit 0f58d95

2 files changed

Lines changed: 503 additions & 58 deletions

File tree

Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,262 @@
1+
import assert from 'node:assert/strict';
2+
import { afterEach, test } from 'vitest';
3+
import type { DeviceLease } from '@agent-device/contracts/device';
4+
import { AppError } from '@agent-device/kernel/errors';
5+
import { createCloudWebDriverRuntime, type CloudWebDriverRuntimeOptions } from './runtime.ts';
6+
import { consumeAllProfileFields } from './profile-fields.fixtures.ts';
7+
8+
const realFetch = globalThis.fetch;
9+
10+
afterEach(() => {
11+
globalThis.fetch = realFetch;
12+
});
13+
14+
/**
15+
* `POST /session` is billed and the lease is the only handle that can stop it again. Two allocations
16+
* that overlapped both passed the live-session check, so the second hub session overwrote the first
17+
* in the manager's map and only one of the two was ever releasable.
18+
*/
19+
test('two concurrent allocations of one lease create one billed session', async () => {
20+
const hub = startWebDriverHub();
21+
globalThis.fetch = hub.fetch;
22+
const provider = holdProviderPreparation();
23+
const runtime = makeRuntime({ prepareSession: provider.prepareSession });
24+
const lease = makeLease();
25+
26+
try {
27+
const first = settle(() => runtime.leaseLifecycle.allocate!(lease));
28+
await provider.waitForPreparation();
29+
const second = settle(() => runtime.leaseLifecycle.allocate!(lease));
30+
// One scheduler boundary while the first allocation is still held before the hub: the second has
31+
// then reached the lease, and the first cannot have published its session yet.
32+
await schedulerYield();
33+
provider.answerPreparation();
34+
35+
const [firstOutcome, secondOutcome] = await Promise.all([first, second]);
36+
assert.equal(hub.createdSessionIds.length, 1, 'one lease bills one session');
37+
assertResolvedWith(firstOutcome, 'session-1');
38+
assertResolvedWith(secondOutcome, 'session-1');
39+
40+
await runtime.leaseLifecycle.release!(lease);
41+
assert.deepEqual(hub.deletedSessionIds, hub.createdSessionIds);
42+
assert.equal(provider.cleanups, 1);
43+
} finally {
44+
provider.answerPreparation();
45+
await runtime.shutdown();
46+
}
47+
});
48+
49+
/**
50+
* A release that landed mid-creation found no published session and answered that the lease held
51+
* nothing, so the hub session that creation went on to make had no route left to stop it. The caller
52+
* whose session was disposed is refused, rather than handed a session id nothing will release.
53+
*/
54+
test('a release during creation disposes the session that creation makes', async () => {
55+
const hub = startWebDriverHub();
56+
globalThis.fetch = hub.fetch;
57+
const provider = holdProviderPreparation();
58+
const runtime = makeRuntime({ prepareSession: provider.prepareSession });
59+
const lease = makeLease();
60+
61+
try {
62+
const allocating = settle(() => runtime.leaseLifecycle.allocate!(lease));
63+
await provider.waitForPreparation();
64+
// The release takes the lease's entry in its first synchronous step, while the create is held.
65+
const releasing = settle(() => runtime.leaseLifecycle.release!(lease));
66+
provider.answerPreparation();
67+
68+
assert.deepEqual(await releasing, {
69+
status: 'resolved',
70+
value: { provider: 'webdriver-test', providerSessionId: 'session-1' },
71+
});
72+
assert.deepEqual(hub.deletedSessionIds, ['session-1']);
73+
assert.equal(provider.cleanups, 1);
74+
assertRejectedWith(
75+
await allocating,
76+
'provider_session_released_during_allocation',
77+
lease.leaseId,
78+
);
79+
} finally {
80+
provider.answerPreparation();
81+
await runtime.shutdown();
82+
}
83+
});
84+
85+
/** The same race on the route that ends every session this process holds. */
86+
test('shutdown during creation disposes the session that creation makes', async () => {
87+
const hub = startWebDriverHub();
88+
globalThis.fetch = hub.fetch;
89+
const provider = holdProviderPreparation();
90+
const runtime = makeRuntime({ prepareSession: provider.prepareSession });
91+
92+
try {
93+
const allocating = settle(() => runtime.leaseLifecycle.allocate!(makeLease()));
94+
await provider.waitForPreparation();
95+
// Shutdown takes every entry in its first synchronous step, while the create is held.
96+
const shuttingDown = settle(() => runtime.shutdown());
97+
provider.answerPreparation();
98+
99+
assert.deepEqual(await shuttingDown, { status: 'resolved', value: undefined });
100+
assert.deepEqual(hub.deletedSessionIds, ['session-1']);
101+
assert.equal(provider.cleanups, 1);
102+
assertRejectedWith(await allocating, 'provider_session_shutdown_during_allocation');
103+
} finally {
104+
provider.answerPreparation();
105+
await runtime.shutdown();
106+
}
107+
});
108+
109+
/**
110+
* The hub answers every WebDriver request and records the session ids it was asked to create and
111+
* delete, so a test's lever over an in-flight create is {@link holdProviderPreparation}.
112+
*/
113+
function startWebDriverHub() {
114+
const createdSessionIds: string[] = [];
115+
const deletedSessionIds: string[] = [];
116+
return {
117+
createdSessionIds,
118+
deletedSessionIds,
119+
fetch: (async (input: unknown, init?: RequestInit) => {
120+
const request = new Request(input as RequestInfo, init);
121+
const path = new URL(request.url).pathname;
122+
if (request.method === 'POST' && path.endsWith('/session')) {
123+
const sessionId = `session-${String(createdSessionIds.length + 1)}`;
124+
createdSessionIds.push(sessionId);
125+
return jsonResponse({ sessionId, capabilities: {} });
126+
}
127+
if (request.method === 'DELETE') {
128+
deletedSessionIds.push(decodeURIComponent(path.split('/').pop()!));
129+
}
130+
return jsonResponse({ value: null });
131+
}) as typeof fetch,
132+
};
133+
}
134+
135+
/**
136+
* Holds `prepareSession` — the step that runs before this manager sends `POST /session` — until the
137+
* test answers it, so a create stays in flight across a release or a shutdown instead of the test
138+
* racing one with a sleep. It also counts the provider cleanup a prepared session owes, which is the
139+
* half of disposal the hub's `DELETE` does not cover.
140+
*/
141+
function holdProviderPreparation() {
142+
const state = { prepared: 0, cleanups: 0 };
143+
const arrivals: (() => void)[] = [];
144+
let settleGate: () => void = () => {};
145+
const gate = new Promise<void>((settle) => {
146+
settleGate = settle;
147+
});
148+
const prepareSession: NonNullable<CloudWebDriverRuntimeOptions['prepareSession']> = async ({
149+
base,
150+
}) => {
151+
state.prepared += 1;
152+
for (const arrived of arrivals.splice(0)) arrived();
153+
await gate;
154+
return {
155+
...base,
156+
cleanup: async () => {
157+
state.cleanups += 1;
158+
return undefined;
159+
},
160+
};
161+
};
162+
return Object.assign(state, {
163+
prepareSession,
164+
/** Resolves once an allocation has reached `prepareSession` and is held there. */
165+
waitForPreparation: async () => {
166+
while (state.prepared < 1) {
167+
await new Promise<void>((arrived) => {
168+
arrivals.push(arrived);
169+
});
170+
}
171+
},
172+
answerPreparation: () => {
173+
settleGate();
174+
},
175+
});
176+
}
177+
178+
/** How one lease-lifecycle call ended, so a test can assert disposal before the caller's outcome. */
179+
type Settled<T> =
180+
| Readonly<{ status: 'resolved'; value: T }>
181+
| Readonly<{ status: 'rejected'; error: unknown }>;
182+
183+
/** Runs a call and turns its outcome into a value, so no rejection goes unobserved. */
184+
async function settle<T>(run: () => Promise<T>): Promise<Settled<T>> {
185+
try {
186+
return { status: 'resolved', value: await run() };
187+
} catch (error) {
188+
return { status: 'rejected', error };
189+
}
190+
}
191+
192+
/** Yields one macrotask, so every promise the test has started runs the microtasks it already queued. */
193+
async function schedulerYield(): Promise<void> {
194+
await new Promise<void>((resolve) => {
195+
setTimeout(resolve, 0);
196+
});
197+
}
198+
199+
/** The allocation envelope names the hub session the lease now holds. */
200+
function assertResolvedWith(
201+
outcome: Settled<Record<string, unknown> | undefined>,
202+
sessionId: string,
203+
): void {
204+
assert.equal(outcome.status, 'resolved', `expected ${sessionId}, got ${describe(outcome)}`);
205+
assert.equal(
206+
(outcome as Readonly<{ value: Record<string, unknown> | undefined }>).value?.sessionId,
207+
sessionId,
208+
);
209+
}
210+
211+
/**
212+
* The typed reason an allocation reports when the session it made had already been taken to be
213+
* disposed, and the lease it was refused for.
214+
*/
215+
function assertRejectedWith(outcome: Settled<unknown>, reason: string, leaseId?: string): void {
216+
assert.equal(outcome.status, 'rejected', `expected ${reason}, got ${describe(outcome)}`);
217+
const error = (outcome as Readonly<{ error: unknown }>).error;
218+
assert.ok(error instanceof AppError, `expected an AppError, got ${String(error)}`);
219+
assert.equal(error.details?.reason, reason);
220+
if (leaseId !== undefined) assert.equal(error.details?.leaseId, leaseId);
221+
}
222+
223+
function describe(outcome: Settled<unknown>): string {
224+
return outcome.status === 'rejected'
225+
? `rejection: ${String(outcome.error)}`
226+
: `resolution: ${JSON.stringify(outcome.value)}`;
227+
}
228+
229+
function makeRuntime(overrides: Partial<CloudWebDriverRuntimeOptions> = {}) {
230+
return createCloudWebDriverRuntime({
231+
clientVersion: 'test',
232+
provider: 'webdriver-test',
233+
endpoint: 'https://webdriver.test/wd/hub/',
234+
platform: 'android',
235+
deviceName: 'Test device',
236+
profileFields: consumeAllProfileFields('webdriver-test'),
237+
requestPolicy: { retryAttempts: 0 },
238+
...overrides,
239+
});
240+
}
241+
242+
function jsonResponse(body: unknown, status = 200): Response {
243+
return new Response(JSON.stringify(body), {
244+
status,
245+
headers: { 'Content-Type': 'application/json' },
246+
});
247+
}
248+
249+
function makeLease(): DeviceLease {
250+
return {
251+
leaseId: 'lease-1',
252+
tenantId: 'team-a',
253+
runId: 'run-a',
254+
clientId: 'client-a',
255+
leaseProvider: 'webdriver-test',
256+
backend: 'android-instance',
257+
deviceKey: 'webdriver-test:device-a',
258+
createdAt: 1,
259+
expiresAt: 2,
260+
heartbeatAt: 1,
261+
};
262+
}

0 commit comments

Comments
 (0)