node: allow one pinned actor to use isolate port scope - #7352
Closed
petebacondarwin wants to merge 2 commits into
Closed
petebacondarwin wants to merge 2 commits into
petebacondarwin wants to merge 2 commits into
Conversation
Closed
5 tasks
Contributor
|
I'm Bonk, and I've done a quick review of your PR. Adds isolate-scoped Node port tables for pinned Durable Objects. Posted 1 inline high-severity finding. |
Contributor
Author
|
Closing in favour of #7357 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#7306 correctly made Node.js virtual port tables Durable Object instance-scoped. Vite and Vitest, however, evaluate user modules inside pinned synthetic runner actors and later invoke the exported handlers from their real I/O contexts. A Node server registered during module evaluation is therefore currently invisible to
httpServerHandler()during dispatch.This adds an unsafe Durable Object namespace option whose value names the one ephemeral-local actor allowed to use its worker isolate's Node port table. The namespace compares each actor's ID before carrying that scope choice into
Worker::Actor, wherecloudflare-internal:socketsselects the isolate table.Configuration is rejected if
preventEvictionis absent, if the namespace is not ephemeral-local, or if another namespace in the same Worker already names an isolate-scope actor. All other durable and ephemeral-local actors retain their per-instance port tables.Why this is in workerd
Redirecting handler invocation back through the synthetic runner actor can make a trivial GET pass, but changes the request/response transport boundary and breaks body and streaming semantics. The runner actor is an artificial module-evaluation context, so selecting the intended port-table host at the runtime boundary preserves normal handler dispatch.
Feedback requested
This is a draft to align the runtime and tooling teams before settling the API. In particular:
unsafeUseIsolateNodePortScopeForActorthe right name and level of specificity?Restricting the option to
ephemeralLocalalso puts it behind workerd's existing--experimentalgate.Paired workers-sdk draft: cloudflare/workers-sdk#15637
Tests