From 5938f01ccc42ea9009aec64d6f0b4c897ab1bb9e Mon Sep 17 00:00:00 2001 From: Ketan Gupta Date: Mon, 24 Nov 2025 12:27:48 +0000 Subject: [PATCH] Allow null name when creating dynamic workers --- src/workerd/api/worker-loader-test.js | 16 +++++++- src/workerd/api/worker-loader.c++ | 2 +- src/workerd/api/worker-loader.h | 2 +- src/workerd/io/io-channels.h | 6 +-- src/workerd/server/server.c++ | 39 ++++++++++++------- .../experimental/index.d.ts | 2 +- .../generated-snapshot/experimental/index.ts | 2 +- types/generated-snapshot/latest/index.d.ts | 2 +- types/generated-snapshot/latest/index.ts | 2 +- 9 files changed, 47 insertions(+), 26 deletions(-) diff --git a/src/workerd/api/worker-loader-test.js b/src/workerd/api/worker-loader-test.js index 05c5299a118..b6f1610ad10 100644 --- a/src/workerd/api/worker-loader-test.js +++ b/src/workerd/api/worker-loader-test.js @@ -510,7 +510,21 @@ export let isolateUniqueness = { assert.strictEqual(await anonymousAgain.increment(), 2); assert.strictEqual(await anonymousAgain.increment(), 3); - assert.strictEqual(loadCount, 5); + // Test that null/undefined name parameter creates unique isolates each time + // Each call should generate a unique name internally. + let noName1 = env.loader.get(null, loadCodeCallback).getEntrypoint(); + let noName2 = env.loader.get(undefined, loadCodeCallback).getEntrypoint(); + let noName3 = env.loader.get(undefined, loadCodeCallback).getEntrypoint(); + + // Each should be a different isolate + assert.strictEqual(await noName1.increment(), 0); + assert.strictEqual(await noName1.increment(), 1); + assert.strictEqual(await noName2.increment(), 0); + assert.strictEqual(await noName2.increment(), 1); + assert.strictEqual(await noName3.increment(), 0); + assert.strictEqual(await noName3.increment(), 1); + + assert.strictEqual(loadCount, 8); }, }; diff --git a/src/workerd/api/worker-loader.c++ b/src/workerd/api/worker-loader.c++ index d00e88969f2..96c2caf91c2 100644 --- a/src/workerd/api/worker-loader.c++ +++ b/src/workerd/api/worker-loader.c++ @@ -59,7 +59,7 @@ jsg::Ref WorkerStub::getDurableObjectClass(jsg::Lock& js, } jsg::Ref WorkerLoader::get( - jsg::Lock& js, kj::String name, jsg::Function()> getCode) { + jsg::Lock& js, kj::Maybe name, jsg::Function()> getCode) { auto& ioctx = IoContext::current(); auto reenterAndGetCode = ioctx.makeReentryCallback( diff --git a/src/workerd/api/worker-loader.h b/src/workerd/api/worker-loader.h index 7a35c0cc444..1aab99e7399 100644 --- a/src/workerd/api/worker-loader.h +++ b/src/workerd/api/worker-loader.h @@ -125,7 +125,7 @@ class WorkerLoader: public jsg::Object { }; jsg::Ref get( - jsg::Lock& js, kj::String name, jsg::Function()> getCode); + jsg::Lock& js, kj::Maybe name, jsg::Function()> getCode); JSG_RESOURCE_TYPE(WorkerLoader) { JSG_METHOD(get); diff --git a/src/workerd/io/io-channels.h b/src/workerd/io/io-channels.h index 25739b4b73b..18e6371e7a1 100644 --- a/src/workerd/io/io-channels.h +++ b/src/workerd/io/io-channels.h @@ -245,11 +245,9 @@ class IoChannelFactory { KJ_UNIMPLEMENTED("Only implemented by single-tenant workerd runtime"); } - // Use a dynamic Worker loader binding to obtain an Worker by name. If the named Worker - // doesn't already exist, the callback will be called to fetch the source code from which the - // Worker should be created. + // Use a dynamic Worker loader binding to obtain an Worker by name. If name is null, or if the named Worker doesn't already exist, the callback will be called to fetch the source code from which the Worker should be created. virtual kj::Own loadIsolate(uint loaderChannel, - kj::String name, + kj::Maybe name, kj::Function()> fetchSource) { JSG_FAIL_REQUIRE(Error, "Dynamic worker loading is not supported by this runtime."); } diff --git a/src/workerd/server/server.c++ b/src/workerd/server/server.c++ index 5a60918869e..92581e08b41 100644 --- a/src/workerd/server/server.c++ +++ b/src/workerd/server/server.c++ @@ -3321,7 +3321,7 @@ class Server::WorkerService final: public Service, } kj::Own loadIsolate(uint loaderChannel, - kj::String name, + kj::Maybe name, kj::Function()> fetchSource) override; // --------------------------------------------------------------------------- @@ -3865,19 +3865,24 @@ class Server::WorkerLoaderNamespace: public kj::Refcounted { } kj::Own loadIsolate( - kj::String name, kj::Function()> fetchSource) { - return isolates - .findOrCreate(name, - [&]() -> decltype(isolates)::Entry { - // This name isn't actually used in any maps nor is it ever revealed back to the app, but it - // may be used in error logs. - auto isolateName = kj::str(namespaceName, ':', name); - - return {.key = kj::mv(name), - .value = kj::rc(server, kj::mv(isolateName), kj::mv(fetchSource))}; - }) - .addRef() - .toOwn(); + kj::Maybe name, kj::Function()> fetchSource) { + KJ_IF_SOME(n, name) { + return isolates + .findOrCreate(n, + [&]() -> decltype(isolates)::Entry { + // This name isn't actually used in any maps nor is it ever revealed back to the app, but it + // may be used in error logs. + auto isolateName = kj::str(namespaceName, ':', n); + + return {.key = kj::mv(n), + .value = kj::rc(server, kj::mv(isolateName), kj::mv(fetchSource))}; + }) + .addRef() + .toOwn(); + } else { + auto isolateName = kj::str(namespaceName, ":dynamic:", randomUUID(server.entropySource)); + return kj::rc(server, kj::mv(isolateName), kj::mv(fetchSource)).toOwn(); + } } private: @@ -3916,6 +3921,10 @@ class Server::WorkerLoaderNamespace: public kj::Refcounted { kj::Function()> fetchSource) : startupTask(start(server, kj::mv(isolateName), kj::mv(fetchSource)).fork()) {} + ~WorkerStubImpl() { + unlink(); + } + void unlink() { KJ_IF_SOME(s, service) { s->unlink(); @@ -4147,7 +4156,7 @@ void Server::unlinkWorkerLoaders() { } kj::Own Server::WorkerService::loadIsolate(uint loaderChannel, - kj::String name, + kj::Maybe name, kj::Function()> fetchSource) { auto& channels = KJ_REQUIRE_NONNULL(ioChannels.tryGet(), "link() has not been called"); diff --git a/types/generated-snapshot/experimental/index.d.ts b/types/generated-snapshot/experimental/index.d.ts index 3ac4cbba3ee..adfc8a75147 100755 --- a/types/generated-snapshot/experimental/index.d.ts +++ b/types/generated-snapshot/experimental/index.d.ts @@ -4112,7 +4112,7 @@ interface WorkerStubEntrypointOptions { } interface WorkerLoader { get( - name: string, + name: string | null, getCode: () => WorkerLoaderWorkerCode | Promise, ): WorkerStub; } diff --git a/types/generated-snapshot/experimental/index.ts b/types/generated-snapshot/experimental/index.ts index 9bea6dae435..72f498c1667 100755 --- a/types/generated-snapshot/experimental/index.ts +++ b/types/generated-snapshot/experimental/index.ts @@ -4125,7 +4125,7 @@ export interface WorkerStubEntrypointOptions { } export interface WorkerLoader { get( - name: string, + name: string | null, getCode: () => WorkerLoaderWorkerCode | Promise, ): WorkerStub; } diff --git a/types/generated-snapshot/latest/index.d.ts b/types/generated-snapshot/latest/index.d.ts index f5e3fba3913..cd0058efd9b 100755 --- a/types/generated-snapshot/latest/index.d.ts +++ b/types/generated-snapshot/latest/index.d.ts @@ -3832,7 +3832,7 @@ interface WorkerStubEntrypointOptions { } interface WorkerLoader { get( - name: string, + name: string | null, getCode: () => WorkerLoaderWorkerCode | Promise, ): WorkerStub; } diff --git a/types/generated-snapshot/latest/index.ts b/types/generated-snapshot/latest/index.ts index 97f72b8e135..7ed0c189006 100755 --- a/types/generated-snapshot/latest/index.ts +++ b/types/generated-snapshot/latest/index.ts @@ -3843,7 +3843,7 @@ export interface WorkerStubEntrypointOptions { } export interface WorkerLoader { get( - name: string, + name: string | null, getCode: () => WorkerLoaderWorkerCode | Promise, ): WorkerStub; }