Skip to content

Commit 3cad02c

Browse files
committed
Fix Durable Object ordering when mixing RPC and fetch calls
When a caller fires interleaved stub.rpc() and stub.fetch() calls on the same DO stub without awaiting, all fetch calls were processed before all RPC calls, regardless of send order. This violated the expected E-order guarantee for actors. Root cause: fetch reaches the InputGate in ~1 async hop (via WorkerEntrypoint::request() -> context.run() -> InputGate::wait()), while RPC takes ~4+ hops (customEvent -> JsRpcSessionCustomEvent::run() -> Cap'n Proto session setup -> capability fulfillment -> pipelined call dispatch -> JsRpcTargetBase::call() -> kj::yield() -> ctx.run() -> InputGate::wait()). Since all operations originate from the same synchronous JS execution, fetch calls always reached the FIFO queue before any RPC calls. Fix: eagerly acquire the InputGate position in JsRpcSessionCustomEvent::run() — at the same level where WorkerEntrypoint::request() acquires it — then thread the lock through to the first ctx.run() call via the existing IoContext::run(func, Maybe<InputGate::Lock>) overload. The lock is consumed by the first RPC method invocation and is NOT held for the session lifetime. The kj::yield() in JsRpcTargetBase::call() and ExternalPusher ordering are unaffected. Fixes #6561 Made-with: Cursor
1 parent d9cfdd0 commit 3cad02c

4 files changed

Lines changed: 113 additions & 1 deletion

File tree

src/workerd/api/tests/js-rpc-socket-test.wd-test

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const unitTests :Workerd.Config = (
2929
(name = "MyServiceProxy", service = "MyServiceProxy-loop"),
3030
(name = "MyActor", durableObjectNamespace = "MyActor"),
3131
(name = "ActorNoExtends", durableObjectNamespace = "ActorNoExtends"),
32+
(name = "OrderingActor", durableObjectNamespace = "OrderingActor"),
3233
(name = "defaultExport", service = "default-loop"),
3334
(name = "twelve", json = "12"),
3435
(name = "GreeterFactory", service = "GreeterFactory-loop"),
@@ -37,6 +38,7 @@ const unitTests :Workerd.Config = (
3738
durableObjectNamespaces = [
3839
(className = "MyActor", uniqueKey = "foo"),
3940
(className = "ActorNoExtends", uniqueKey = "bar"),
41+
(className = "OrderingActor", uniqueKey = "ordering"),
4042
],
4143

4244
durableObjectStorage = (inMemory = void),

src/workerd/api/tests/js-rpc-test.js

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,25 @@ export class ActorNoExtends {
586586
}
587587
}
588588

589+
// DO used to test that mixed RPC+fetch calls preserve send order.
590+
export class OrderingActor extends DurableObject {
591+
#log = [];
592+
593+
record(input) {
594+
this.#log.push(input);
595+
}
596+
597+
async fetch(request) {
598+
let url = new URL(request.url);
599+
this.#log.push(url.pathname);
600+
return new Response('ok');
601+
}
602+
603+
getLog() {
604+
return this.#log;
605+
}
606+
}
607+
589608
export default class DefaultService extends WorkerEntrypoint {
590609
async fetch(req) {
591610
// Test this.env here just to prove omitting the constructor entirely works.
@@ -2117,3 +2136,67 @@ export let eOrderTest = {
21172136
assert.deepEqual(results, [1, 2, 3, 4, 5, 6]);
21182137
},
21192138
};
2139+
2140+
// Verify that interleaved RPC and fetch calls on a DO stub are delivered in send order.
2141+
export let mixedRpcFetchOrdering = {
2142+
async test(controller, env, ctx) {
2143+
let id = env.OrderingActor.idFromName('mixed');
2144+
let stub = env.OrderingActor.get(id);
2145+
2146+
let promises = [];
2147+
let expected = [];
2148+
for (let i = 0; i < 20; i++) {
2149+
if (i % 2 === 0) {
2150+
promises.push(stub.record(`rpc-${i}`));
2151+
expected.push(`rpc-${i}`);
2152+
} else {
2153+
promises.push(stub.fetch(`http://x/fetch-${i}`));
2154+
expected.push(`/fetch-${i}`);
2155+
}
2156+
}
2157+
await Promise.all(promises);
2158+
2159+
let log = await stub.getLog();
2160+
assert.deepEqual(log, expected);
2161+
},
2162+
};
2163+
2164+
// Verify that pure RPC calls on a DO stub preserve send order.
2165+
export let pureRpcOrdering = {
2166+
async test(controller, env, ctx) {
2167+
let id = env.OrderingActor.idFromName('pure-rpc');
2168+
let stub = env.OrderingActor.get(id);
2169+
2170+
let promises = [];
2171+
for (let i = 0; i < 20; i++) {
2172+
promises.push(stub.record(`call-${i}`));
2173+
}
2174+
await Promise.all(promises);
2175+
2176+
let log = await stub.getLog();
2177+
assert.deepEqual(
2178+
log,
2179+
Array.from({ length: 20 }, (_, i) => `call-${i}`)
2180+
);
2181+
},
2182+
};
2183+
2184+
// Verify that pure fetch calls on a DO stub preserve send order.
2185+
export let pureFetchOrdering = {
2186+
async test(controller, env, ctx) {
2187+
let id = env.OrderingActor.idFromName('pure-fetch');
2188+
let stub = env.OrderingActor.get(id);
2189+
2190+
let promises = [];
2191+
for (let i = 0; i < 20; i++) {
2192+
promises.push(stub.fetch(`http://x/${i}`));
2193+
}
2194+
await Promise.all(promises);
2195+
2196+
let log = await stub.getLog();
2197+
assert.deepEqual(
2198+
log,
2199+
Array.from({ length: 20 }, (_, i) => `/${i}`)
2200+
);
2201+
},
2202+
};

src/workerd/api/tests/js-rpc-test.wd-test

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const unitTests :Workerd.Config = (
2525
(name = "MyServiceProxy", service = (name = "js-rpc-test", entrypoint = "MyServiceProxy")),
2626
(name = "MyActor", durableObjectNamespace = "MyActor"),
2727
(name = "ActorNoExtends", durableObjectNamespace = "ActorNoExtends"),
28+
(name = "OrderingActor", durableObjectNamespace = "OrderingActor"),
2829
(name = "defaultExport", service = "js-rpc-test"),
2930
(name = "twelve", json = "12"),
3031
(name = "GreeterFactory", service = (name = "js-rpc-test", entrypoint = "GreeterFactory")),
@@ -33,6 +34,7 @@ const unitTests :Workerd.Config = (
3334
durableObjectNamespaces = [
3435
(className = "MyActor", uniqueKey = "foo"),
3536
(className = "ActorNoExtends", uniqueKey = "bar"),
37+
(className = "OrderingActor", uniqueKey = "ordering"),
3638
],
3739

3840
durableObjectStorage = (inMemory = void),

src/workerd/api/worker-rpc.c++

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1000,9 +1000,14 @@ class JsRpcTargetBase: public rpc::JsRpcTarget::Server {
10001000
: enterIsolateAndCall([this, &ctx](CallContext callContext) {
10011001
// Note: No need to topUpActor() since this is the start of a top-level request, so the
10021002
// actor will already have been topped up by IncomingRequest::delivered().
1003+
//
1004+
// If a pre-acquired InputGate lock is available (set by JsRpcSessionCustomEvent::run()
1005+
// to preserve ordering with other event types like fetch), consume it here so that
1006+
// ctx.run() skips InputGate::wait() and uses the already-reserved position.
1007+
auto inputLock = kj::mv(preAcquiredInputLock);
10031008
return ctx.run([this, &ctx, callContext](Worker::Lock& lock) mutable {
10041009
return callImpl(lock, ctx, callContext);
1005-
});
1010+
}, kj::mv(inputLock));
10061011
}),
10071012
externalPusher(ctx.getExternalPusher()) {}
10081013

@@ -1057,6 +1062,12 @@ class JsRpcTargetBase: public rpc::JsRpcTarget::Server {
10571062
return externalPusher->pushAbortSignal(context);
10581063
}
10591064

1065+
// Pre-acquired InputGate lock for the first RPC call in a session. When set by
1066+
// JsRpcSessionCustomEvent::run(), this ensures the RPC call's position in the InputGate
1067+
// FIFO matches its arrival order relative to other event types (fetch, connect, etc.).
1068+
// Consumed on first use by enterIsolateAndCall; subsequent calls acquire normally.
1069+
kj::Maybe<InputGate::Lock> preAcquiredInputLock;
1070+
10601071
KJ_DISALLOW_COPY_AND_MOVE(JsRpcTargetBase);
10611072

10621073
private:
@@ -2174,6 +2185,19 @@ kj::Promise<WorkerInterface::CustomEvent::Result> JsRpcSessionCustomEvent::run(
21742185

21752186
incomingRequest->delivered();
21762187

2188+
// For actors, eagerly reserve our position in the InputGate FIFO. Without this, fetch calls
2189+
// reach the InputGate in ~1 async hop (via WorkerEntrypoint::request() -> context.run()) while
2190+
// RPC takes ~4+ hops (session setup, Cap'n Proto dispatch, kj::yield()). By acquiring the lock
2191+
// here — at the same level as request() — we ensure mixed RPC+fetch calls are ordered by
2192+
// arrival time, not by how many async hops each code path takes.
2193+
//
2194+
// The lock is passed to EntrypointJsRpcTarget and consumed by the first ctx.run() call,
2195+
// so it is NOT held for the session lifetime.
2196+
kj::Maybe<InputGate::Lock> inputLock;
2197+
KJ_IF_SOME(a, ioctx.getActor()) {
2198+
inputLock = co_await a.getInputGate().wait(ioctx.getCurrentTraceSpan());
2199+
}
2200+
21772201
KJ_DEFER({
21782202
// waitUntil() should allow extending execution on the server side even when the client
21792203
// disconnects.
@@ -2182,6 +2206,7 @@ kj::Promise<WorkerInterface::CustomEvent::Result> JsRpcSessionCustomEvent::run(
21822206

21832207
EntrypointJsRpcTarget target(ioctx, entrypointName, kj::mv(versionInfo), kj::mv(props),
21842208
kj::mv(wrapperModule), mapAddRef(incomingRequest->getWorkerTracer()), isDynamicDispatch);
2209+
target.preAcquiredInputLock = kj::mv(inputLock);
21852210
capnp::RevocableServer<rpc::JsRpcTarget> revcableTarget(target);
21862211

21872212
try {

0 commit comments

Comments
 (0)