Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions changelog.d/7981-thread-parent-class-id-from-registry.md
Original file line number Diff line number Diff line change
@@ -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 → <a shape id>` 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.
129 changes: 129 additions & 0 deletions crates/perry-runtime/src/object/delete_rest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down
31 changes: 30 additions & 1 deletion crates/perry-runtime/src/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 → <a shape id>` 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
Expand Down Expand Up @@ -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
Expand Down
79 changes: 79 additions & 0 deletions crates/perry-runtime/src/thread_parent_class_id_tests.rs
Original file line number Diff line number Diff line change
@@ -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, <shape id>)`. 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;
Comment on lines +37 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Root each test object across key(...) allocations.

key(...) allocates a StringHeader while each site retains obj only as a raw Rust pointer. A collection can relocate the object before the subsequent unsafe use.

  • crates/perry-runtime/src/thread_parent_class_id_tests.rs#L37-L43: root obj before creating keys and reload it after each key allocation.
  • crates/perry-runtime/src/object/delete_rest.rs#L638-L658: root and reload the plain-object pointer around each key allocation.
  • crates/perry-runtime/src/object/delete_rest.rs#L685-L703: root and reload the class-instance pointer around the deletion key allocation.

Based on learnings: “raw Rust pointer locals are neither GC roots nor reliable pins” across operations that can allocate or invoke GC.

📍 Affects 2 files
  • crates/perry-runtime/src/thread_parent_class_id_tests.rs#L37-L43 (this comment)
  • crates/perry-runtime/src/object/delete_rest.rs#L638-L658
  • crates/perry-runtime/src/object/delete_rest.rs#L685-L703
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/thread_parent_class_id_tests.rs` around lines 37 -
43, Root each raw object pointer before any key(...) allocation or other
GC-triggering operation, then reload the pointer from the root after each
allocation before using it. Apply this to the test object in
crates/perry-runtime/src/thread_parent_class_id_tests.rs:37-43, the plain-object
pointer in crates/perry-runtime/src/object/delete_rest.rs:638-658, and the
class-instance pointer in
crates/perry-runtime/src/object/delete_rest.rs:685-703; preserve the existing
field access and deletion behavior.

Source: Learnings

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"
);
}
}
Loading
Loading