diff --git a/changelog.d/10952-async-resource-direct-objects.md b/changelog.d/10952-async-resource-direct-objects.md new file mode 100644 index 0000000000..fd06e893e4 --- /dev/null +++ b/changelog.d/10952-async-resource-direct-objects.md @@ -0,0 +1,5 @@ +`new AsyncResource(...)` and `createHook(...)` now return ordinary objects (#10926, direct half). Both used to hand JS a raw `Box::into_raw` address with no `GcHeader`, so every type probe read the bytes in front of the `Box` and `JSON.stringify(new AsyncResource("x"))` printed `""` or `[]` depending on the build, where Node prints `{}`. The `Box` is still the module's internal state. The JS value is now a `GC_TYPE_OBJECT` whose `ObjectMeta.native_state` word records the backing address, and every `js_async_resource_*` / `js_async_hook_*` entry point resolves its receiver to that backing (`resolve_async_resource_handle` / `resolve_async_hook_handle`). The resolver never does a property get: it sits on the generic property-miss path, and an earlier draft that did recursed until the stack overflowed. Class ids: `AsyncHook` takes `native_class_ids::ASYNC_HOOK` (`0xFFFF_2411`). `AsyncResource` keeps its legacy `0xFFFF_0079` as `ASYNC_RESOURCE_LEGACY`, because emitted code already bakes that id in. Both ids are in `is_native_backed_class_id`, so neither can cross a worker boundary. The `hot_diag` receiver-representation arms for both families are gone, and their fixtures now assert they are migrated. The subclass half, which links `R.prototype.[[Prototype]]` to `AsyncResource.prototype`, is held for a codegen change in `property_get.rs`. + +Native callers that stored `js_async_resource_new`'s result still assumed it was the backing, and they were fixed in the same change. `set_async_resource_event_emitter` now resolves its argument. It used to drop the link without any error, so `eear.asyncResource.eventEmitter` was `undefined`. The `EventEmitterAsyncResource` subclass brand check in `node_stream_dispatch` now resolves as well. It used to reject its own hidden resource, so `emit` on a subclass threw `Cannot read private member`. `EventEmitterHandle.async_resource_handle` now holds a movable object, so both copies of the events scanner visit it: the in-tree `stdlib:events` one and the one in `perry-ext-events`, which is the archive the compiler actually routes `node:events` to. Without the visit, the first collection left `eear.asyncId` reading `0` and ran listeners outside the resource's scope. `js_event_emitter_async_resource_subclass_init` roots the resource object across the hidden-key allocation that comes before storing it. Adds `test-files/test_gap_10952_eventemitter_async_resource_object.ts`. + +`async_hooks.rs` would have gone over the 2000-line cap, so its argument-conversion and error-rendering helpers move to `async_hooks/arg_values.rs`. That block has no raw-handle sites, so no ratchet entry moves. The test-only `TEST_FORCE_RESOLVE_GC` hook moved from the resolver into `js_async_resource_run_in_async_scope`, and its root-holders exemption is deleted. diff --git a/crates/perry-ext-events/src/lib.rs b/crates/perry-ext-events/src/lib.rs index a78b808863..4f1e792fda 100644 --- a/crates/perry-ext-events/src/lib.rs +++ b/crates/perry-ext-events/src/lib.rs @@ -301,6 +301,8 @@ pub struct EventEmitterHandle { max_listeners: f64, capture_rejections: bool, domain_handle: Option, + /// The public AsyncResource handle OBJECT (#10926): movable, so the GC + /// scanner visits this slot. async_resource_handle: i64, } @@ -541,6 +543,13 @@ fn scan_events_roots(visitor: &mut EventsRootVisitor) { } } } + // #10926: `js_async_resource_new` returns an ordinary, movable handle + // object now (it used to be a never-freed `Box`), and this slot is its + // only native-side holder -- the same visit the in-tree + // `perry-stdlib` events scanner makes. + if is_heap_pointer_candidate(emitter.async_resource_handle) { + visitor.visit_i64_slot(&mut emitter.async_resource_handle); + } for pending in emitter.pending_once_promises.values_mut() { for p in pending.iter_mut() { visitor.visit_raw_mut_ptr_slot(&mut p.promise); diff --git a/crates/perry-runtime/src/async_hooks.rs b/crates/perry-runtime/src/async_hooks.rs index 11abb6c2f2..a4169d7c3c 100644 --- a/crates/perry-runtime/src/async_hooks.rs +++ b/crates/perry-runtime/src/async_hooks.rs @@ -19,6 +19,11 @@ use crate::object::{js_object_get_field_by_name, ObjectHeader}; use crate::string::{js_string_from_bytes, StringHeader}; use crate::value::{JSValue, POINTER_MASK}; +mod arg_values; +use arg_values::{ + async_id_to_js_number, is_callable_value, js_string_value_to_string, require_string_arg, + throw_apply_not_function, trigger_id_from_options, validate_bind_callback, +}; mod provider_ffi; pub use provider_ffi::{ defer_destroy_after_check_turns, js_async_hooks_provider_defer_destroy, @@ -165,6 +170,8 @@ per_test_global! { static ASYNC_WRAP_PROVIDERS: AtomicU64 = AtomicU64::new(0); } +const ASYNC_RESOURCE_SUBCLASS_KEY: &[u8] = b"__perryAsyncResourceBacking"; + /// Live `AsyncResource` handles. Handles are raw `Box::into_raw` pointers /// (never freed → membership is monotonic), NaN-boxed with POINTER_TAG like /// heap objects — so the dynamic method path needs this registry to recognize @@ -173,7 +180,6 @@ per_test_global! { static ASYNC_RESOURCE_HANDLES: LazyLock>> = LazyLock::new(|| Mutex::new(HashSet::new())); static ASYNC_RESOURCE_HANDLE_COUNT: AtomicUsize = AtomicUsize::new(0); -const ASYNC_RESOURCE_SUBCLASS_KEY: &[u8] = b"__perryAsyncResourceBacking"; /// Live `AsyncHook` handles, for the same dynamic-receiver reason as /// `ASYNC_RESOURCE_HANDLES`. A helper that returns @@ -227,53 +233,208 @@ pub(crate) fn is_async_hook_handle(handle: i64) -> bool { /// Resolve either a native `AsyncResource` handle or the ordinary object used /// for a source-compiled subclass to its native backing allocation. -pub(crate) fn resolve_async_resource_handle(receiver: i64) -> Option { - if is_async_resource_handle(receiver) { - return Some(receiver); + +// =========================================================================== +// Honest tags (#340/#341, #10926): an `AsyncResource` / `AsyncHook` handed to +// JS is an ORDINARY object. +// +// Both used to be raw `Box::into_raw` addresses under `POINTER_TAG` with NO +// `GcHeader` (the plan's `NR_FOREIGN_PTR` class), so every type probe read the +// bytes in front of the `Box`: `JSON.stringify` answered `""` or `[]` where +// node answers `{}`, build-dependently. A subclass fared worse — with no +// prototype link (now wired in `class_registry::state`) the five methods were +// copied onto each instance and the raw `Box` was parked in a user-visible +// `__perryAsyncResourceBacking` field, so `Object.keys(new R())` leaked it. +// +// The `Box` is unchanged and stays the module's internal currency: every +// `js_async_resource_*` entry point still resolves to it through +// `resolve_async_resource_handle`. Only the value crossing into JS changes. +// +// State word: the backing address ORed with `PRESENT` (and `KIND_HOOK` for a +// hook). A `Box` is 8-aligned, so the low three bits are free. The address is +// then confirmed against `ASYNC_RESOURCE_HANDLES` / `ASYNC_HOOK_HANDLES` — +// exact membership, so a word written by some other family can never produce a +// false positive. +// =========================================================================== + +/// Legacy reserved id, already used by `instanceof` and +/// `class_registry::parent_static`. NOT moved into the `0xFFFF_24xx` block: +/// it is baked into emitted code in three places and renumbering a live class +/// id is #10824's hazard for no gain. +pub(crate) const ASYNC_RESOURCE_CLASS_ID: u32 = crate::native_class_ids::ASYNC_RESOURCE_LEGACY; +/// A fresh id from the `native_class_ids` web-builtin block (`0x2411`, the +/// next one after the `perry/tui` family's `0x240B..=0x2410`). +pub(crate) const ASYNC_HOOK_CLASS_ID: u32 = crate::native_class_ids::ASYNC_HOOK; + +const ASYNC_STATE_PRESENT: u64 = 1; +const ASYNC_STATE_KIND_HOOK: u64 = 1 << 1; +const ASYNC_STATE_ADDR_MASK: u64 = !0b111; + +fn async_state_word(backing: i64, is_hook: bool) -> u64 { + debug_assert_eq!(backing as u64 & 0b111, 0, "a Box backing must be 8-aligned"); + let mut word = (backing as u64 & ASYNC_STATE_ADDR_MASK) | ASYNC_STATE_PRESENT; + if is_hook { + word |= ASYNC_STATE_KIND_HOOK; + } + word +} + +/// The backing address recorded in `receiver`'s `ObjectMeta`, or `None`. +/// Accepts ANY object: a direct instance carries its own class id, a +/// `class R extends AsyncResource` instance carries the USER's class id, so +/// the brand cannot be the class id here. Exact registry membership is the +/// brand instead, applied by the caller. +fn async_state_backing(receiver: i64, is_hook: bool) -> Option { + if receiver <= 0 { + return None; } - let raw = receiver as usize; - if !crate::value::addr_class::is_plausible_heap_addr(raw) { + let addr = receiver as usize; + // `try_read_TRACKED_gc_header`, not `try_read_gc_header`: this resolver is + // handed arbitrary receivers, including the header-less `Box`es this very + // family still produces, and the unchecked reader would take `addr - 8` + // from a non-object and then dereference a fabricated `meta` (measured: a + // SIGSEGV). The tracked reader proves the allocator owns the address + // before anything is read through it -- the same lesson as #10925/#10933. + let header = unsafe { crate::value::addr_class::try_read_tracked_gc_header(addr)? }; + if unsafe { header.as_ref() }.obj_type != crate::gc::GC_TYPE_OBJECT { return None; } + let obj = addr as *mut ObjectHeader; + unsafe { + let meta = (*obj).meta; + if meta.is_null() { + return None; + } + let word = (*meta).native_state; + if word & ASYNC_STATE_PRESENT == 0 { + return None; + } + if (word & ASYNC_STATE_KIND_HOOK != 0) != is_hook { + return None; + } + Some((word & ASYNC_STATE_ADDR_MASK) as i64) + } +} + +/// Record `backing` on an existing object (the `class R extends AsyncResource` +/// receiver). +fn set_async_state(receiver: *mut ObjectHeader, backing: i64, is_hook: bool) { + unsafe { + let meta = crate::object::object_meta_ensure(receiver); + debug_assert!(!meta.is_null(), "an async handle must carry its meta"); + if !meta.is_null() { + (*meta).native_state = async_state_word(backing, is_hook); + } + } +} + +/// Wrap a backing `Box` in the JS-visible handle object. +fn async_handle_object(backing: i64, is_hook: bool) -> i64 { + let class_id = if is_hook { + ASYNC_HOOK_CLASS_ID + } else { + ASYNC_RESOURCE_CLASS_ID + }; + let obj = crate::object::js_object_alloc(class_id, 0); + if obj.is_null() { + return 0; + } + // Resolving the prototype allocates and `GC_TYPE_OBJECT` is movable, so + // the instance is re-read through its handle after each allocating step. let scope = crate::gc::RuntimeHandleScope::new(); - let receiver = scope.root_raw_mut_ptr(raw as *mut ObjectHeader); - #[cfg(test)] - if TEST_FORCE_RESOLVE_GC.swap(0, Ordering::Relaxed) != 0 { - let _ = crate::gc::gc_collect_minor(); + let handle = scope.root_raw_mut_ptr(obj); + if !is_hook { + // The SAME prototype object a subclass now inherits, resolved through + // the same helper, so `getPrototypeOf(new AsyncResource(x))` and + // `getPrototypeOf(R.prototype)` are one object. + let proto = crate::object::async_resource_prototype_value(); + if crate::value::JSValue::from_bits(proto.to_bits()).is_pointer() { + handle.with_mut_ptr::(|obj| { + crate::object::prototype_chain::object_link_class_default_prototype( + obj as usize, + proto.to_bits(), + ); + }); + } } - let key = js_string_from_bytes( - ASYNC_RESOURCE_SUBCLASS_KEY.as_ptr(), - ASYNC_RESOURCE_SUBCLASS_KEY.len() as u32, - ); - let value = receiver - .with_mut_ptr::(|receiver| js_object_get_field_by_name(receiver, key)); - if !value.is_pointer() { - return None; + handle.with_mut_ptr::(|obj| set_async_state(obj, backing, is_hook)); + handle.with_mut_ptr::(|obj| obj as i64) +} + +/// The backing behind an `AsyncResource` receiver. +/// +/// **This resolver must never perform a property get.** #10926 put it on the +/// generic property-miss path: `js_object_get_field_by_name` calls +/// `try_async_resource_property_dispatch` for ANY receiver +/// (`field_get_set/get_field_by_name.rs`), and that entry point now resolves +/// the receiver instead of the identity check it used before. Reading an own +/// property from here therefore closes a cycle -- +/// `get_field_by_name` -> `try_async_resource_property_dispatch` -> +/// `resolve_async_resource_handle` -> `get_field_by_name` -- and because the +/// key it looked for (`__perryAsyncResourceBacking`) is absent on ordinary +/// objects, the inner lookup always misses and re-enters. A first draft of +/// this split did exactly that and blew the 8 MB stack on the FIRST property +/// miss after `node:async_hooks` was linked: `import "node:async_hooks"` alone +/// was a SIGSEGV. The `ObjectMeta.native_state` word is allocation-free and +/// cannot re-enter, which is why both representations are recorded there. +/// +/// Exact registry membership is the brand: a `native_state` word written by +/// any other family cannot name a live resource backing. +pub(crate) fn resolve_async_resource_handle(receiver: i64) -> Option { + if is_async_resource_handle(receiver) { + return Some(receiver); } - let backing = value.as_pointer::() as i64; + let backing = async_state_backing(receiver, false)?; is_async_resource_handle(backing).then_some(backing) } +/// The hook backing behind a JS value, for `hook.enable()` / `.disable()`. +fn resolve_async_hook_handle(receiver: i64) -> Option { + if is_async_hook_handle(receiver) { + return Some(receiver); + } + let backing = async_state_backing(receiver, true)?; + is_async_hook_handle(backing).then_some(backing) +} + #[cfg(test)] pub(crate) fn test_force_next_async_resource_resolve_gc() { TEST_FORCE_RESOLVE_GC.store(1, Ordering::Relaxed); } +/// Link a subclass receiver to its backing exactly the way +/// `js_async_resource_subclass_init` does: the held `__perryAsyncResourceBacking` +/// own property AND the `ObjectMeta.native_state` word the resolver actually +/// reads. Planting only one of the two would let the GC-root test below pass +/// against a representation production does not produce. #[cfg(test)] -pub(crate) fn test_link_async_resource_subclass(receiver: *mut ObjectHeader, backing: i64) { +pub(crate) fn test_link_async_resource_subclass( + receiver: *mut ObjectHeader, + backing: i64, +) -> *mut ObjectHeader { let scope = crate::gc::RuntimeHandleScope::new(); let receiver = scope.root_raw_mut_ptr(receiver); let key = js_string_from_bytes( ASYNC_RESOURCE_SUBCLASS_KEY.as_ptr(), ASYNC_RESOURCE_SUBCLASS_KEY.len() as u32, ); - receiver.with_mut_ptr::(|receiver| { - crate::object::js_object_set_field_by_name( - receiver, - key, - crate::value::js_nanbox_pointer(backing), - ); + // Both writes allocate, so either can move the receiver. `across_mut` + // reloads it from the root AFTER them and hands the caller that refreshed + // address: a test that kept the pre-call raw pointer would root a + // from-space address and the resolve would decline. + let ((), refreshed) = receiver.across_mut::(|| { + receiver.with_mut_ptr::(|receiver| { + crate::object::js_object_set_field_by_name( + receiver, + key, + crate::value::js_nanbox_pointer(backing), + ); + }); + receiver.with_mut_ptr::(|receiver| { + set_async_state(receiver, backing, false); + }); }); + refreshed } #[inline(always)] @@ -572,7 +733,8 @@ pub extern "C" fn js_async_hooks_create_hook(options: f64) -> i64 { crate::hot_diag::ReceiverReprFamily::AsyncHook, ); } - handle + // #10926: hand JS an ordinary object wrapping the backing. + async_handle_object(handle, true) } /// Dynamic method dispatch for `AsyncHook` values whose static class was lost @@ -593,10 +755,11 @@ pub fn try_async_hook_method_dispatch(handle: i64, method_name: &str) -> Option< } #[no_mangle] -pub extern "C" fn js_async_hook_enable(handle: i64) -> i64 { - if handle == 0 { - return handle; - } +pub extern "C" fn js_async_hook_enable(receiver: i64) -> i64 { + // #10926: the receiver is the handle OBJECT; resolve it to the backing. + let Some(handle) = resolve_async_hook_handle(receiver) else { + return receiver; + }; let hook = unsafe { &*(handle as *const AsyncHookHandle) }; if HOOK_CALLBACK_DEPTH.with(Cell::get) != 0 { PENDING_HOOK_STATES.with(|pending| { @@ -633,10 +796,11 @@ fn set_hook_enabled(index: usize, enabled: bool) { } #[no_mangle] -pub extern "C" fn js_async_hook_disable(handle: i64) -> i64 { - if handle == 0 { - return handle; - } +pub extern "C" fn js_async_hook_disable(receiver: i64) -> i64 { + // #10926: the receiver is the handle OBJECT; resolve it to the backing. + let Some(handle) = resolve_async_hook_handle(receiver) else { + return receiver; + }; let hook = unsafe { &*(handle as *const AsyncHookHandle) }; if HOOK_CALLBACK_DEPTH.with(Cell::get) != 0 { PENDING_HOOK_STATES.with(|pending| { @@ -953,270 +1117,6 @@ pub fn drain_gc_destroy_queue() -> i32 { count } -#[inline] -fn async_id_to_js_number(id: u64) -> f64 { - if id == u64::MAX { - -1.0 - } else { - id as f64 - } -} - -fn string_header_to_string(ptr: *const StringHeader) -> String { - if ptr.is_null() { - return String::new(); - } - unsafe { - let len = (*ptr).byte_len as usize; - let data = crate::string::string_data(ptr); - String::from_utf8_lossy(std::slice::from_raw_parts(data, len)).into_owned() - } -} - -fn js_string_value_to_string(value: f64) -> String { - let ptr = crate::value::js_get_string_pointer_unified(value) as *const StringHeader; - string_header_to_string(ptr) -} - -fn symbol_to_string(value: f64) -> String { - if unsafe { crate::symbol::js_is_symbol(value) == 0 } { - return "Symbol()".to_string(); - } - let ptr = unsafe { crate::symbol::js_symbol_to_string(value) } as *const StringHeader; - string_header_to_string(ptr) -} - -fn value_is_array(value: f64) -> bool { - let jv = JSValue::from_bits(value.to_bits()); - if !jv.is_pointer() { - return false; - } - let ptr = jv.as_pointer::(); - if ptr.is_null() || (ptr as usize) < crate::gc::GC_HEADER_SIZE + 0x1000 { - return false; - } - unsafe { - let gc_header = &*(ptr.sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader); - gc_header.obj_type == crate::gc::GC_TYPE_ARRAY - } -} - -fn is_callable_value(value: f64) -> bool { - !crate::fs::extract_closure_ptr(value).is_null() -} - -fn describe_received_async_hooks(value: f64) -> String { - if is_callable_value(value) { - return "function ".to_string(); - } - if unsafe { crate::symbol::js_is_symbol(value) != 0 } { - return format!("type symbol ({})", symbol_to_string(value)); - } - crate::fs::validate::describe_received(value) -} - -fn require_string_arg(arg_name: &str, value: f64) -> String { - let jv = JSValue::from_bits(value.to_bits()); - if !jv.is_any_string() { - let message = format!( - "The \"{}\" argument must be of type string. Received {}", - arg_name, - describe_received_async_hooks(value) - ); - crate::fs::validate::throw_type_error_with_code(&message, "ERR_INVALID_ARG_TYPE"); - } - js_string_value_to_string(value) -} - -fn format_js_number_for_error(value: f64) -> String { - if value.is_nan() { - "NaN".to_string() - } else if value == f64::INFINITY { - "Infinity".to_string() - } else if value == f64::NEG_INFINITY { - "-Infinity".to_string() - } else if value.fract() == 0.0 { - format!("{}", value as i64) - } else { - value.to_string() - } -} - -const MAX_SAFE_JS_INTEGER: f64 = 9_007_199_254_740_991.0; - -fn trigger_async_id_value(value: f64) -> Option { - let jv = JSValue::from_bits(value.to_bits()); - let id = if jv.is_int32() { - jv.as_int32() as f64 - } else if jv.is_number() { - jv.as_number() - } else { - return None; - }; - - if !id.is_finite() || id.fract() != 0.0 || !(-1.0..=MAX_SAFE_JS_INTEGER).contains(&id) { - return None; - } - if id == -1.0 { - Some(u64::MAX) - } else { - Some(id as u64) - } -} - -fn render_invalid_trigger_async_id(value: f64) -> String { - let jv = JSValue::from_bits(value.to_bits()); - if jv.is_undefined() { - return "undefined".to_string(); - } - if jv.is_null() { - return "null".to_string(); - } - if jv.is_bool() { - return jv.as_bool().to_string(); - } - if jv.is_any_string() { - return js_string_value_to_string(value); - } - if unsafe { crate::symbol::js_is_symbol(value) != 0 } { - return symbol_to_string(value); - } - if jv.is_int32() { - return jv.as_int32().to_string(); - } - if jv.is_number() { - return format_js_number_for_error(jv.as_number()); - } - if value_is_array(value) { - return "[]".to_string(); - } - if jv.is_pointer() { - return "{}".to_string(); - } - "undefined".to_string() -} - -fn trigger_async_id_or_throw(value: f64) -> u64 { - if let Some(id) = trigger_async_id_value(value) { - return id; - } - let message = format!( - "Invalid triggerAsyncId value: {}", - render_invalid_trigger_async_id(value) - ); - crate::fs::validate::throw_range_error_named(&message, "ERR_INVALID_ASYNC_ID") -} - -fn throw_null_trigger_async_id_options() -> ! { - let message = b"Cannot read properties of null (reading 'triggerAsyncId')"; - let msg = js_string_from_bytes(message.as_ptr(), message.len() as u32); - let err = crate::error::js_typeerror_new(msg); - crate::exception::js_throw(crate::value::js_nanbox_pointer(err as i64)) -} - -fn trigger_id_from_options(options: f64) -> u64 { - let options_value = JSValue::from_bits(options.to_bits()); - if options_value.is_undefined() { - return execution_async_id_u64(); - } - if options_value.is_int32() || options_value.is_number() { - return trigger_async_id_or_throw(options); - } - if options_value.is_null() { - throw_null_trigger_async_id_options(); - } - - // Node's constructor first validates the option and then consumes it, - // making an accessor observable twice. Preserve that exact ordering; the - // `requireManualDestroy` option is read after both trigger-id reads. - let first_trigger_value = object_field(options, b"triggerAsyncId"); - if !JSValue::from_bits(first_trigger_value.to_bits()).is_undefined() { - let _ = trigger_async_id_or_throw(first_trigger_value); - } - let trigger_value = object_field(options, b"triggerAsyncId"); - let trigger_value_kind = JSValue::from_bits(trigger_value.to_bits()); - let trigger_id = if trigger_value_kind.is_undefined() { - execution_async_id_u64() - } else { - trigger_async_id_or_throw(trigger_value) - }; - let _ = object_field(options, b"requireManualDestroy"); - trigger_id -} - -fn render_apply_value(value: f64) -> String { - let jv = JSValue::from_bits(value.to_bits()); - if jv.is_undefined() { - return "undefined".to_string(); - } - if jv.is_null() { - return "null".to_string(); - } - if jv.is_bool() { - return jv.as_bool().to_string(); - } - if jv.is_any_string() { - return js_string_value_to_string(value); - } - if unsafe { crate::symbol::js_is_symbol(value) != 0 } { - return symbol_to_string(value); - } - if jv.is_int32() { - return jv.as_int32().to_string(); - } - if jv.is_number() { - return format_js_number_for_error(jv.as_number()); - } - if value_is_array(value) { - return "[object Array]".to_string(); - } - if jv.is_pointer() { - return "#".to_string(); - } - "undefined".to_string() -} - -fn describe_apply_type(value: f64) -> &'static str { - let jv = JSValue::from_bits(value.to_bits()); - if jv.is_undefined() { - "undefined" - } else if jv.is_null() { - "null" - } else if jv.is_bool() { - "a boolean" - } else if jv.is_any_string() { - "a string" - } else if unsafe { crate::symbol::js_is_symbol(value) != 0 } { - "a symbol" - } else if jv.is_int32() || jv.is_number() { - "a number" - } else { - "an object" - } -} - -fn throw_apply_not_function(value: f64) -> ! { - let message = format!( - "Function.prototype.apply was called on {}, which is {} and not a function", - render_apply_value(value), - describe_apply_type(value) - ); - let msg = js_string_from_bytes(message.as_ptr(), message.len() as u32); - let err = crate::error::js_typeerror_new(msg); - crate::exception::js_throw(crate::value::js_nanbox_pointer(err as i64)) -} - -fn validate_bind_callback(value: f64) { - if is_callable_value(value) { - return; - } - let message = format!( - "The \"fn\" argument must be of type function. Received {}", - describe_received_async_hooks(value) - ); - crate::fs::validate::throw_type_error_with_code(&message, "ERR_INVALID_ARG_TYPE") -} - #[no_mangle] pub extern "C" fn js_async_resource_new(type_value: f64, options: f64) -> i64 { new_async_resource_with_public_value(type_value, options, None) @@ -1255,10 +1155,27 @@ fn new_async_resource_with_public_value( crate::hot_diag::ReceiverReprFamily::AsyncResource, ); } - let resource_value = public_resource.unwrap_or_else(|| crate::value::js_nanbox_pointer(handle)); - let ids = init_resource_with_trigger(&type_name, resource_value, true, trigger_async_id); + // #10926: what crosses into JS is an ordinary object wrapping `handle`. + // For a subclass the public value is the user's `this`, which + // `js_async_resource_subclass_init` stamps instead. + let public = match public_resource { + Some(v) => v, + None => { + let obj = async_handle_object(handle, false); + if obj == 0 { + return 0; + } + crate::value::js_nanbox_pointer(obj) + } + }; + let ids = init_resource_with_trigger(&type_name, public, true, trigger_async_id); unsafe { (*(handle as *mut AsyncResourceHandle)).ids = ids }; - handle + if public_resource.is_some() { + // Subclass: the caller owns the public object and returns it; hand back + // the backing so it can stamp `this`. + return handle; + } + crate::value::js_nanbox_get_pointer(public) as i64 } /// Initialize the native backing for a source-compiled @@ -1282,6 +1199,12 @@ pub extern "C" fn js_async_resource_subclass_init( let raw = crate::value::js_nanbox_get_pointer(this_handle.get_nanbox_f64()) as *mut ObjectHeader; if !raw.is_null() && crate::value::addr_class::is_plausible_heap_addr(raw as usize) { + // HELD for the subclass half of #10926: the backing stays an own + // `__perryAsyncResourceBacking` property and the five methods stay + // copied onto each instance, because `R.prototype.[[Prototype]]` is + // still `Object.prototype` -- the two-arm prototype link needs a + // codegen condition in `property_get.rs` that belongs to another lane + // (see this PR's description). This half changes only the DIRECT path. let key = js_string_from_bytes( ASYNC_RESOURCE_SUBCLASS_KEY.as_ptr(), ASYNC_RESOURCE_SUBCLASS_KEY.len() as u32, @@ -1321,6 +1244,15 @@ pub extern "C" fn js_async_resource_subclass_init( crate::object::PropertyAttrs::new(true, false, true), ); } + // The RESOLUTION path is the metadata word, not the own property + // above. `resolve_async_resource_handle` runs on the generic + // property-miss path, so it may not do a property get (see its doc + // comment); recording the backing here is what lets it stay + // allocation-free while the subclass surface is held unchanged. + // Re-read `this` from its root: every write above can collect. + let current_raw = + crate::value::js_nanbox_get_pointer(this_handle.get_nanbox_f64()) as *mut ObjectHeader; + set_async_state(current_raw, backing, false); } this_handle.get_nanbox_f64() } @@ -1329,9 +1261,13 @@ pub extern "C" fn js_async_resource_subclass_init( /// public emitter. Node exposes this as `emitter.asyncResource.eventEmitter`. /// Both sides are stable native handles, so the link does not need GC rooting. pub fn set_async_resource_event_emitter(handle: i64, event_emitter: i64) { - if handle == 0 || !ASYNC_RESOURCE_HANDLES.lock().unwrap().contains(&handle) { + // #10926: callers hold what `js_async_resource_new` returned, which is the + // handle OBJECT now, not the backing. Resolve it like every other entry + // point; an exact-membership check here silently dropped the link, so + // `eear.asyncResource.eventEmitter` answered `undefined`. + let Some(handle) = resolve_async_resource_handle(handle) else { return; - } + }; unsafe { (*(handle as *mut AsyncResourceHandle)).event_emitter = event_emitter }; } @@ -1398,10 +1334,16 @@ fn async_resource_bind_method_value(handle: i64) -> f64 { crate::value::js_nanbox_pointer(closure as i64) } -pub fn try_async_resource_property_dispatch(handle: i64, property: &str) -> Option { - if !is_async_resource_handle(handle) { - return None; - } +pub fn try_async_resource_property_dispatch(receiver: i64, property: &str) -> Option { + // #10926: resolve the RECEIVER rather than requiring it to be the backing + // itself. Before the migration the JS value WAS the `Box`, so an identity + // check sufficed; now it is an ordinary object (and for a + // `class R extends AsyncResource` it always was). Going through the same + // resolver every other entry point uses is what makes a property READ of + // `bind` work on a subclass instance -- a fused CALL already resolved, + // which is why `sub.asyncId()` worked while `typeof sub.bind` was + // `undefined`. + let handle = resolve_async_resource_handle(receiver)?; // User-defined own properties shadow AsyncResource.prototype just as they // do on Node's ordinary public resource object. The backing allocation is // a native Box, so keep expandos in the same traced side table used by @@ -1558,6 +1500,20 @@ pub extern "C" fn js_async_resource_run_in_async_scope( let callback_handle = scope.root_nanbox_f64(callback_value); let this_arg_handle = scope.root_nanbox_f64(this_arg); let args_array_handle = scope.root_raw_const_ptr(args_array as *const ArrayHeader); + // #10926: the forced collection used to sit INSIDE + // `resolve_async_resource_handle`, because that resolver allocated the + // `__perryAsyncResourceBacking` key and therefore had inputs of its own to + // root. It reads `ObjectMeta.native_state` now and cannot allocate, so a + // collection can no longer originate there and forcing one inside it would + // only be testing scaffolding -- and would hand the resolver a stale + // receiver, since nothing refreshes it. The axis that still exists is this + // frame's: a collection between rooting the receiver and resolving it must + // not lose the receiver. `with_mut_ptr` below refreshes from the root, so + // the resolve still finds the backing; drop the rooting and it does not. + #[cfg(test)] + if TEST_FORCE_RESOLVE_GC.swap(0, Ordering::Relaxed) != 0 { + let _ = crate::gc::gc_collect_minor(); + } let Some(handle) = receiver_handle .with_mut_ptr::(|receiver| resolve_async_resource_handle(receiver as i64)) else { diff --git a/crates/perry-runtime/src/async_hooks/arg_values.rs b/crates/perry-runtime/src/async_hooks/arg_values.rs new file mode 100644 index 0000000000..07f3bab4e8 --- /dev/null +++ b/crates/perry-runtime/src/async_hooks/arg_values.rs @@ -0,0 +1,276 @@ +//! Argument conversion and Node-shaped error rendering for `node:async_hooks` +//! (split out of `async_hooks.rs` for the 2000-line cap, #10952). +//! +//! Everything here reads a JS value and either converts it (async ids, trigger +//! ids, strings) or renders it the way Node's `ERR_INVALID_ARG_TYPE` / +//! `Function.prototype.apply` messages do. None of it touches the hook or +//! resource registries, and none of it holds a GC root across an allocation. + +use super::{execution_async_id_u64, object_field}; +use crate::string::{js_string_from_bytes, StringHeader}; +use crate::value::JSValue; + +#[inline] +pub(super) fn async_id_to_js_number(id: u64) -> f64 { + if id == u64::MAX { + -1.0 + } else { + id as f64 + } +} + +fn string_header_to_string(ptr: *const StringHeader) -> String { + if ptr.is_null() { + return String::new(); + } + unsafe { + let len = (*ptr).byte_len as usize; + let data = crate::string::string_data(ptr); + String::from_utf8_lossy(std::slice::from_raw_parts(data, len)).into_owned() + } +} + +pub(super) fn js_string_value_to_string(value: f64) -> String { + let ptr = crate::value::js_get_string_pointer_unified(value) as *const StringHeader; + string_header_to_string(ptr) +} + +fn symbol_to_string(value: f64) -> String { + if unsafe { crate::symbol::js_is_symbol(value) == 0 } { + return "Symbol()".to_string(); + } + let ptr = unsafe { crate::symbol::js_symbol_to_string(value) } as *const StringHeader; + string_header_to_string(ptr) +} + +fn value_is_array(value: f64) -> bool { + let jv = JSValue::from_bits(value.to_bits()); + if !jv.is_pointer() { + return false; + } + // The TRACKED reader: `value` is arbitrary user input, and a POINTER-tagged + // value can still name a header-less native allocation (the held subclass + // `__perryAsyncResourceBacking` field). It proves the allocator owns the + // address before reading the header in front of it, where the hand-rolled + // `< GC_HEADER_SIZE + 0x1000` floor this replaced rejected neither handle + // bands nor foreign allocations. + let addr = jv.as_pointer::() as usize; + unsafe { crate::value::addr_class::try_read_tracked_gc_header(addr) } + .is_some_and(|header| unsafe { header.as_ref() }.obj_type == crate::gc::GC_TYPE_ARRAY) +} + +pub(super) fn is_callable_value(value: f64) -> bool { + !crate::fs::extract_closure_ptr(value).is_null() +} + +fn describe_received_async_hooks(value: f64) -> String { + if is_callable_value(value) { + return "function ".to_string(); + } + if unsafe { crate::symbol::js_is_symbol(value) != 0 } { + return format!("type symbol ({})", symbol_to_string(value)); + } + crate::fs::validate::describe_received(value) +} + +pub(super) fn require_string_arg(arg_name: &str, value: f64) -> String { + let jv = JSValue::from_bits(value.to_bits()); + if !jv.is_any_string() { + let message = format!( + "The \"{}\" argument must be of type string. Received {}", + arg_name, + describe_received_async_hooks(value) + ); + crate::fs::validate::throw_type_error_with_code(&message, "ERR_INVALID_ARG_TYPE"); + } + js_string_value_to_string(value) +} + +fn format_js_number_for_error(value: f64) -> String { + if value.is_nan() { + "NaN".to_string() + } else if value == f64::INFINITY { + "Infinity".to_string() + } else if value == f64::NEG_INFINITY { + "-Infinity".to_string() + } else if value.fract() == 0.0 { + format!("{}", value as i64) + } else { + value.to_string() + } +} + +const MAX_SAFE_JS_INTEGER: f64 = 9_007_199_254_740_991.0; + +fn trigger_async_id_value(value: f64) -> Option { + let jv = JSValue::from_bits(value.to_bits()); + let id = if jv.is_int32() { + jv.as_int32() as f64 + } else if jv.is_number() { + jv.as_number() + } else { + return None; + }; + + if !id.is_finite() || id.fract() != 0.0 || !(-1.0..=MAX_SAFE_JS_INTEGER).contains(&id) { + return None; + } + if id == -1.0 { + Some(u64::MAX) + } else { + Some(id as u64) + } +} + +fn render_invalid_trigger_async_id(value: f64) -> String { + let jv = JSValue::from_bits(value.to_bits()); + if jv.is_undefined() { + return "undefined".to_string(); + } + if jv.is_null() { + return "null".to_string(); + } + if jv.is_bool() { + return jv.as_bool().to_string(); + } + if jv.is_any_string() { + return js_string_value_to_string(value); + } + if unsafe { crate::symbol::js_is_symbol(value) != 0 } { + return symbol_to_string(value); + } + if jv.is_int32() { + return jv.as_int32().to_string(); + } + if jv.is_number() { + return format_js_number_for_error(jv.as_number()); + } + if value_is_array(value) { + return "[]".to_string(); + } + if jv.is_pointer() { + return "{}".to_string(); + } + "undefined".to_string() +} + +fn trigger_async_id_or_throw(value: f64) -> u64 { + if let Some(id) = trigger_async_id_value(value) { + return id; + } + let message = format!( + "Invalid triggerAsyncId value: {}", + render_invalid_trigger_async_id(value) + ); + crate::fs::validate::throw_range_error_named(&message, "ERR_INVALID_ASYNC_ID") +} + +fn throw_null_trigger_async_id_options() -> ! { + let message = b"Cannot read properties of null (reading 'triggerAsyncId')"; + let msg = js_string_from_bytes(message.as_ptr(), message.len() as u32); + let err = crate::error::js_typeerror_new(msg); + crate::exception::js_throw(crate::value::js_nanbox_pointer(err as i64)) +} + +pub(super) fn trigger_id_from_options(options: f64) -> u64 { + let options_value = JSValue::from_bits(options.to_bits()); + if options_value.is_undefined() { + return execution_async_id_u64(); + } + if options_value.is_int32() || options_value.is_number() { + return trigger_async_id_or_throw(options); + } + if options_value.is_null() { + throw_null_trigger_async_id_options(); + } + + // Node's constructor first validates the option and then consumes it, + // making an accessor observable twice. Preserve that exact ordering; the + // `requireManualDestroy` option is read after both trigger-id reads. + let first_trigger_value = object_field(options, b"triggerAsyncId"); + if !JSValue::from_bits(first_trigger_value.to_bits()).is_undefined() { + let _ = trigger_async_id_or_throw(first_trigger_value); + } + let trigger_value = object_field(options, b"triggerAsyncId"); + let trigger_value_kind = JSValue::from_bits(trigger_value.to_bits()); + let trigger_id = if trigger_value_kind.is_undefined() { + execution_async_id_u64() + } else { + trigger_async_id_or_throw(trigger_value) + }; + let _ = object_field(options, b"requireManualDestroy"); + trigger_id +} + +fn render_apply_value(value: f64) -> String { + let jv = JSValue::from_bits(value.to_bits()); + if jv.is_undefined() { + return "undefined".to_string(); + } + if jv.is_null() { + return "null".to_string(); + } + if jv.is_bool() { + return jv.as_bool().to_string(); + } + if jv.is_any_string() { + return js_string_value_to_string(value); + } + if unsafe { crate::symbol::js_is_symbol(value) != 0 } { + return symbol_to_string(value); + } + if jv.is_int32() { + return jv.as_int32().to_string(); + } + if jv.is_number() { + return format_js_number_for_error(jv.as_number()); + } + if value_is_array(value) { + return "[object Array]".to_string(); + } + if jv.is_pointer() { + return "#".to_string(); + } + "undefined".to_string() +} + +fn describe_apply_type(value: f64) -> &'static str { + let jv = JSValue::from_bits(value.to_bits()); + if jv.is_undefined() { + "undefined" + } else if jv.is_null() { + "null" + } else if jv.is_bool() { + "a boolean" + } else if jv.is_any_string() { + "a string" + } else if unsafe { crate::symbol::js_is_symbol(value) != 0 } { + "a symbol" + } else if jv.is_int32() || jv.is_number() { + "a number" + } else { + "an object" + } +} + +pub(super) fn throw_apply_not_function(value: f64) -> ! { + let message = format!( + "Function.prototype.apply was called on {}, which is {} and not a function", + render_apply_value(value), + describe_apply_type(value) + ); + let msg = js_string_from_bytes(message.as_ptr(), message.len() as u32); + let err = crate::error::js_typeerror_new(msg); + crate::exception::js_throw(crate::value::js_nanbox_pointer(err as i64)) +} + +pub(super) fn validate_bind_callback(value: f64) { + if is_callable_value(value) { + return; + } + let message = format!( + "The \"fn\" argument must be of type function. Received {}", + describe_received_async_hooks(value) + ); + crate::fs::validate::throw_type_error_with_code(&message, "ERR_INVALID_ARG_TYPE") +} diff --git a/crates/perry-runtime/src/async_hooks/test_support.rs b/crates/perry-runtime/src/async_hooks/test_support.rs index 18efb29c0a..93fc9330bb 100644 --- a/crates/perry-runtime/src/async_hooks/test_support.rs +++ b/crates/perry-runtime/src/async_hooks/test_support.rs @@ -162,19 +162,32 @@ mod tests { track_promises: true, }, ]); - let suppressed = AsyncHookHandle { index: 0 }; - let tracked = AsyncHookHandle { index: 1 }; - js_async_hook_enable(&suppressed as *const AsyncHookHandle as i64); + // #10926: `js_async_hook_enable`/`disable` take the JS receiver and + // resolve it, where they used to dereference whatever address they + // were handed. A BORROWED STACK handle is no longer a valid input -- + // it is not in the registry, so the resolve declines and the call is a + // no-op. Build the backing the way production does instead: a leaked + // `Box` in the registry, which is what makes membership monotonic and + // an address safe to keep. That exercises the resolver's registry arm; + // the handle-OBJECT arm is covered end to end by `hook.enable()` / + // `hook.disable()` in the object-surface integration test. + let suppressed = Box::into_raw(Box::new(AsyncHookHandle { index: 0 })) as i64; + let tracked = Box::into_raw(Box::new(AsyncHookHandle { index: 1 })) as i64; + for backing in [suppressed, tracked] { + ASYNC_HOOK_HANDLES.lock().unwrap().insert(backing); + ASYNC_HOOK_HANDLE_COUNT.fetch_add(1, Ordering::Relaxed); + } + js_async_hook_enable(suppressed); assert!(hooks_active()); assert!(!promise_hooks_active()); - js_async_hook_enable(&tracked as *const AsyncHookHandle as i64); + js_async_hook_enable(tracked); assert!(promise_hooks_active()); assert_eq!(enabled_callbacks(false).len(), 2); assert_eq!(enabled_callbacks(true).len(), 1); - js_async_hook_disable(&tracked as *const AsyncHookHandle as i64); + js_async_hook_disable(tracked); assert!(hooks_active()); assert!(!promise_hooks_active()); - js_async_hook_disable(&suppressed as *const AsyncHookHandle as i64); + js_async_hook_disable(suppressed); assert!(!hooks_active()); reset_for_tests(); } @@ -198,7 +211,13 @@ mod tests { crate::symbol::test_clear_symbol_side_table_roots(); let type_ptr = js_string_from_bytes(b"ExpandoResource".as_ptr(), 15); let type_value = crate::value::js_nanbox_string(type_ptr as i64); - let handle = js_async_resource_new(type_value, TAG_UNDEFINED_F64); + // #10926: `js_async_resource_new` hands JS the ordinary handle OBJECT + // now, not the raw backing. The subject here is still the NATIVE + // backing's expando side table, so resolve through the same entry + // point every caller uses and keep going with the backing. + let resource_object = js_async_resource_new(type_value, TAG_UNDEFINED_F64); + let handle = resolve_async_resource_handle(resource_object) + .expect("a freshly constructed AsyncResource must resolve to its backing"); assert!(is_async_resource_handle(handle)); let resource = crate::value::js_nanbox_pointer(handle); let symbol = unsafe { crate::symbol::js_symbol_new_empty() }; diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/hook_dispatch_handles.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/hook_dispatch_handles.rs index a635cf9329..220c7d74b8 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/hook_dispatch_handles.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/hook_dispatch_handles.rs @@ -5,7 +5,7 @@ extern "C" fn test_current_async_id(_closure: *const crate::closure::ClosureHead } #[test] -fn test_async_resource_subclass_run_in_scope_roots_inputs_during_key_alloc_gc() { +fn test_async_resource_subclass_run_in_scope_roots_inputs_across_a_resolve_gc() { let _async_hook_guard = AsyncHookRuntimeTestGuard::new(); let _guard = CopyingNurseryTestGuard::new(0); let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); @@ -14,13 +14,22 @@ fn test_async_resource_subclass_run_in_scope_roots_inputs_during_key_alloc_gc() register_runtime_handle_root_scanner_for_tests(); let resource_type = test_string_value(b"SubclassResource"); - let backing = crate::async_hooks::js_async_resource_new( + // #10926: `js_async_resource_new` hands back the handle OBJECT now. What a + // subclass receiver is linked to is the NATIVE backing behind it -- the + // address the registry brands -- so resolve it. Linking the object instead + // stores an address `is_async_resource_handle` rejects, and the resolve + // this test is about declines for a reason that has nothing to do with GC. + let resource_object = crate::async_hooks::js_async_resource_new( resource_type, f64::from_bits(crate::value::TAG_UNDEFINED), ); + let backing = crate::async_hooks::resolve_async_resource_handle(resource_object) + .expect("a freshly constructed AsyncResource must resolve to its backing"); let expected_async_id = crate::async_hooks::js_async_resource_async_id(backing); let receiver = crate::object::js_object_alloc(0, 1); - crate::async_hooks::test_link_async_resource_subclass(receiver, backing); + // The helper allocates (a key string, and the meta record the backing word + // lives in), so it can move the receiver; take the address it hands back. + let receiver = crate::async_hooks::test_link_async_resource_subclass(receiver, backing); let callback = crate::closure::js_closure_alloc(test_current_async_id as *const u8, 0); crate::async_hooks::test_force_next_async_resource_resolve_gc(); @@ -33,7 +42,10 @@ fn test_async_resource_subclass_run_in_scope_roots_inputs_during_key_alloc_gc() ); let after = crate::gc::copying_minor_cycles(); - assert!(after > before, "the resolver must complete a copying minor"); + assert!( + after > before, + "run_in_async_scope must complete a copying minor before it resolves" + ); assert_eq!(result, expected_async_id); assert_eq!(crate::async_hooks::execution_async_id_u64(), 0); } diff --git a/crates/perry-runtime/src/hot_diag/receiver_repr.rs b/crates/perry-runtime/src/hot_diag/receiver_repr.rs index 8eb0952a74..37c05bf2fa 100644 --- a/crates/perry-runtime/src/hot_diag/receiver_repr.rs +++ b/crates/perry-runtime/src/hot_diag/receiver_repr.rs @@ -213,15 +213,13 @@ fn observe_pointer(addr: usize) { // an arbitrary heap address is one of their ids took three mutexes to // answer "no". // Each remaining family deletes its arm here as it moves. - if crate::async_hooks::is_async_hook_handle(addr as i64) { - mark_old(ReceiverReprFamily::AsyncHook); - } - if crate::async_hooks::is_async_resource_handle(addr as i64) { - mark_old(ReceiverReprFamily::AsyncResource); - } // #340/#341 GATE A: `null_stub` has migrated to an ordinary object, so // its arm is gone from here, and `is_null_stub_address` with it: it could // only ever have answered for a `.data` static no longer handed to JS. + // #340/#341 GATE A: `async_hook` and `async_resource` have migrated to + // ordinary objects, so neither can hand a header-less `Box` to a funnel + // any more and their arms are gone from here. The fixtures below assert + // `observed_old == 0` for both. if crate::shared_sab::is_shared_sab(addr) { mark_old(ReceiverReprFamily::Sab); } @@ -463,22 +461,18 @@ mod tests { assert_fixture_migrated(ReceiverReprFamily::Tui, || { crate::tui::state::js_perry_tui_state_alloc(0.0) as usize }); - assert_fixture(ReceiverReprFamily::AsyncHook, || { + // #340/#341: `async_hook` is migrated — gate A, inverted (see `text`). + assert_fixture_migrated(ReceiverReprFamily::AsyncHook, || { let options = crate::object::js_object_alloc(0, 0); let value = f64::from_bits(crate::value::JSValue::pointer(options.cast()).bits()); - ( - crate::async_hooks::js_async_hooks_create_hook(value) as usize, - false, - ) + crate::async_hooks::js_async_hooks_create_hook(value) as usize }); - assert_fixture(ReceiverReprFamily::AsyncResource, || { + // #340/#341: `async_resource` is migrated — gate A, inverted. + assert_fixture_migrated(ReceiverReprFamily::AsyncResource, || { let name = crate::string::js_string_from_bytes(b"receiver-repr".as_ptr(), 13); let type_value = f64::from_bits(crate::value::js_nanbox_string(name as i64).to_bits()); let options = f64::from_bits(crate::value::TAG_UNDEFINED); - ( - crate::async_hooks::js_async_resource_new(type_value, options) as usize, - false, - ) + crate::async_hooks::js_async_resource_new(type_value, options) as usize }); assert_fixture(ReceiverReprFamily::SymbolGlobal, || { ( diff --git a/crates/perry-runtime/src/native_class_ids.rs b/crates/perry-runtime/src/native_class_ids.rs index d60c068239..15e00d65bb 100644 --- a/crates/perry-runtime/src/native_class_ids.rs +++ b/crates/perry-runtime/src/native_class_ids.rs @@ -50,12 +50,20 @@ pub(crate) const TUI_REF_BOX: u32 = 0xFFFF_240D; pub(crate) const TUI_APP: u32 = 0xFFFF_240E; pub(crate) const TUI_STDOUT: u32 = 0xFFFF_240F; pub(crate) const TUI_FOCUS_MANAGER: u32 = 0xFFFF_2410; +pub(crate) const ASYNC_HOOK: u32 = 0xFFFF_2411; + +/// #10926: `AsyncResource` is native-backed too, but keeps its LEGACY +/// `0xFFFF_0079`, which `instanceof` and `class_registry::parent_static` +/// already bake into emitted code -- moving a live class id is #10824's hazard +/// for no gain, so the range gets a legacy companion instead of a renumbering. +/// Outside the block, so it is not in `ALL`. +pub(crate) const ASYNC_RESOURCE_LEGACY: u32 = 0xFFFF_0079; /// The first id in the native-state range and the last one, inclusive. Every /// family between them carries a `native_state` word the far side of a /// `postMessage` could not reconstruct. const NATIVE_BACKED_FIRST: u32 = TEXT_ENCODER; -const NATIVE_BACKED_LAST: u32 = TUI_FOCUS_MANAGER; +const NATIVE_BACKED_LAST: u32 = ASYNC_HOOK; /// Class ids whose instances are ordinary objects carrying native state that /// cannot cross a thread boundary (#340/#341). @@ -68,6 +76,7 @@ const NATIVE_BACKED_LAST: u32 = TUI_FOCUS_MANAGER; /// `TypeError` #6185 made these surface. pub(crate) fn is_native_backed_class_id(class_id: u32) -> bool { (NATIVE_BACKED_FIRST..=NATIVE_BACKED_LAST).contains(&class_id) + || class_id == ASYNC_RESOURCE_LEGACY } /// Every id this module hands out, newest last. Used by the assertions below @@ -89,6 +98,7 @@ const ALL: &[u32] = &[ TUI_APP, TUI_STDOUT, TUI_FOCUS_MANAGER, + ASYNC_HOOK, ]; /// Strictly ascending ⟹ no two families share an id, and the block stays @@ -138,6 +148,8 @@ mod tests { TUI_APP, TUI_STDOUT, TUI_FOCUS_MANAGER, + ASYNC_HOOK, + ASYNC_RESOURCE_LEGACY, ] { assert!( is_native_backed_class_id(id), diff --git a/crates/perry-runtime/src/node_stream_constructors/builders.rs b/crates/perry-runtime/src/node_stream_constructors/builders.rs index 5f2cbc7a08..a7abbda2e4 100644 --- a/crates/perry-runtime/src/node_stream_constructors/builders.rs +++ b/crates/perry-runtime/src/node_stream_constructors/builders.rs @@ -172,17 +172,28 @@ pub extern "C" fn js_event_emitter_async_resource_subclass_init(this: f64, optio }; let resource = crate::async_hooks::js_async_resource_new(name_handle.get_nanbox_f64(), async_options); - let obj = this_handle.get_nanbox_f64(); - let raw = raw_ptr_from_value(obj); - crate::async_hooks::js_async_resource_set_event_emitter(resource, raw as i64); + // #10926: `resource` is the public AsyncResource OBJECT now -- ordinary and + // movable, where it used to be a never-freed `Box` -- and this frame is its + // only holder until it is stored below, while the hidden key allocates. + // Root it, and re-read `this` and the resource after that allocation. + let resource_handle = scope.root_nanbox_f64(f64::from_bits( + crate::value::js_nanbox_pointer(resource).to_bits(), + )); + crate::async_hooks::js_async_resource_set_event_emitter( + resource, + raw_ptr_from_value(this_handle.get_nanbox_f64()) as i64, + ); + let key = scope.root_string_ptr(hidden_key(EVENT_EMITTER_ASYNC_RESOURCE_KEY)); unsafe { - crate::object::js_object_set_field_by_name( - raw as *mut ObjectHeader, - hidden_key(EVENT_EMITTER_ASYNC_RESOURCE_KEY), - f64::from_bits(crate::value::js_nanbox_pointer(resource).to_bits()), - ); + key.with_const_ptr::(|key| { + crate::object::js_object_set_field_by_name( + raw_ptr_from_value(this_handle.get_nanbox_f64()) as *mut ObjectHeader, + key, + resource_handle.get_nanbox_f64(), + ) + }); install_event_emitter_async_resource_instance_methods( - raw as *mut ObjectHeader, + raw_ptr_from_value(this_handle.get_nanbox_f64()) as *mut ObjectHeader, this_handle.get_nanbox_f64(), ); } diff --git a/crates/perry-runtime/src/node_stream_dispatch.rs b/crates/perry-runtime/src/node_stream_dispatch.rs index a97122747a..bb883745b1 100644 --- a/crates/perry-runtime/src/node_stream_dispatch.rs +++ b/crates/perry-runtime/src/node_stream_dispatch.rs @@ -292,7 +292,13 @@ fn event_emitter_async_resource_backing(receiver: f64) -> Option> 48 == 0x7FFD { let resource = (value.to_bits() & crate::value::POINTER_MASK) as i64; - if crate::async_hooks::is_async_resource_handle(resource) { + // #10926: the hidden field holds what `js_async_resource_new` + // returned -- the public handle OBJECT, not the native backing + // -- so brand it by resolving, not by backing-registry + // membership. `resource` stays the public object: the + // `asyncResource` getter hands it to JS, and every + // `js_async_resource_*` entry point resolves it. + if crate::async_hooks::resolve_async_resource_handle(resource).is_some() { return Some(EventEmitterAsyncResourceBacking::RuntimeResource(resource)); } } diff --git a/crates/perry-runtime/src/object/class_registry.rs b/crates/perry-runtime/src/object/class_registry.rs index 9e9c6f8c9c..ca90f9c0ea 100644 --- a/crates/perry-runtime/src/object/class_registry.rs +++ b/crates/perry-runtime/src/object/class_registry.rs @@ -74,6 +74,7 @@ pub(crate) use accessor_attrs::{ }; // ── state.rs ──────────────────────────────────────────────────────────────── +pub(crate) use state::async_resource_prototype_value; #[cfg(test)] pub(crate) use state::class_decl_prototype_object_root_store; pub(crate) use state::{ diff --git a/crates/perry-runtime/src/object/class_registry/state.rs b/crates/perry-runtime/src/object/class_registry/state.rs index 027b9ac305..5fab2ff7e7 100644 --- a/crates/perry-runtime/src/object/class_registry/state.rs +++ b/crates/perry-runtime/src/object/class_registry/state.rs @@ -1043,6 +1043,18 @@ fn reserved_native_parent_prototype_bits(parent_id: u32) -> Option { class_parent_prototype_bits(parent_proto) } +/// `AsyncResource.prototype` as a NaN-boxed value, resolved exactly the way +/// the subclass edge above resolves it, so a direct `new AsyncResource(...)` +/// instance and a subclass prototype reach the SAME object by identity. +/// Returns `undefined` if the export is not materialized. +pub(crate) fn async_resource_prototype_value() -> f64 { + let func_value = super::super::native_module::bound_native_callable_export_value( + "async_hooks", + "AsyncResource", + ); + super::function_prototype::js_function_prototype_value_for_read(func_value) +} + pub(crate) fn class_decl_prototype_value(class_id: u32) -> f64 { // #7757: a specialization answers with its generic's prototype. let class_id = decl_prototype_identity_id(class_id); diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 127e306473..02d586e981 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -81,6 +81,7 @@ mod class_gc_roots; mod class_handles; pub mod class_image; mod class_registry; +pub(crate) use class_registry::async_resource_prototype_value; pub(crate) use class_registry::class_registry_census; #[cfg(feature = "regex-engine")] pub(crate) use class_registry::construct_two_rooted; diff --git a/crates/perry-stdlib/src/events.rs b/crates/perry-stdlib/src/events.rs index 2115929fd5..0c86d7d84d 100644 --- a/crates/perry-stdlib/src/events.rs +++ b/crates/perry-stdlib/src/events.rs @@ -312,7 +312,9 @@ pub struct EventEmitterHandle { /// Constructor-level `{ captureRejections: true }` flag. When enabled, /// rejected promises returned from listeners are routed to `"error"`. capture_rejections: bool, - /// Backing AsyncResource handle for EventEmitterAsyncResource instances. + /// The AsyncResource for EventEmitterAsyncResource instances: the public + /// handle OBJECT `js_async_resource_new` returns (#10926), a movable GC + /// object, so `scan_events_roots_mut` visits this slot. async_resource_handle: i64, pub(crate) domain_handle: Option, } @@ -364,6 +366,13 @@ fn scan_events_roots_mut(visitor: &mut perry_runtime::gc::RuntimeRootVisitor<'_> } } } + // #10926: an ordinary, movable object since AsyncResource stopped + // being a header-less `Box`. The emitter is its only holder on the + // native side, so without this a collection frees or moves it and the + // next `emit` resolves a dangling address. + if emitter.async_resource_handle != 0 { + visitor.visit_i64_slot(&mut emitter.async_resource_handle); + } for pending in emitter.pending_once_promises.values_mut() { for p in pending.iter_mut() { visitor.visit_raw_mut_ptr_slot(&mut p.promise); diff --git a/crates/perry/tests/async_resource_object_surface.rs b/crates/perry/tests/async_resource_object_surface.rs new file mode 100644 index 0000000000..7103f6d971 --- /dev/null +++ b/crates/perry/tests/async_resource_object_surface.rs @@ -0,0 +1,210 @@ +//! #10926 / honest-tags row 13 — `AsyncResource` and `AsyncHook` are ordinary +//! objects, and a subclass inherits through the prototype chain. +//! +//! Two bugs, one representation. The DIRECT path +//! (`new AsyncResource(...)`, `createHook(...)`) handed JS a raw +//! `Box::into_raw` address with no `GcHeader`, so `JSON.stringify` dispatched +//! on whatever bytes preceded the `Box` and answered `""` (#10926). The +//! SUBCLASS path compensated for a missing prototype link by copying five +//! methods onto every instance as own properties and stashing that same +//! header-less `Box` in a user-visible `__perryAsyncResourceBacking` field — +//! so `Object.keys(new R())` leaked it and `JSON.stringify` serialised the +//! header-less value. +//! +//! The leak was the symptom. The cause is that +//! `Object.getPrototypeOf(R.prototype) === AsyncResource.prototype` is false: +//! `reserved_native_parent_prototype_bits` wires `EventEmitter` and +//! `EventEmitterAsyncResource` and simply never got an `AsyncResource` arm. +//! Linking the chain deletes the copies and the field. +//! +//! THIS HALF fixes the DIRECT path only: `new AsyncResource(...)` and +//! `createHook(...)` become ordinary objects, so `direct-json`, `hook-json` +//! and `nested` match node. The SUBCLASS half is HELD: linking +//! `R.prototype.[[Prototype]]` to `AsyncResource.prototype` needs a codegen +//! condition in `perry-codegen/src/expr/property_get.rs:1351`, which matches +//! `class_name == "AsyncResource"` exactly and so misses a subclass; widening +//! it belongs to the lane already restructuring that file. Until then +//! `sub-keys`, `sub-gopn`, `sub-json` and `proto-chain` keep their CURRENT +//! (node-divergent) values here, marked below, so this test states the truth +//! rather than an aspiration. +//! +//! MUST-FAIL: committed BEFORE the fix. On v0.5.1633 the three direct lines +//! differ; with this half they match node 26.8.1. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(dir: &std::path::Path, source: &str) -> String { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + let run = Command::new(&output) + .current_dir(dir) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status.code(), + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +const PROGRAM: &str = r#" +import { AsyncResource, createHook } from "node:async_hooks"; +class MyRes extends AsyncResource { constructor() { super("MYRES"); } } +const AR: any = AsyncResource; +const sub: any = new MyRes(); +const direct: any = new AsyncResource("DIRECT"); +const hook: any = createHook({ init() {} }); +console.log("sub-keys", JSON.stringify(Object.keys(sub))); +console.log("sub-gopn", JSON.stringify(Object.getOwnPropertyNames(sub).sort())); +console.log("sub-json", JSON.stringify(sub)); +console.log("direct-keys", JSON.stringify(Object.keys(direct))); +console.log("direct-json", JSON.stringify(direct)); +console.log("hook-json", JSON.stringify(hook)); +console.log("nested", JSON.stringify({ a: direct, b: hook })); +console.log("proto-chain", Object.getPrototypeOf(MyRes.prototype) === AR.prototype); +console.log("instanceof", sub instanceof AsyncResource, sub instanceof MyRes, direct instanceof AsyncResource); +console.log("typeof", typeof sub, typeof direct, typeof hook); +console.log("asyncId", typeof sub.asyncId(), typeof direct.asyncId()); +console.log("trigger", typeof sub.triggerAsyncId()); +let ran = 0; +sub.runInAsyncScope(() => { ran++; }); +direct.runInAsyncScope(() => { ran++; }); +console.log("runInAsyncScope", ran); +console.log("bind", typeof sub.bind(() => 1)); +hook.enable(); hook.disable(); +console.log("hook-methods", typeof hook.enable, typeof hook.disable); +console.log("identity", sub === sub, direct === new AsyncResource("OTHER")); +const m = new Map([[direct, 1], [hook, 2]]); +console.log("map", m.size, m.get(direct), m.get(hook)); +"#; + +/// Every line is node 26.8.1's. Seven differ on v0.5.1633: +/// `sub-keys`, `sub-gopn`, `sub-json`, `direct-json`, `hook-json`, `nested` +/// and `proto-chain`. The other ten are already correct and are pinned here so +/// the representation change cannot quietly break them — `instanceof`, +/// `typeof`, the method results, identity and `Map` keys all have to survive. +#[test] +fn async_resource_and_hook_match_nodes_object_surface() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run(dir.path(), PROGRAM); + assert_eq!( + stdout, + // The first three lines and `proto-chain` are HELD (subclass half, + // pending the codegen widening at `property_get.rs:1351`) -- node + // gives `[]`, `[]`, `{}` and `true` for those four. `direct-keys`, + // `direct-json`, `hook-json` and `nested` are what THIS half fixes. + // Do not put a `//` comment inside the literal below: the lines end + // in `\` continuations, so it would become expected output. + "sub-keys [\"__perryAsyncResourceBacking\"]\n\ + sub-gopn [\"__perryAsyncResourceBacking\",\"asyncId\",\"bind\",\"emitDestroy\",\"runInAsyncScope\",\"triggerAsyncId\"]\n\ + sub-json {\"__perryAsyncResourceBacking\":\"\"}\n\ + direct-keys []\n\ + direct-json {}\n\ + hook-json {}\n\ + nested {\"a\":{},\"b\":{}}\n\ + proto-chain false\n\ + instanceof true true true\n\ + typeof object object object\n\ + asyncId number number\n\ + trigger number\n\ + runInAsyncScope 2\n\ + bind function\n\ + hook-methods function function\n\ + identity true false\n\ + map 2 1 2\n" + ); +} + +/// #10926 regression -- the resolver may not re-enter the property path. +/// +/// `js_object_get_field_by_name` calls `try_async_resource_property_dispatch` +/// for EVERY receiver, and #10926 changed that entry point to RESOLVE the +/// receiver where it used to identity-check it. A resolver that reads an own +/// property therefore closes a cycle: `get_field_by_name` -> dispatch -> +/// `resolve_async_resource_handle` -> `get_field_by_name`. The key it reads +/// (`__perryAsyncResourceBacking`) is absent on ordinary objects, so the inner +/// lookup always misses and re-enters. Merely LINKING `node:async_hooks` was +/// then fatal: the first property miss in the program exhausted the 8 MB stack +/// and the binary died with SIGSEGV before printing anything. The first draft +/// of this split did exactly that. +#[test] +fn a_property_miss_does_not_recurse_once_async_hooks_is_linked() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + r#" +import { AsyncResource } from "node:async_hooks"; +const plain: any = { kept: 1 }; +console.log("miss-plain", plain.absent === undefined, plain.__perryAsyncResourceBacking === undefined); +const res: any = new AsyncResource("PROBE"); +console.log("miss-resource", res.absent === undefined, typeof res.asyncId()); +console.log("linked", typeof AsyncResource, plain.kept); +"#, + ); + assert_eq!( + stdout, + "miss-plain true true\n\ + miss-resource true number\n\ + linked function 1\n" + ); +} + +/// The resources must survive a collection: the handle is an ordinary movable +/// object now, and its backing record is reached through `ObjectMeta`. A probe +/// that only called the methods immediately after construction would not cover +/// the axis the representation changes. +#[test] +fn async_resources_survive_a_collection() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + r#" +import { AsyncResource, createHook } from "node:async_hooks"; +class MyRes extends AsyncResource { constructor() { super("MYRES"); } } +const sub: any = new MyRes(); +const direct: any = new AsyncResource("DIRECT"); +const hook: any = createHook({ init() {} }); +const idBefore = sub.asyncId(); +let sink: any[] = []; +for (let i = 0; i < 200000; i++) { sink.push({ i: i, j: [i, i + 1] }); } +sink = []; +console.log("stable-id", sub.asyncId() === idBefore); +let ran = 0; +sub.runInAsyncScope(() => { ran++; }); +direct.runInAsyncScope(() => { ran++; }); +console.log("after-gc", ran, typeof direct.asyncId(), typeof hook.enable); +console.log("surface", JSON.stringify(sub), JSON.stringify(direct)); +"#, + ); + assert_eq!( + stdout, + // `sub` still carries the held own property, so its JSON is the + // subclass half's business; `direct` is this half's. + "stable-id true\n\ + after-gc 2 number function\n\ + surface {\"__perryAsyncResourceBacking\":\"\"} {}\n" + ); +} diff --git a/scripts/addr_class_ratchet_baseline.txt b/scripts/addr_class_ratchet_baseline.txt index 80009e19ad..4416e30c8c 100644 --- a/scripts/addr_class_ratchet_baseline.txt +++ b/scripts/addr_class_ratchet_baseline.txt @@ -36,7 +36,6 @@ handle-floor | crates/perry-runtime/src/array/indexing_proto_chain.rs | 1 handle-floor | crates/perry-runtime/src/array/iter_object.rs | 1 handle-floor | crates/perry-runtime/src/array/iterator.rs | 2 handle-floor | crates/perry-runtime/src/array/push_pop.rs | 1 -handle-floor | crates/perry-runtime/src/async_hooks.rs | 1 handle-floor | crates/perry-runtime/src/bigint/convert.rs | 2 handle-floor | crates/perry-runtime/src/bigint/mod.rs | 2 handle-floor | crates/perry-runtime/src/box.rs | 2 diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index 0cd5f20927..bce18560d7 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -203,12 +203,6 @@ "verdict": "not_a_gc_pointer", "why": "Monotonic counter of live async-resource handles. Holds no address at all; the resource objects live in RESOURCES, which scan_async_hooks_roots_mut visits." }, - { - "file": "crates/perry-runtime/src/async_hooks.rs", - "name": "TEST_FORCE_RESOLVE_GC", - "verdict": "test_only", - "why": "#[cfg(test)] AtomicUsize one-shot flag that asks resolve_async_resource_handle to force a collection; stores only 0 or 1 and is absent from shipped binaries." - }, { "file": "crates/perry-runtime/src/box.rs", "name": "BOX_YOUNG_ROOTS", diff --git a/test-files/test_gap_10952_eventemitter_async_resource_object.ts b/test-files/test_gap_10952_eventemitter_async_resource_object.ts new file mode 100644 index 0000000000..b5e53627b3 --- /dev/null +++ b/test-files/test_gap_10952_eventemitter_async_resource_object.ts @@ -0,0 +1,52 @@ +// #10952 / #10926: once `new AsyncResource(...)` returns an ordinary handle +// OBJECT instead of the raw native `Box`, every native caller that stored +// `js_async_resource_new`'s result and then branded it by BACKING-registry +// membership stopped matching: +// * `set_async_resource_event_emitter` silently dropped the link, so +// `eear.asyncResource.eventEmitter` was `undefined` (direct path); +// * the EventEmitterAsyncResource subclass brand check in +// `node_stream_dispatch` rejected its own hidden resource, so `emit` threw +// "Cannot read private member ..." (subclass path). +// The stdlib emitter also cached the (now movable) resource object in a slot +// no GC scanner visited; the churn below crosses collections before re-reading. +import { EventEmitterAsyncResource } from "node:events"; +import { executionAsyncId, AsyncResource } from "node:async_hooks"; + +const e: any = new EventEmitterAsyncResource({ name: "PROBE" }); +const id = e.asyncId; +console.log("id-positive", typeof id === "number" && id > 0); +console.log("trigger-number", typeof e.triggerAsyncId); +const r: any = e.asyncResource; +console.log("resource-is-AR", r instanceof AsyncResource); +console.log("resource-asyncId-matches", r.asyncId() === id); +console.log("resource-emitter", r.eventEmitter === e); +let inside = -1; +e.on("x", () => { inside = executionAsyncId(); }); +e.emit("x"); +console.log("listener-in-scope", inside === id); + +class Sub extends EventEmitterAsyncResource { constructor() { super({ name: "SUB" }); } } +const s: any = new Sub(); +const sid = s.asyncId; +console.log("sub-id-positive", typeof sid === "number" && sid > 0); +let sinside = -1; +s.on("y", () => { sinside = executionAsyncId(); }); +s.emit("y"); +console.log("sub-listener-in-scope", sinside === sid); +console.log("sub-resource-emitter", s.asyncResource.eventEmitter === s); + +// churn: the emitter's resource must survive collections +const junk: any[] = []; +for (let i = 0; i < 200000; i++) { junk.push({ i, s: "x" + i }); if (junk.length > 1000) junk.length = 0; } +if (typeof (globalThis as any).gc === "function") (globalThis as any).gc(); +console.log("after-gc-id", e.asyncId === id, s.asyncId === sid); +inside = -1; e.emit("x"); +console.log("after-gc-listener-in-scope", inside === id); +sinside = -1; s.emit("y"); +console.log("after-gc-sub-listener-in-scope", sinside === sid); +// (The SUBCLASS back-reference after a collection is not asserted: the native +// backing caches the subclass `this` as a raw address no scanner visits, which +// predates #10926 and is tracked separately.) +console.log("after-gc-resource-emitter", e.asyncResource.eventEmitter === e); +e.emitDestroy(); +console.log("done");