diff --git a/crates/perry-runtime/src/node_stream_constructors/web_adapter.rs b/crates/perry-runtime/src/node_stream_constructors/web_adapter.rs index bde1e3c07af..fe3e0f6875a 100644 --- a/crates/perry-runtime/src/node_stream_constructors/web_adapter.rs +++ b/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) } } diff --git a/crates/perry-runtime/src/object/instanceof.rs b/crates/perry-runtime/src/object/instanceof.rs index 03770e1cebd..3f6fec74e0c 100644 --- a/crates/perry-runtime/src/object/instanceof.rs +++ b/crates/perry-runtime/src/object/instanceof.rs @@ -602,12 +602,53 @@ 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; @@ -615,11 +656,14 @@ fn dispatch_own_has_instance(cb: f64, value: f64) -> HasInstanceOutcome { 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::() 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) diff --git a/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs b/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs index 51d3d675b3d..d7a2f0e9300 100644 --- a/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs +++ b/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); } } } diff --git a/crates/perry-runtime/src/object/instanceof/static_dispatch.rs b/crates/perry-runtime/src/object/instanceof/static_dispatch.rs index 10b60e3ebab..7dae3fb79ed 100644 --- a/crates/perry-runtime/src/object/instanceof/static_dispatch.rs +++ b/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; } } diff --git a/test-files/test_gap_stream_from_web_non_web_operand.ts b/test-files/test_gap_stream_from_web_non_web_operand.ts new file mode 100644 index 00000000000..635d85b874a --- /dev/null +++ b/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));