diff --git a/changelog.d/8651-array-growth-generation.md b/changelog.d/8651-array-growth-generation.md new file mode 100644 index 0000000000..624c36be02 --- /dev/null +++ b/changelog.d/8651-array-growth-generation.md @@ -0,0 +1,12 @@ +Fixed retained array-growth forwarding stubs pointing into resetting nursery +space. When an old array outgrew its backing storage, `js_array_grow` could +allocate the replacement in the copying nursery and leave the permanent old +stub pointing at it. Minor GC does not trace that forwarding payload as a +normal array slot, so a later nursery reset could recycle the target while +stale aliases still followed the old forwarding word. + +Array growth now keeps the replacement out of the nursery whenever the source +is old or otherwise non-moving. A young source uses nursery space only through +the no-collection allocator; if growth could collect and promote the source, +the replacement is born old instead. This fixes the intermittent ECS failure +reported as `Cannot assign to read only property 'length' of object`. diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index 183714dde3..a86a4c6748 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -1,6 +1,5 @@ //! push / pop / shift / unshift / set_length / delete + grow primitive. use super::*; -use crate::arena::arena_alloc_gc; use std::ptr; /// `pop`/`shift`/`push`/`unshift` on a frozen array perform a `Set`/`Delete` @@ -133,8 +132,38 @@ pub extern "C" fn js_array_grow(arr: *mut ArrayHeader, min_capacity: u32) -> *mu let old_size = array_byte_size(old_capacity as usize); let new_size = array_byte_size(new_capacity as usize); - // Allocate new from arena and copy old data. - let new_ptr = arena_alloc_gc(new_size, 8, crate::gc::GC_TYPE_ARRAY) as *mut ArrayHeader; + // A growth stub outlives the array operation: aliases can keep its + // address and `clean_arr_ptr` follows it on a later access. Therefore + // a non-moving source must not forward into the copying nursery. A + // minor does not trace a retained `GC_FLAG_FORWARDED` stub as a normal + // array object, so its payload forwarding word is outside ordinary + // layout and remembered-set scanning. It would neither move nor retain + // a young target; resetting from-space would leave the permanent old + // stub pointing at recycled bytes. + // + // For a young source, use the nursery only when the already-open block + // can satisfy the grow without collecting. If that allocation would + // collect, the source may be promoted while its handle is reloaded; + // birth the target old instead so the post-collection source cannot + // acquire the same old->young forwarding edge. + let old_header = + (arr as *mut u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader; + let source_requires_old_target = (*old_header).gc_flags & crate::gc::GC_FLAG_TENURED != 0 + || !matches!( + crate::arena::classify_heap_generation(arr as usize), + crate::arena::HeapGeneration::Nursery + ); + let new_ptr = if source_requires_old_target { + crate::arena::arena_alloc_gc_old_born_tenured(new_size, 8, crate::gc::GC_TYPE_ARRAY) + } else { + let young = + crate::arena::arena_alloc_gc_no_collect(new_size, 8, crate::gc::GC_TYPE_ARRAY); + if young.is_null() { + crate::arena::arena_alloc_gc_old_born_tenured(new_size, 8, crate::gc::GC_TYPE_ARRAY) + } else { + young + } + } as *mut ArrayHeader; let arr = arr_handle.get_raw_mut_ptr::(); // GC_STORE_AUDIT(BARRIERED): array growth copy transfers layout and replays write barriers below. ptr::copy_nonoverlapping(arr as *const u8, new_ptr as *mut u8, old_size); diff --git a/crates/perry-runtime/src/array/tests.rs b/crates/perry-runtime/src/array/tests.rs index f77a7ce8ed..dffd1d87c0 100644 --- a/crates/perry-runtime/src/array/tests.rs +++ b/crates/perry-runtime/src/array/tests.rs @@ -590,6 +590,43 @@ fn stale_array_reference_survives_three_growths_and_forced_minor_gc() { } } +#[test] +fn growth_of_old_array_keeps_forwarding_target_out_of_copying_nursery() { + let _triggers = crate::gc::GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let capacity = MIN_ARRAY_CAPACITY; + let initial = crate::arena::arena_alloc_gc_old_born_tenured( + array_byte_size(capacity as usize), + 8, + crate::gc::GC_TYPE_ARRAY, + ) as *mut ArrayHeader; + + unsafe { + (*initial).length = 0; + (*initial).capacity = capacity; + let elements = (initial as *mut u8).add(std::mem::size_of::()) as *mut u64; + for i in 0..capacity as usize { + // GC_STORE_AUDIT(INIT): initialize unpublished fresh array storage + // with the non-pointer hole sentinel before exposing the array. + ptr::write(elements.add(i), crate::value::TAG_HOLE); + } + set_array_numeric_layout(initial, NumericArrayLayout::RawF64); + crate::gc::layout_init_pointer_free(initial as *mut u8); + } + + let mut head = initial; + for i in 0..=capacity { + head = js_array_push_f64(head, i as f64); + } + + assert_ne!(head, initial, "the capacity-crossing push must grow"); + assert_eq!(clean_arr_ptr_mut(initial), head); + assert_eq!( + crate::arena::classify_heap_generation(head as usize), + crate::arena::HeapGeneration::Old, + "an old forwarding stub must not point into resetting copying-nursery space" + ); +} + #[test] fn install_array_growth_forwarding_with_installs_stub_for_injected_header() { // Actual low-address classification is covered by