fix(runtime): instanceof default @@hasInstance and Readable.fromWeb on non-web operand (Next.js /list hang)

crates/perry-runtime/src/node_stream_constructors/web_adapter.rs

@@ -1105,23 +1105,38 @@ pub extern "C" fn js_node_stream_duplex_to_web(node_stream: f64) -> f64 {
     }
 }
 
+/// node's `ERR_INVALID_ARG_TYPE` for a `fromWeb` operand that is not the web
+/// stream it names: `what` is `"readableStream" argument` or
+/// `"pair.readable" property`.
+fn throw_not_web_stream(what: &str, class: &str, value: f64) -> ! {
+    let message = format!(
+        "The {what} must be an instance of {class}. Received {}",
+        crate::fs::validate::describe_received(value)
+    );
+    crate::fs::validate::throw_type_error_with_code(&message, "ERR_INVALID_ARG_TYPE")
+}
+
+/// `Readable.fromWeb(readableStream, options)`. An operand without a web
+/// reader is not a `ReadableStream`: a TypeError, as in node.
 #[no_mangle]
 pub extern "C" fn js_node_stream_readable_from_web(web_stream: f64, opts: f64) -> f64 {
     let readable = js_node_stream_readable_new(readable_from_options(opts));
     if install_web_readable_adapter(readable, web_stream) {
         readable
     } else {
-        js_node_stream_from_web(web_stream)
+        throw_not_web_stream("\"readableStream\" argument", "ReadableStream", web_stream)
     }
 }
 
+/// `Writable.fromWeb(writableStream, options)`; an operand without a web
+/// writer is a TypeError, as in node.
 #[no_mangle]
 pub extern "C" fn js_node_stream_writable_from_web(web_stream: f64, opts: f64) -> f64 {
     let writable = js_node_stream_writable_new(opts);
     if install_web_writable_adapter(writable, web_stream) {
         writable
     } else {
-        js_node_stream_from_web(web_stream)
+        throw_not_web_stream("\"writableStream\" argument", "WritableStream", web_stream)
     }
 }
 
@@ -1140,7 +1155,10 @@ pub extern "C" fn js_node_stream_duplex_from_web(pair: f64, opts: f64) -> f64 {
     if readable_ok || writable_ok {
         duplex
     } else {
-        js_node_stream_from_web(pair)
+        // Neither side is a web stream. This is the end of the generic
+        // `fromWeb` too (it routes every `{ readable, writable }` here), so
+        // falling back to it would come straight back.
+        throw_not_web_stream("\"pair.readable\" property", "ReadableStream", readable_web)
     }
 }
 

crates/perry-runtime/src/object/instanceof.rs

@@ -602,24 +602,68 @@ enum HasInstanceOutcome {
     Fallthrough,
 }
 
+/// C's `@@hasInstance`: the value already resolved by the caller, or read
+/// here from C with the given `@@hasInstance` key.
+#[derive(Clone, Copy)]
+enum OwnHook {
+    Value(f64),
+    Read(f64),
+}
+
 /// Spec `InstanceofOperator` / `GetMethod(C, @@hasInstance)`: a `null`/`undefined`
 /// value means "no hook" (→ ordinary `instanceof`), but a present **non-callable**
