Skip to content

Commit 32c7859

Browse files
sirtimidclaude
andcommitted
fix(kernel-browser-runtime): unblock drain so reentrant RPC doesn't deadlock
The drain loop awaited each request handler before pulling the next message, including responses. That serialization deadlocked any request handler that fired an outbound RPC back to the other side and awaited its response — the response could not be processed by the drain until the request handler returned, but the request handler was waiting for the response. This bit the new `onIncarnationChange` callback the transport invokes during the handshake: kernel-worker sends `sendRemoteMessage` → offscreen runs the handler → handshake calls `onIncarnationChange` RPC back to kernel-worker → kernel-worker returns a verdict → response arrives at offscreen but its drain is still awaiting the original request handler. Hung until the 40s URL redemption timeout. Fix: dispatch request handlers in the background (no await inside the drain). Responses still process synchronously. Apply on both sides since either drain can hit the same shape. The new `onIncarnationChange` round-trip during every handshake makes this latent issue actually surface in the e2e — the original asymmetry (kernel-worker drain rarely blocked, offscreen drain frequently blocked while sendRemoteMessage was in flight) is what made it hide for so long. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 812f84e commit 32c7859

2 files changed

Lines changed: 65 additions & 33 deletions

File tree

packages/kernel-browser-runtime/src/PlatformServicesClient.ts

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import type {
2525
PostMessageTarget,
2626
} from '@metamask/streams/browser';
2727
import { isJsonRpcResponse, isJsonRpcRequest } from '@metamask/utils';
28+
import type { JsonRpcRequest } from '@metamask/utils';
2829
import type { JsonRpcId } from '@metamask/utils';
2930

3031
// Appears in the docs.
@@ -415,22 +416,34 @@ export class PlatformServicesClient implements PlatformServices {
415416

416417
this.#rpcClient.handleResponse(id, event.data);
417418
} else if (isJsonRpcRequest(event.data)) {
418-
const { id, method, params } = event.data;
419-
try {
420-
this.#rpcServer.assertHasMethod(method);
421-
const result = await this.#rpcServer.execute(method, params);
422-
await this.#sendMessage({
423-
id,
424-
result,
425-
jsonrpc: '2.0',
426-
});
427-
} catch (error) {
428-
await this.#sendMessage({
429-
id,
430-
error: serializeError(error),
431-
jsonrpc: '2.0',
432-
});
433-
}
419+
// Run the request handler in the background instead of awaiting it
420+
// inside the drain. The drain processes responses too, and a request
421+
// handler that fires an outbound RPC back to the other side would
422+
// deadlock waiting for its response — the drain can't get to that
423+
// response until the request handler returns.
424+
this.#executeRequest(event.data).catch(() => undefined);
425+
}
426+
}
427+
428+
/**
429+
* Execute a JSON-RPC request and write the response back. Errors during
430+
* execution are serialized into a JSON-RPC error response; errors during
431+
* response delivery are swallowed.
432+
*
433+
* @param request - The JSON-RPC request to execute.
434+
*/
435+
async #executeRequest(request: JsonRpcRequest): Promise<void> {
436+
const { id, method, params } = request;
437+
try {
438+
this.#rpcServer.assertHasMethod(method);
439+
const result = await this.#rpcServer.execute(method, params);
440+
await this.#sendMessage({ id, result, jsonrpc: '2.0' });
441+
} catch (error) {
442+
await this.#sendMessage({
443+
id,
444+
error: serializeError(error),
445+
jsonrpc: '2.0',
446+
}).catch(() => undefined);
434447
}
435448
}
436449
}

packages/kernel-browser-runtime/src/PlatformServicesServer.ts

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import type {
2525
PostMessageTarget,
2626
} from '@metamask/streams/browser';
2727
import { isJsonRpcRequest, isJsonRpcResponse } from '@metamask/utils';
28+
import type { JsonRpcRequest } from '@metamask/utils';
2829

2930
// Appears in the docs.
3031
// eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -179,23 +180,41 @@ export class PlatformServicesServer {
179180
const message = event.data;
180181
this.#rpcClient.handleResponse(message.id as string, message);
181182
} else if (isJsonRpcRequest(event.data)) {
182-
const { id, method, params } = event.data;
183-
try {
184-
this.#rpcServer.assertHasMethod(method);
185-
// Ridiculous cast to bypass TypeScript vs. JsonRpc tug-o-war
186-
const port: MessagePort | undefined = (await this.#rpcServer.execute(
187-
method,
188-
params,
189-
)) as unknown as MessagePort | undefined;
190-
await this.#sendMessage({ id, result: null, jsonrpc: '2.0' }, port);
191-
} catch (error) {
192-
this.#logger.error(`Error handling "${method}" request:`, error);
193-
this.#sendMessage({
194-
id,
195-
error: serializeError(error),
196-
jsonrpc: '2.0',
197-
}).catch(() => undefined);
198-
}
183+
// Run the request handler in the background instead of awaiting it
184+
// inside the drain. The drain processes responses too, and a request
185+
// handler that fires an outbound RPC back to the other side (e.g.
186+
// transport.sendRemoteMessage's handshake calling onIncarnationChange)
187+
// would deadlock waiting for its response — the drain can't get to
188+
// that response until the request handler returns.
189+
this.#executeRequest(event.data).catch(() => undefined);
190+
}
191+
}
192+
193+
/**
194+
* Execute a JSON-RPC request and write the response back. Errors during
195+
* execution are serialized into a JSON-RPC error response; errors during
196+
* response delivery are logged and swallowed (the caller has nowhere to
197+
* surface them).
198+
*
199+
* @param request - The JSON-RPC request to execute.
200+
*/
201+
async #executeRequest(request: JsonRpcRequest): Promise<void> {
202+
const { id, method, params } = request;
203+
try {
204+
this.#rpcServer.assertHasMethod(method);
205+
// Ridiculous cast to bypass TypeScript vs. JsonRpc tug-o-war
206+
const port: MessagePort | undefined = (await this.#rpcServer.execute(
207+
method,
208+
params,
209+
)) as unknown as MessagePort | undefined;
210+
await this.#sendMessage({ id, result: null, jsonrpc: '2.0' }, port);
211+
} catch (error) {
212+
this.#logger.error(`Error handling "${method}" request:`, error);
213+
this.#sendMessage({
214+
id,
215+
error: serializeError(error),
216+
jsonrpc: '2.0',
217+
}).catch(() => undefined);
199218
}
200219
}
201220

0 commit comments

Comments
 (0)