diff --git a/changelog.d/7981-thread-parent-class-id-from-registry.md b/changelog.d/7981-thread-parent-class-id-from-registry.md new file mode 100644 index 0000000000..f53cee7e7a --- /dev/null +++ b/changelog.d/7981-thread-parent-class-id-from-registry.md @@ -0,0 +1,45 @@ +### `perry/thread` no longer replays a plain object's ShapeId stamp as a class-parent edge + +`ObjectHeader.parent_class_id` carries two different things: the parent class id +for a class instance, and — since #6759 C3c — the runtime **ShapeId stamp** for a +plain object (`class_id == 0`), written lazily by every by-name resolve path. + +`thread.rs::serialize_object` copied the raw word, and the worker-side +deserializer hands it to `js_object_alloc_with_parent`, which does +`if parent_class_id != 0 { register_class(class_id, parent_class_id) }`. So any +object literal that had been read once and then crossed a `spawn` / +`parallelMap` boundary registered `class 0 → ` in the process-global +class-parent registry (`PARENT_DENSE[0] = shape_id + 1`, +`CLASS_REGISTRY[0] = shape_id`) and bumped the store-plan epoch, once per +deserialized object. Every consumer checked guards `class_id == 0` off, so no +live victim was found — but it is registry pollution reachable from ordinary +user code. + +The authoritative parent edge never lived in the header. Every parent-chain walk +reads `get_parent_class_id(class_id)`, and each edge is registered from a +compile-time constant: by `js_register_class_parent` in the module-init prelude +for codegen's inline `new C()` path (which writes the header word and +deliberately skips the per-alloc `register_class`), and by `register_class` +inside every runtime allocator that takes a `parent_class_id` argument. The +serializer now reads it from there. + +This also removes the **last consumer of the header word as inheritance data** — +the blocking dependency for #6759 Phase C3's unification of class layouts and +plain-object shapes into one shape-id space, which is itself the prerequisite for +#7916's header shrink (`class_field_inline_guard` can only trade its +`keys_array`-identity compare for a one-word ShapeId compare once a class +instance has a shape word). + +Three tests, each written to fail for a stated reason: + +- the stamp is asserted present in the fixture *before* asserting it does not + reach the wire, so the test cannot pass vacuously; +- a class instance's parent still round-trips *from the registry* with the header + word deliberately overwritten by a stamp, so the fix is not "always send 0"; +- `object/delete_rest.rs::shape_transition_tests_6759` pins both halves of the C3 + entry gate — a plain object's `delete` mints a genuinely different ShapeId, + while a class instance's `delete` compacts its slots leaving `class_id` **and** + `parent_class_id` untouched. The second goes red when a future rung gives class + instances a stamp, which is the intended signal to switch the guard. + +Refs #6759, #7916. diff --git a/crates/perry-runtime/src/object/delete_rest.rs b/crates/perry-runtime/src/object/delete_rest.rs index 120821e828..6a2aa424bc 100644 --- a/crates/perry-runtime/src/object/delete_rest.rs +++ b/crates/perry-runtime/src/object/delete_rest.rs @@ -601,6 +601,135 @@ pub extern "C" fn js_object_rest( } } +#[cfg(test)] +mod shape_transition_tests_6759 { + //! #6759 C3: what a `delete` does to an object's SHAPE IDENTITY, pinned for + //! both object representations — because they differ, and the difference is + //! the entry gate for the header shrink (#7916). + //! + //! `perry-codegen`'s `class_field_inline_guard` speculates that a receiver's + //! packed slot layout is its class's canonical one. `delete` breaks that + //! (slots after the deleted key shift down one) while PRESERVING + //! `class_id`, so the guard compares the live `keys_array` POINTER against + //! the class's `@perry_class_keys_*` token. Replacing that pointer compare + //! with a one-word ShapeId compare — which is what makes the header shrink + //! a load *removal* instead of a load-for-probe trade — requires a class + //! instance to HAVE a shape word. It does not: `parent_class_id` is the + //! shape word only when `class_id == 0`. + //! + //! These tests state both facts so that a future change to either is a + //! deliberate edit rather than a silent drift. + use super::*; + use crate::object::shapes::is_shape_id; + + fn key(name: &str) -> *mut crate::StringHeader { + crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32) + } + + /// A plain object's `delete` DOES mint a new ShapeId — lazily. The record + /// for the compacted keys array is dropped (`shape_drop`) and the stamp + /// cleared, so the next resolve allocates a genuinely fresh id rather than + /// reviving the old one. Ids are never reused, so "different" is the whole + /// property. + #[test] + fn delete_mints_a_fresh_shape_id_for_a_plain_object() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let obj = crate::object::js_object_alloc(0, 8); + for name in ["del6759_a", "del6759_b", "del6759_c"] { + crate::object::js_object_set_field_by_name(obj, key(name), 1.0); + } + let _ = crate::object::js_object_get_field_by_name(obj, key("del6759_b")); + let before = (*obj).parent_class_id; + assert!( + is_shape_id(before), + "fixture is vacuous — no shape stamp to transition (got {before:#x})" + ); + + assert_eq!(js_object_delete_field(obj, key("del6759_a")), 1); + + // The stamp is cleared eagerly … + assert_eq!( + (*obj).parent_class_id, + 0, + "the stamp still describes the PRE-delete key list" + ); + // … and re-minted, distinctly, on the next resolve. + let _ = crate::object::js_object_get_field_by_name(obj, key("del6759_c")); + let after = (*obj).parent_class_id; + assert!( + is_shape_id(after), + "no shape id re-minted after the delete (got {after:#x})" + ); + assert_ne!( + after, before, + "the delete re-used the pre-delete ShapeId — a shape-id compare \ + would accept a compacted object as its own class's shape" + ); + } + } + + /// The gap. A class instance has NO shape word to mint into: `class_id` is + /// preserved by design (it is the vtable/instanceof identity) and + /// `parent_class_id` is inheritance data, so the ONLY header evidence that a + /// compaction happened is the `keys_array` pointer. That is why + /// `class_field_inline_guard` loads it, and why the #7916 header shrink + /// cannot delete it before #6759 C3 gives class instances a shape word. + #[test] + fn delete_leaves_a_class_instance_with_no_shape_word_to_transition() { + let _lock = crate::gc::global_side_table_test_lock(); + const CID: u32 = 0x0C3C_6760; + const PARENT: u32 = 0x0C3C_6761; + let packed = b"del6759_x\0del6759_y\0del6759_z"; + unsafe { + let obj = crate::object::js_object_alloc_class_with_keys( + CID, + PARENT, + 3, + packed.as_ptr(), + packed.len() as u32, + ); + for (i, v) in [10.0f64, 20.0, 30.0].iter().enumerate() { + js_object_set_field(obj, i as u32, JSValue::from_bits(v.to_bits())); + } + let keys_before = (*obj).keys_array; + let parent_before = (*obj).parent_class_id; + assert_eq!((*obj).class_id, CID, "test premise: a class instance"); + assert!( + !is_shape_id(parent_before), + "test premise: the header word is inheritance data, not a shape stamp" + ); + + assert_eq!(js_object_delete_field(obj, key("del6759_x")), 1); + + // The compaction really happened: `z` moved from slot 2 to slot 1. + assert_eq!( + f64::from_bits(js_object_get_field(obj, 1).bits()), + 30.0, + "test premise: the delete did not compact the slots" + ); + // Only the keys pointer records it. + assert_ne!( + (*obj).keys_array, + keys_before, + "the keys pointer is the guard's ONLY compaction evidence and it did not change" + ); + assert_eq!( + (*obj).class_id, + CID, + "class_id changed — a class-id compare would now catch the delete" + ); + assert_eq!( + (*obj).parent_class_id, + parent_before, + "the header word changed — if this is now a minted ShapeId, \ + class_field_inline_guard can switch to a one-word compare and \ + this test should be replaced by that assertion (#6759 C3)" + ); + } + } +} + #[cfg(test)] mod sso_tests_1781 { use super::*; diff --git a/crates/perry-runtime/src/thread.rs b/crates/perry-runtime/src/thread.rs index dbb4bc4a83..ac48468410 100644 --- a/crates/perry-runtime/src/thread.rs +++ b/crates/perry-runtime/src/thread.rs @@ -605,7 +605,32 @@ unsafe fn serialize_object(obj: *const crate::object::ObjectHeader) -> Serialize } let class_id = (*obj).class_id; - let parent_class_id = (*obj).parent_class_id; + // #6759 C3c: `ObjectHeader.parent_class_id` is NOT purely inheritance data. + // For a plain object (`class_id == 0`) the same word carries the runtime + // ShapeId stamp (`shapes::SHAPE_ID_BASE..SHAPE_ID_END`), written lazily by + // every resolve path. Replaying that word verbatim on the destination + // thread — `deserialize` hands it to `js_object_alloc_with_parent`, which + // does `if parent != 0 { register_class(class_id, parent) }` — registers + // `class 0 → ` in the process-global class-parent registry and + // bumps the store-plan epoch, once per deserialized stamped object. + // + // The authoritative parent edge does not live in the header at all: every + // parent-chain walk in the runtime reads `get_parent_class_id(class_id)` + // (`object/class_meta_registry.rs`), and each edge is registered from a + // compile-time constant — by `js_register_class_parent` in the module-init + // prelude for the codegen inline `new C()` path, and by `register_class` + // inside every runtime allocator that takes a `parent_class_id` argument. + // So read it from the registry, which is both correct for class instances + // and immune to the stamp. + // + // This also removes the LAST consumer of the header word as inheritance + // data, which is the blocking dependency for #6759 C3's unification of + // class layouts and plain-object shapes into one shape-id space. + let parent_class_id = if class_id != 0 { + crate::object::get_parent_class_id(class_id).unwrap_or(0) + } else { + 0 + }; let field_count = (*obj).field_count as usize; // Serialize field values @@ -1751,6 +1776,10 @@ pub(crate) fn purge_agent_thread_results(agent: crate::agent::AgentId) { pending.retain(|item| item.owner != agent); } +#[cfg(test)] +#[path = "thread_parent_class_id_tests.rs"] +mod parent_class_id_serialization_tests; + #[cfg(test)] mod transfer_guard_tests { //! #6185 (2026-07-09 GC audit §6): a non-transferable value crossing a diff --git a/crates/perry-runtime/src/thread_parent_class_id_tests.rs b/crates/perry-runtime/src/thread_parent_class_id_tests.rs new file mode 100644 index 0000000000..b233508f48 --- /dev/null +++ b/crates/perry-runtime/src/thread_parent_class_id_tests.rs @@ -0,0 +1,79 @@ +//! #6759 C3c: `ObjectHeader.parent_class_id` carries TWO different things — +//! a real parent class id for a class instance, and the runtime ShapeId +//! stamp for a plain object (`class_id == 0`). `serialize_object` used to +//! copy the raw word, and `deserialize` feeds it to +//! `js_object_alloc_with_parent`, which registers it as a class-parent +//! edge. These tests pin that the serializer reads the class-parent +//! REGISTRY instead, which is (a) correct for both kinds and (b) the last +//! thing that had to stop reading the header word before C3 can re-purpose +//! it as a uniform shape word. +use super::*; +use crate::object::shapes::is_shape_id; + +fn key(name: &str) -> *mut crate::StringHeader { + crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32) +} + +fn serialized_parent(obj: *mut crate::object::ObjectHeader) -> u32 { + let bits = crate::value::js_nanbox_pointer(obj as i64).to_bits(); + match unsafe { serialize_nanbox_for_thread(bits) } { + SerializedValue::Object { + parent_class_id, .. + } => parent_class_id, + other => panic!("expected SerializedValue::Object, got {other:?}"), + } +} + +/// The discriminating quantity. A plain object that has been READ once +/// carries a ShapeId in `parent_class_id`; before this fix that id was +/// serialized and replayed as a class-parent edge, so the receiving thread +/// ran `register_class(0, )`. The test asserts BOTH halves: the +/// stamp is really there (so the fixture is not vacuous) and it does not +/// reach the wire. +#[test] +fn a_plain_objects_shape_stamp_is_not_serialized_as_a_parent_class_id() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let obj = crate::object::js_object_alloc(0, 8); + for name in ["thr6759_a", "thr6759_b", "thr6759_c"] { + crate::object::js_object_set_field_by_name(obj, key(name), 1.0); + } + let _ = crate::object::js_object_get_field_by_name(obj, key("thr6759_b")); + + let stamp = (*obj).parent_class_id; + assert!( + is_shape_id(stamp), + "fixture is vacuous — the object carries no shape stamp to leak (got {stamp:#x})" + ); + assert_eq!( + serialized_parent(obj), + 0, + "a ShapeId ({stamp:#x}) reached the worker as a class-parent edge; \ + deserialization would call register_class(0, {stamp:#x})" + ); + } +} + +/// The other half: the fix must not be "always send 0". A class instance's +/// parent edge still round-trips — and it comes from the REGISTRY, so it +/// survives the header word being overwritten (which is exactly what C3's +/// uniform shape stamp will do). +#[test] +fn a_class_instances_parent_comes_from_the_registry_not_the_header() { + let _lock = crate::gc::global_side_table_test_lock(); + const CHILD: u32 = 0x0C3C_6759; + const PARENT: u32 = 0x0C3C_675A; + unsafe { + let obj = crate::object::js_object_alloc_with_parent(CHILD, PARENT, 2); + assert_eq!(serialized_parent(obj), PARENT, "parent edge lost"); + + // Simulate the C3 end state: the header word now holds a shape + // stamp. The registry is unchanged, so the wire value must be too. + (*obj).parent_class_id = crate::object::shapes::SHAPE_ID_BASE + 7; + assert_eq!( + serialized_parent(obj), + PARENT, + "the serializer is still reading the header word, not the registry" + ); + } +} diff --git a/test-files/test_issue_7981_thread_shape_stamp_parent.ts b/test-files/test_issue_7981_thread_shape_stamp_parent.ts new file mode 100644 index 0000000000..526c698ef5 --- /dev/null +++ b/test-files/test_issue_7981_thread_shape_stamp_parent.ts @@ -0,0 +1,97 @@ +// #7981 / #6759 C3c: `ObjectHeader.parent_class_id` is overloaded. For a class +// instance it is the parent class id; for a plain object (`class_id == 0`) the +// same word carries the runtime ShapeId stamp, written lazily by every by-name +// resolve path. +// +// The `perry/thread` serializer used to copy that word verbatim, and the +// worker-side deserializer feeds it to `js_object_alloc_with_parent`, which +// registers it as a class-parent edge — so a plain object that had been READ +// once and then crossed a `spawn` / `parallelMap` boundary registered +// `class 0 -> ` in the process-global registry. The serializer now +// reads the edge from the class-parent registry instead. +// +// The discriminating assertion (that the registry is not polluted) is a +// perry-runtime unit test — the registry is not observable from JS. THIS test +// is the behavioural regression half: both object kinds must still round-trip, +// and inheritance must still work on the far side after a stamped plain object +// has crossed first. +// +// perry-only (`perry/thread` has no Node equivalent), so this is an +// `test_issue_*` behavioural test, not a byte-for-byte gap test. +import { parallelMap, spawn } from "perry/thread"; + +class Base { + b: number; + constructor(b: number) { + this.b = b; + } + kind(): string { + return "base"; + } +} +class Mid extends Base { + m: number; + constructor(b: number, m: number) { + super(b); + this.m = m; + } + kind(): string { + return "mid"; + } +} +// Fieldless indirect subclass — the shape CLAUDE.md flags as weak. +class Leaf extends Mid {} + +// A plain object literal, READ on the main thread first so the resolve path +// stamps a ShapeId into `parent_class_id`. Without the read the word is 0 and +// the test is vacuous, so read it and print the value we read. +const lit: Record = { alpha: 1, beta: 2, gamma: 3 }; +console.log("stamped read:", lit.beta); + +function summarize(o: Record): string { + const keys = Object.keys(o); + let sum = 0; + for (const k of keys) sum += o[k]; + return keys.join(",") + "=" + sum; +} + +// 1. The stamped plain object crosses first. Pre-fix this is the deserialize +// that polluted the registry with `class 0 -> `. +const mapped = parallelMap([lit, lit, lit, lit], (o: Record) => + summarize(o), +); +console.log("plain mapped:", mapped.join(" | ")); + +// 2. Inheritance must still resolve on a worker AFTER that deserialize — the +// edges the serializer now reads come from the same registry the pollution +// landed in. +function chain(n: number): string { + const leaf = new Leaf(n, n * 10); + const parts: string[] = [ + leaf.kind(), + String(leaf.b), + String(leaf.m), + String(leaf instanceof Mid), + String(leaf instanceof Base), + String(new Mid(n, n) instanceof Base), + ]; + return parts.join(","); +} +const expected = chain(4); +console.log("main chain:", expected); + +const chained = parallelMap([4, 4, 4, 4], (n: number): string => chain(n)); +let allMatch = true; +for (let i = 0; i < chained.length; i++) { + if (chained[i] !== expected) allMatch = false; +} +console.log("worker chain count:", chained.length, "allMatch:", allMatch); + +// 3. A class INSTANCE crossing the boundary keeps its fields. +const inst = new Mid(7, 70); +const back = await spawn((): string => chain(7)); +console.log("spawn chain:", back, "match:", back === chain(7)); +console.log("instance fields:", inst.b, inst.m, inst.kind()); + +// 4. The main thread is still correct after all of it. +console.log("main again:", chain(4) === expected, summarize(lit));