-/// value is a `TypeError` rather than a silent fall-through. `cb` is the already
-/// resolved OWN `@@hasInstance` value; this never resolves the inherited
-/// `Function.prototype` default thunk, so there is no `instanceof` recursion.
-fn dispatch_own_has_instance(cb: f64, value: f64) -> HasInstanceOutcome {
+/// value is a `TypeError` rather than a silent fall-through. `hook` is the
+/// `@@hasInstance` value of `target` (C), or the key to read it with; a read
+/// runs a getter that can collect, so both operands are rooted across it and
+/// written back refreshed. A callable hook runs as `Call(cb, C, « value »)`:
+/// C is its receiver.
+///
+/// `Function.prototype[@@hasInstance]` (the intrinsic default, reached through
+/// a linked chain or copied onto C) answers `OrdinaryHasInstance(C, value)`,
+/// which is the ordinary dispatch the caller continues with, so it falls
+/// through. It is recognized by its code, the function's identity: comparing
+/// value bits against a fresh read of `Function.prototype`'s slot misses it
+/// whenever that read allocates and the collector moves `cb` first, and the
+/// thunk run with C would re-enter this same dispatch.
+///
+/// Out of line and cold: a hook is the rare case, and its handle scope and
+/// calls stay out of the ordinary dispatch's frame.
+#[cold]
+#[inline(never)]
+fn dispatch_own_has_instance(
+    hook: OwnHook,
+    target: &mut f64,
+    value: &mut f64,
+) -> HasInstanceOutcome {
+    let cb = match hook {
+        OwnHook::Value(cb) => cb,
+        OwnHook::Read(key) => {
+            let scope = crate::gc::RuntimeHandleScope::new();
+            let lhs = scope.root_nanbox_f64(*value);
+            let rhs = scope.root_nanbox_f64(*target);
+            let cb =
+                unsafe { crate::symbol::js_object_get_symbol_property(rhs.get_nanbox_f64(), key) };
+            *value = lhs.get_nanbox_f64();
+            *target = rhs.get_nanbox_f64();
+            cb
+        }
+    };
+    let (target, value) = (*target, *value);
     let jv = crate::JSValue::from_bits(cb.to_bits());
     if jv.is_undefined() || jv.is_null() {
         return HasInstanceOutcome::Fallthrough;
     }
     if !value_is_callable(cb) {
         throw_type_error(b"Symbol(Symbol.hasInstance) is not a function");
     }
+    if is_intrinsic_has_instance(cb) {
+        return HasInstanceOutcome::Fallthrough;
+    }
     let args = [value];
     let r = unsafe {
         crate::closure::js_native_call_value(
             cb,
-            crate::closure::plain_call_receiver(),
+            crate::closure::JsThis::from_f64(target),
             args.as_ptr(),
             1,
         )
@@ -631,6 +675,20 @@ fn dispatch_own_has_instance(cb: f64, value: f64) -> HasInstanceOutcome {
     })
 }
 
+/// Is `cb` the intrinsic `Function.prototype[@@hasInstance]`? Its code is
+/// [`function_prototype_has_instance_thunk`]; only the runtime mints a
+/// function with that code.
+fn is_intrinsic_has_instance(cb: f64) -> bool {
+    let jv = crate::JSValue::from_bits(cb.to_bits());
+    if !jv.is_pointer() {
+        return false;
+    }
+    let addr = jv.as_pointer::<u8>() as usize;
+    crate::closure::is_closure_ptr(addr)
+        && unsafe { (*(addr as *const crate::closure::ClosureHeader)).code() }
+            == function_prototype_has_instance_thunk as *const u8
+}
+
 fn is_event_emitter_instance_value(value: f64) -> bool {
     if is_native_module_namespace_value(value, "cluster.default")
         || crate::cluster::is_worker_instance_value(value)

crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs

@@ -149,33 +149,14 @@ pub extern "C" fn js_instanceof_dynamic(value: f64, mut type_ref: f64) -> f64 {
             };
             if owns_hook {
                 // An own @@hasInstance getter can collect even when it returns
-                // undefined and ordinary dispatch continues. Refresh both
-                // operands before the callback or any later prototype check.
-                let scope = crate::gc::RuntimeHandleScope::new();
-                let lhs_h = scope.root_nanbox_f64(value);
-                let rhs_h = scope.root_nanbox_f64(type_ref);
-                let cb = unsafe {
-                    crate::symbol::js_object_get_symbol_property(rhs_h.get_nanbox_f64(), hi_f64)
-                };
-                value = lhs_h.get_nanbox_f64();
-                type_ref = rhs_h.get_nanbox_f64();
-                rhs.refresh_after_getter(type_ref);
-                // A linked function whose chain reaches the intrinsic default
-                // takes the ordinary dispatch below (its native brand checks
-                // are that default's answer).
-                let intrinsic = linked && {
-                    let fp = crate::object::builtin_prototype_value("Function");
-                    crate::JSValue::from_bits(fp.to_bits()).is_pointer()
-                        && unsafe { crate::symbol::js_object_get_symbol_property(fp, hi_f64) }
-                            .to_bits()
-                            == cb.to_bits()
-                };
-                if !intrinsic {
-                    if let HasInstanceOutcome::Result(result) = dispatch_own_has_instance(cb, value)
-                    {
-                        return result;
-                    }
+                // undefined and ordinary dispatch continues: the hook read
+                // refreshes both operands.
+                if let HasInstanceOutcome::Result(result) =
+                    dispatch_own_has_instance(OwnHook::Read(hi_f64), &mut type_ref, &mut value)
+                {
+                    return result;
                 }
+                rhs.refresh_after_getter(type_ref);
             }
         }
     }

crates/perry-runtime/src/object/instanceof/static_dispatch.rs

@@ -148,8 +148,26 @@ fn instanceof_dispatch(
         if !hi_sym.is_null() {
             let hi_f64 = f64::from_bits(crate::value::JSValue::pointer(hi_sym as *const u8).bits());
             if let Some(vb) = crate::symbol::class_static_symbol_lookup_in_chain(class_id, hi_f64) {
-                let cb = f64::from_bits(vb);
-                if let HasInstanceOutcome::Result(r) = dispatch_own_has_instance(cb, value) {
+                let mut cb = f64::from_bits(vb);
+                // The hook's receiver is C. A static-id caller names C by
+                // its id; its function object is the class value, whose first
+                // read may allocate, so both operands are rooted across it.
+                let mut target = match constructor {
+                    Some(target) => target,
+                    None => {
+                        let scope = crate::gc::RuntimeHandleScope::new();
+                        let lhs = scope.root_nanbox_f64(value);
+                        let hook = scope.root_nanbox_f64(cb);
+                        let target =
+                            crate::object::native_module::class_constructor_ref_value(class_id);
+                        value = lhs.get_nanbox_f64();
+                        cb = hook.get_nanbox_f64();
+                        target
+                    }
+                };
+                if let HasInstanceOutcome::Result(r) =
+                    dispatch_own_has_instance(OwnHook::Value(cb), &mut target, &mut value)
+                {
                     return r;
                 }
             }

test-files/test_gap_stream_from_web_non_web_operand.ts

@@ -0,0 +1,32 @@
+// `fromWeb` of a value that is not a web stream is a TypeError, as in node,
+// and an `@@hasInstance` hook runs with C as its receiver. Next.js's
+// `e instanceof Readable ? e : Readable.fromWeb(e)` once took the second arm
+// for a real Readable and then spun forever inside fromWeb.
+import { Readable, Writable, Duplex, PassThrough } from "node:stream";
+
+function attempt(label: string, f: () => unknown) {
+  try {
+    const r = f();
+    console.log(label, "returned", r instanceof Readable || r instanceof Writable);
+  } catch (e: any) {
+    console.log(label, "threw", e instanceof TypeError, e.code, e.message);
+  }
+}
+attempt("1 Readable.fromWeb(PassThrough)", () => Readable.fromWeb(new PassThrough() as any));
+attempt("2 Readable.fromWeb({readable})", () => Readable.fromWeb({ readable: true } as any));
+attempt("3 Writable.fromWeb(PassThrough)", () => Writable.fromWeb(new PassThrough() as any));
+attempt("4 Duplex.fromWeb({readable, writable})", () => Duplex.fromWeb({ readable: 1, writable: 2 } as any));
+
+const web = new ReadableStream({ start(c) { c.enqueue(new TextEncoder().encode("hi")); c.close(); } });
+const node = Readable.fromWeb(web as any);
+const chunks: string[] = [];
+node.on("data", (d: any) => chunks.push(Buffer.from(d).toString()));
+node.on("end", () => console.log("5 web stream read", node instanceof Readable, chunks.join("")));
+
+function F() {}
+Object.defineProperty(F, Symbol.hasInstance, { value: function (this: unknown, v: unknown) { return this === F && v === 1; } });
+console.log("6 own hook receiver", (1 as any) instanceof (F as any), (2 as any) instanceof (F as any));
+
+function G(this: any) {}
+Object.defineProperty(G, Symbol.hasInstance, { value: Function.prototype[Symbol.hasInstance] });
+console.log("7 copied default hook", new (G as any)() instanceof (G as any), ({}) instanceof (G as any));