Repository navigation
fix(runtime): instanceof default @@hasInstance and Readable.fromWeb on non-web operand (Next.js /list hang) - #13720
Merged
Merged
Conversation
Next.js /list hung for two chained reasons. instanceof: for a linked right-hand side the default Function.prototype [@@hasInstance] was recognized by comparing bits against a fresh, allocating read, so a GC move made the compare fail and the default thunk ran with an undefined receiver (always false). The default is now recognized by its code, and other hooks are called with the constructor as this. Readable/Writable.fromWeb on a non-web operand fell back to the generic from_web, which routed a Node Readable into duplex_from_web and back, looping forever. They now throw ERR_INVALID_ARG_TYPE like node, and duplex_from_web throws when neither side installs.
|
Caution Review failedThe pull request is closed.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
The hook read and call sat inline in js_instanceof_dynamic. Removing the bits compare in the previous commit changed register allocation in that large function, so every ordinary instanceof paid about 19 instructions more (fastify startup +154 Ir). dispatch_own_has_instance now takes the hook as a value or as the key to read it with, roots and refreshes both operands across that read, and is cold and out of line. The ordinary dispatch no longer carries the handle scope or the getter call, and the runtime keeps one hook function as before.
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.
Next.js 16 App Router standalone (
secret-tests/nextjs-hello): on main,GET /list(a force-dynamic server component) never answers. The server spins at 100% CPU, with the profile dominated by GC work (oldgen evacuation snapshot, conservative pinning, ValidPointerSetBuilder). The regression is in eea3ac6..30e6cba: the same app on eea3ac6 served/listidentically to node.Causes
instanceofmisses the default@@hasInstance. Since fix(runtime): the shape is the one record of [[Prototype]] (null links, function links) #13666,js_instanceof_dynamic(object/instanceof/dynamic_dispatch.rs) recognized the defaultFunction.prototype[@@hasInstance]by comparing the hook value against a fresh read of that hook. The read can allocate, and the comparison can miss. On a miss, the hook ran withthis= undefined and returned false. So Next'se instanceof Readable ? e : Readable.fromWeb(e)sent a real nodeReadableintofromWeb. A userstatic [Symbol.hasInstance]also ran withoutthis === C.Readable.fromWebloops on a non-web operand.js_node_stream_from_webandjs_node_stream_duplex_from_web(node_stream_constructors/web_adapter.rs) bounced between each other forever, minting a duplex per turn that died immediately. That is the GC-dominated profile, with RSS flat at ~308 MB. Node throwsERR_INVALID_ARG_TYPE.Fix
@@hasInstanceis recognized by its code, not its value.this.Readable/Writable/Duplex.fromWebthrow node'sERR_INVALID_ARG_TYPETypeError for a non-web-stream operand, instead of recursing.Tests
test-files/test_gap_stream_from_web_non_web_operand.ts: fixed == node; main times out (rc=124).Verification (qb9, base 1cb8838, main arm 6725177)
instanceofchecks (Readable subclass, Function/Array/Object/Error, user class)static [Symbol.hasInstance]withthis === Ctrue falsefalse falseReadable.fromWeb(new PassThrough())Readable.fromWeb(web ReadableStream)a, end/list/,/nope/(5000 requests)The base predates #13695, so the fixture's API routes (500) and
varydiffer in that run. Both are fixed on main by #13695 and are independent of this change.rustfmt: touched hunks are clean. A pre-existing diff at
static_dispatch.rs:303is also on the base.