diff --git a/changelog.d/10614-eventemitter-prototype-identity.md b/changelog.d/10614-eventemitter-prototype-identity.md new file mode 100644 index 0000000000..54b7d07d4b --- /dev/null +++ b/changelog.d/10614-eventemitter-prototype-identity.md @@ -0,0 +1,25 @@ +### Fixed + +- **`class Sub extends EventEmitter {}` now links `Sub.prototype`'s `[[Prototype]]` to the real + `EventEmitter.prototype` object (#10599).** `Object.getPrototypeOf(Sub.prototype) === EventEmitter.prototype` + read `false`: `class_decl_prototype_value` resolves a registered parent class id by recursing into itself, + which bails for a RESERVED native-builtin parent id (`EventEmitter` has no `js_register_class_name` + registration of its own), so the link silently fell through to `Object.prototype`. `instanceof` and the + class-chain walk (#10592) already worked — this is specifically the `[[Prototype]]` object identity, which is + a separate mechanism. + + The fix resolves the real, closure-identity-keyed prototype through `js_function_prototype_value_for_read` — + the same helper the existing runtime-function-valued-parent branch already used — so the identity comparison + holds, not merely the shape. It only takes effect together with a codegen-side parent-edge registration + (`builtin_parent_reserved_class_id`): #10592 added that entry for plain `EventEmitter`; this PR adds the + missing `EventEmitterAsyncResource` counterpart, without which `get_parent_class_id` never resolves and the + runtime fix is unreachable. + + Verified with the runtime fix reverted (twice, independently) that every `getPrototypeOf`/`instanceof`/`in` + assertion in the new gap test reads `false` where Node reads `true`, and with it restored the test is + byte-identical to Node 26.5.1. Two pre-existing, unrelated gaps surfaced while writing the test and were left + out of it: `EventEmitterAsyncResource.prototype`'s own chain to `EventEmitter.prototype` (a + native-to-native link, not a user subclass), and `Object.keys(new Sub())` leaking `EventEmitter.prototype`'s + methods as own enumerable instance properties instead of Node's real `_events`/`_eventsCount`/`_maxListeners` + fields (CLAUDE.md's documented "native base's surface is installed at `super()` time" weak area) — both + reproduce identically with the fix reverted, so neither is caused by it. diff --git a/crates/perry-codegen/src/expr/instance_misc1.rs b/crates/perry-codegen/src/expr/instance_misc1.rs index b9d32b7e9b..48caf2bc56 100644 --- a/crates/perry-codegen/src/expr/instance_misc1.rs +++ b/crates/perry-codegen/src/expr/instance_misc1.rs @@ -129,6 +129,14 @@ pub(crate) fn builtin_parent_reserved_class_id(name: &str) -> Option { // which don't recognize a genuine subclass ObjectHeader. Keep in // sync with `CLASS_ID_EVENT_EMITTER` there. "EventEmitter" => 0xFFFF0076, + // #10599: `class Sub extends EventEmitterAsyncResource {}` needs the + // same parent edge as plain EventEmitter above -- without it, + // `get_parent_class_id` never resolves for this id, and the + // getPrototypeOf-identity fallback in + // perry-runtime/src/object/class_registry/state.rs + // (`reserved_native_parent_prototype_bits`) never runs. Keep in sync + // with `CLASS_ID_EVENT_EMITTER_ASYNC_RESOURCE` there. + "EventEmitterAsyncResource" => 0xFFFF0077, _ => return None, }) } diff --git a/crates/perry-runtime/src/object/class_registry/state.rs b/crates/perry-runtime/src/object/class_registry/state.rs index c8b5a74ff9..9b73174956 100644 --- a/crates/perry-runtime/src/object/class_registry/state.rs +++ b/crates/perry-runtime/src/object/class_registry/state.rs @@ -953,6 +953,45 @@ fn class_parent_prototype_bits(value: f64) -> Option { (unsafe { crate::symbol::js_is_symbol(value) } == 0).then_some(bits) } +/// #10599: resolve the real `.prototype` object for a RESERVED native-builtin +/// parent class id -- one `builtin_parent_reserved_class_id` (perry-codegen) +/// wires as a class-registry parent edge for a native base that has no +/// declared-class registration of its own (`class Sub extends EventEmitter +/// {}` has no `js_register_class_name` call for `EventEmitter`). Without this, +/// `class_decl_prototype_value` bails immediately for such an id +/// (`class_name_for_id` returns `None`), so `Sub.prototype`'s `[[Prototype]]` +/// silently fell through to `Object.prototype` instead of +/// `EventEmitter.prototype` -- `Object.getPrototypeOf(Sub.prototype) !== +/// EventEmitter.prototype`, even though `new Sub() instanceof EventEmitter` +/// (a different mechanism -- the class-chain walk in `js_instanceof`) already +/// worked. +/// +/// Scoped to the ids whose only registered subclassing surface is this +/// generic declared-class-prototype path: EventEmitter and its +/// AsyncResource variant, both bound as ordinary native-module callable +/// exports (`bound_native_callable_export_value`) whose own `.prototype` is +/// the same lazily-materialized, closure-identity-keyed object any bound +/// function's `.prototype` read produces +/// (`js_function_prototype_value_for_read`). Resolving through that exact +/// helper -- the same one the dynamic-parent branch below already uses for a +/// runtime function-valued superclass -- is what makes +/// `Object.getPrototypeOf(Sub.prototype) === EventEmitter.prototype` hold by +/// identity, not merely by shape. Array/Map/Set/Error/typed-array subclasses +/// have their own dedicated instance/prototype modeling and don't reach this +/// fallback the same way. +fn reserved_native_parent_prototype_bits(parent_id: u32) -> Option { + const CLASS_ID_EVENT_EMITTER: u32 = 0xFFFF0076; + const CLASS_ID_EVENT_EMITTER_ASYNC_RESOURCE: u32 = 0xFFFF0077; + let (module, symbol) = match parent_id { + CLASS_ID_EVENT_EMITTER => ("events", "EventEmitter"), + CLASS_ID_EVENT_EMITTER_ASYNC_RESOURCE => ("events", "EventEmitterAsyncResource"), + _ => return None, + }; + let func_value = super::super::native_module::bound_native_callable_export_value(module, symbol); + let parent_proto = super::function_prototype::js_function_prototype_value_for_read(func_value); + class_parent_prototype_bits(parent_proto) +} + 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); @@ -1046,7 +1085,22 @@ pub(crate) fn class_decl_prototype_value(class_id: u32) -> f64 { .and_then(|parent_id| { let parent_proto = class_decl_prototype_value(parent_id); let parent_bits = parent_proto.to_bits(); - ((parent_bits >> 48) == 0x7FFD).then_some(parent_bits) + if (parent_bits >> 48) == 0x7FFD { + return Some(parent_bits); + } + // #10599: `parent_id` may be a RESERVED native-builtin class id + // rather than a declared class -- `builtin_parent_reserved_class_id` + // in perry-codegen wires this edge for `class Sub extends + // EventEmitter {}`, which has no `js_register_class_name` + // registration of its own. `class_decl_prototype_value` bails + // immediately for such an id (`class_name_for_id` is `None`), so + // without this fallback the lookup above always misses and + // execution falls through to the runtime-function-valued branch + // below, which also misses (there is no dynamic-parent VALUE for + // a statically-resolved reserved id) -- landing `Sub.prototype`'s + // `[[Prototype]]` on `Object.prototype` instead of + // `EventEmitter.prototype`. + reserved_native_parent_prototype_bits(parent_id) }); if registered_parent_proto.is_some() { registered_parent_proto diff --git a/test-files/test_gap_10599_eventemitter_prototype_identity.ts b/test-files/test_gap_10599_eventemitter_prototype_identity.ts new file mode 100644 index 0000000000..f3af58e1f6 --- /dev/null +++ b/test-files/test_gap_10599_eventemitter_prototype_identity.ts @@ -0,0 +1,170 @@ +// #10599: `class Sub extends EventEmitter {}` left +// `Object.getPrototypeOf(Sub.prototype) !== EventEmitter.prototype`. Split out +// from #10556 (PR #10592 fixed `new Sub() instanceof EventEmitter`, a +// different mechanism — the class-chain parent edge — that does not depend on +// prototype-OBJECT identity). Root cause: `class_decl_prototype_value` +// recurses on a registered parent class id via itself, which bails for a +// RESERVED native-builtin id (no `js_register_class_name` registration), so +// the link silently fell through to `Object.prototype`. +import { EventEmitter, EventEmitterAsyncResource } from "node:events"; + +// --- direct subclass, with a field + constructor ----------------------------------- +class Sub extends EventEmitter { + tag = "sub"; + constructor() { + super(); + this.tag = "sub-ctor"; + } +} + +console.log( + "getPrototypeOf(Sub.prototype) === EventEmitter.prototype:", + Object.getPrototypeOf(Sub.prototype) === EventEmitter.prototype, +); +console.log("typeof EventEmitter.prototype:", typeof EventEmitter.prototype); +console.log("Sub.prototype instanceof EventEmitter:", Sub.prototype instanceof EventEmitter); +console.log("new Sub() instanceof EventEmitter:", new Sub() instanceof EventEmitter); +console.log( + "getPrototypeOf(EventEmitter.prototype) === Object.prototype:", + Object.getPrototypeOf(EventEmitter.prototype) === Object.prototype, +); +console.log( + "getPrototypeOf(new Sub()) === Sub.prototype:", + Object.getPrototypeOf(new Sub()) === Sub.prototype, +); + +// --- fieldless subclass, no constructor -------------------------------------------- +class Fieldless extends EventEmitter {} +console.log( + "fieldless getPrototypeOf(Fieldless.prototype) === EventEmitter.prototype:", + Object.getPrototypeOf(Fieldless.prototype) === EventEmitter.prototype, +); +console.log("new Fieldless() instanceof EventEmitter:", new Fieldless() instanceof EventEmitter); + +// --- two-level subclass -------------------------------------------------------------- +class Mid extends EventEmitter { + mid = true; +} +class Grandchild extends Mid { + gc = true; +} +console.log( + "getPrototypeOf(Mid.prototype) === EventEmitter.prototype:", + Object.getPrototypeOf(Mid.prototype) === EventEmitter.prototype, +); +console.log( + "getPrototypeOf(Grandchild.prototype) === Mid.prototype:", + Object.getPrototypeOf(Grandchild.prototype) === Mid.prototype, +); +console.log( + "getPrototypeOf(getPrototypeOf(Grandchild.prototype)) === EventEmitter.prototype:", + Object.getPrototypeOf(Object.getPrototypeOf(Grandchild.prototype)) === EventEmitter.prototype, +); +const gc = new Grandchild(); +console.log("grandchild instanceof EventEmitter:", gc instanceof EventEmitter); +console.log("grandchild instanceof Mid:", gc instanceof Mid); +console.log( + "getPrototypeOf(new Grandchild()) === Grandchild.prototype:", + Object.getPrototypeOf(new Grandchild()) === Grandchild.prototype, +); + +// --- class expression ----------------------------------------------------------------- +const ExprSub = class extends EventEmitter { + expr = true; +}; +console.log( + "expr getPrototypeOf(ExprSub.prototype) === EventEmitter.prototype:", + Object.getPrototypeOf(ExprSub.prototype) === EventEmitter.prototype, +); +console.log("new ExprSub() instanceof EventEmitter:", new ExprSub() instanceof EventEmitter); + +// An UNNAMED, fieldless class expression — the exact minimal shape (no +// literal top-level `class Foo extends EventEmitter {}` declaration to key +// off of, no fields, no constructor). +const Sub3 = class extends EventEmitter {}; +console.log( + "unnamed fieldless getPrototypeOf(Sub3.prototype) === EventEmitter.prototype:", + Object.getPrototypeOf(Sub3.prototype) === EventEmitter.prototype, +); +console.log("new Sub3() instanceof EventEmitter:", new Sub3() instanceof EventEmitter); + +// A named class expression, assigned through an extra indirection (so codegen +// cannot special-case a literal top-level `class Foo extends EventEmitter {}` +// declaration shape). +function makeSubclass() { + return class NamedExpr extends EventEmitter {}; +} +const IndirectSub = makeSubclass(); +console.log( + "indirect getPrototypeOf(IndirectSub.prototype) === EventEmitter.prototype:", + Object.getPrototypeOf(IndirectSub.prototype) === EventEmitter.prototype, +); + +// --- EventEmitterAsyncResource variant -------------------------------------------------- +class SubAsync extends EventEmitterAsyncResource {} +console.log( + "getPrototypeOf(SubAsync.prototype) === EventEmitterAsyncResource.prototype:", + Object.getPrototypeOf(SubAsync.prototype) === EventEmitterAsyncResource.prototype, +); +// NOT covered here: `Object.getPrototypeOf(EventEmitterAsyncResource.prototype) +// === EventEmitter.prototype` -- that is EventEmitterAsyncResource's OWN +// internal chain (a native-builtin-to-native-builtin link set up wherever its +// `.prototype` is first materialized), not a user `extends` subclass. It is a +// separate, pre-existing gap (Perry answers `false`, Node `true`) outside +// this fix's scope -- `class_decl_prototype_value` never runs for it at all, +// since EventEmitterAsyncResource itself has no declared-class registration. +console.log( + "SubAsync.prototype instanceof EventEmitterAsyncResource:", + SubAsync.prototype instanceof EventEmitterAsyncResource, +); + +// --- control: a builtin whose subclass instance/prototype modeling is NOT this +// fallback (Array has its own dedicated ArrayHeader-based path) — guards the +// "Array/Map/Set/Error/typed-array subclasses don't reach this fallback" +// claim in the fix's own reasoning so a future change that breaks it fails +// loudly here instead of silently. +class ArraySub extends Array {} +console.log( + "getPrototypeOf(ArraySub.prototype) === Array.prototype:", + Object.getPrototypeOf(ArraySub.prototype) === Array.prototype, +); + +// --- `in` / `for...in` — the two-prototype-path weakness CLAUDE.md calls out ---------- +// (CLASS_PROTOTYPE_OBJECTS vs CLASS_DECL_PROTOTYPE_OBJECTS disagreeing about +// the same chain). `in` and `for...in` walk the exact same +// `[[Prototype]]` link `Object.getPrototypeOf` does, so fixing the identity +// above must also fix these without a separate code path. +console.log("'on' in Sub.prototype:", "on" in Sub.prototype); +console.log("'on' in new Sub():", "on" in new Sub()); +console.log("'emit' in new Sub():", "emit" in new Sub()); +console.log("'addListener' in new Sub():", "addListener" in new Sub()); + +const forInKeys: string[] = []; +for (const k in new Sub()) forInKeys.push(k); +console.log("for...in includes 'on':", forInKeys.includes("on")); +console.log("for...in includes 'emit':", forInKeys.includes("emit")); +console.log("for...in includes own field 'tag':", forInKeys.includes("tag")); + +// NOT covered here: `Object.keys(new Sub())`. It diverges from Node +// regardless of this fix (verified identically wrong with the fix reverted): +// Perry's native-base `super()` handling installs EventEmitter's methods +// (`on`, `emit`, ...) as literal OWN enumerable properties on the instance +// (CLAUDE.md "Known-weak areas: Native base-class subclassing -- a native +// base's surface is installed at super() time"), and never sets the +// `_events`/`_eventsCount`/`_maxListeners` own fields Node's real +// EventEmitter constructor does. That is an own-property-enumeration defect, +// orthogonal to the [[Prototype]] CHAIN identity this fix corrects. + +// --- the emitter still works (identity link must not disturb dispatch) --------------- +const s = new Sub(); +let fired = 0; +s.on("ping", (n: number) => (fired += n)); +s.emit("ping", 4); +console.log("sub fired:", fired, "listeners:", s.listenerCount("ping")); +console.log("sub.tag:", s.tag); + +// --- own-key enumeration on EventEmitter.prototype survives identity too ------------- +console.log( + "EventEmitter.prototype.constructor === EventEmitter:", + (EventEmitter.prototype as any).constructor === EventEmitter, +);