From cc1c89409fe2d23d91cfaab286027532b211b494 Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Mon, 26 Oct 2026 22:48:52 +0000 Subject: [PATCH 1/2] Set the pin bit on pinned roots during moving GCs Roots reported through rb_gc_impl_mark_and_pin and rb_gc_impl_mark_maybe were processed as pinning roots. That keeps them in place for the current GC, but it does not set the pin bit, so rb_gc_impl_pinned_p returned false for them. When it updates references, Ruby re-embeds strings and arrays whose pinned_p is false and frees their heap buffer, even when a C frame still holds RSTRING_PTR. Since ruby/ruby 0b7838e23b, File.read returns a heap string that fits its slot, so String#scan in template/exts.mk.tmpl raised "string modified" under Immix and StickyImmix. In this commit we set the pin bit on roots reported with pin during moving GCs, and record them in the PinningRegistry so that the existing UnpinPinnedObjects work unpins them in VMRefClosure. Roots marked with rb_gc_mark_movable stay movable, so they can still be re-embedded. --- gc/mmtk/src/pinning_registry.rs | 14 ++++++++++++++ gc/mmtk/src/scanning.rs | 9 +++++++++ test/mmtk/test_pinning.rb | 31 +++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 test/mmtk/test_pinning.rb diff --git a/gc/mmtk/src/pinning_registry.rs b/gc/mmtk/src/pinning_registry.rs index 7b295d4..6723a6e 100644 --- a/gc/mmtk/src/pinning_registry.rs +++ b/gc/mmtk/src/pinning_registry.rs @@ -57,6 +57,20 @@ impl PinningRegistry { worker.scheduler().work_buckets[WorkBucketStage::Prepare].bulk_add(work_packets); } + pub fn pin_roots(&self, roots: &[ObjectReference]) { + let mut newly_pinned_objs: Vec = roots + .iter() + .copied() + .filter(|obj| memory_manager::pin_object(*obj)) + .collect(); + if newly_pinned_objs.is_empty() { + return; + } + + let mut pinned_objs = self.pinned_objs.lock().unwrap(); + pinned_objs.append(&mut newly_pinned_objs); + } + pub fn cleanup(&self, worker: &mut GCWorker) { worker.scheduler().work_buckets[WorkBucketStage::VMRefClosure].add(RemoveDeadPinnings); if crate::mmtk().get_plan().current_gc_may_move_object() { diff --git a/gc/mmtk/src/scanning.rs b/gc/mmtk/src/scanning.rs index 7b59483..96439e3 100644 --- a/gc/mmtk/src/scanning.rs +++ b/gc/mmtk/src/scanning.rs @@ -178,7 +178,9 @@ impl VMScanning { factory: &mut impl RootsWorkFactory, callback: F, ) { + let current_gc_may_move = crate::mmtk().get_plan().current_gc_may_move_object(); let mut buffer: Vec = Vec::new(); + let mut pinned_roots: Vec = Vec::new(); let visit_object = |_, object: ObjectReference, pin| { debug!( "[{}] Visiting object: {}{}", @@ -194,6 +196,9 @@ impl VMScanning { mmtk::memory_manager::is_mmtk_object(object.to_raw_address()).is_some(), "Root does not point to MMTk object. object: {object}" ); + if pin && current_gc_may_move { + pinned_roots.push(object); + } buffer.push(object); if buffer.len() >= Self::OBJECT_BUFFER_SIZE { factory.create_process_pinning_roots_work(std::mem::take(&mut buffer)); @@ -204,6 +209,10 @@ impl VMScanning { .object_closure .set_temporarily_and_run_code(visit_object, callback); + // Pinning roots are never moved, but only the pin bit makes rb_gc_impl_pinned_p + // true, and Ruby re-embeds strings and arrays whose pinned_p is false. + crate::binding().pinning_registry.pin_roots(&pinned_roots); + if !buffer.is_empty() { factory.create_process_pinning_roots_work(buffer); } diff --git a/test/mmtk/test_pinning.rb b/test/mmtk/test_pinning.rb new file mode 100644 index 0000000..39042fa --- /dev/null +++ b/test/mmtk/test_pinning.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require_relative "helper" + +module MMTk + class TestPinning < TestCase + %w(Immix StickyImmix).each do |plan| + define_method(:"test_string_buffer_held_by_c_frame_survives_gc_#{plan}") do + assert_separately([{ "MMTK_PLAN" => plan }], <<~'RUBY') + require "tempfile" + + Tempfile.create do |f| + f.write("a\n" * 5000) + f.flush + + s = File.read(f.path) + moving_gc_count = GC.stat(:moving_gc_count) + count = 0 + s.scan(/a/) do + count += 1 + GC.start if count % 250 == 0 + end + + assert_equal(5000, count) + assert_operator(GC.stat(:moving_gc_count), :>, moving_gc_count) + end + RUBY + end + end + end +end From 03ed0ac33ad4cf9f58cc7ec0047a907ffdafecd0 Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Tue, 27 Oct 2026 00:16:18 +0000 Subject: [PATCH 2/2] Vendor the new Ruby headers --- gc/gc.h | 3 --- gc/gc_impl.h | 8 ++++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/gc/gc.h b/gc/gc.h index 64f29a0..6864594 100644 --- a/gc/gc.h +++ b/gc/gc.h @@ -79,10 +79,7 @@ MODULAR_GC_FN void rb_gc_vm_each_objspace(void (*func)(void *objspace, void *dat MODULAR_GC_FN void rb_gc_each_registered_addr(rb_gc_registered_addr_cb func, void *data); /* Verifier support, valid only with the world stopped (no extra locking inside). */ MODULAR_GC_FN bool rb_gc_registered_addr_owned_by_registrant_p(VALUE *addr, void *objspace); -MODULAR_GC_FN bool rb_gc_vm_zombie_objspace_p(void *objspace); -MODULAR_GC_FN size_t rb_gc_vm_zombie_total_pages(void); MODULAR_GC_FN unsigned int rb_gc_vm_ractor_count(void); -MODULAR_GC_FN void rb_gc_vm_refresh_zombie_pages(void); /* No MODULAR_GC_FN: the VM side (ractor.c) calls this too, so it needs external * linkage even in a non-modular build (see internal/gc.h). */ bool rb_gc_single_objspace_p(void); diff --git a/gc/gc_impl.h b/gc/gc_impl.h index 1bcb744..95ac884 100644 --- a/gc/gc_impl.h +++ b/gc/gc_impl.h @@ -153,7 +153,11 @@ GC_IMPL_FN bool rb_gc_impl_during_postmortem_p(void *objspace_ptr); * objspace impl. */ GC_IMPL_FN bool rb_gc_impl_obj_foreign_p(void *objspace_ptr, VALUE obj); GC_IMPL_FN bool rb_gc_impl_shref_marked_p(void *objspace_ptr, VALUE obj); -GC_IMPL_FN size_t rb_gc_impl_zombie_heap_page_count(void *objspace_ptr); +/* gc.c parked objspace_ptr in its zombie table: a terminated Ractor's objspace that + * rb_gc_impl_objspace_absorb later merges. Called under the VM lock, possibly from + * inside a global GC sweep (an unjoined Ractor's dfree disowns its objspace), so it + * must not allocate through the GC or expect to take a barrier. */ +GC_IMPL_FN void rb_gc_impl_objspace_zombify(void *objspace_ptr); GC_IMPL_FN void rb_gc_impl_objspace_absorb(void *dst_ptr, void *src_ptr); GC_IMPL_FN void rb_gc_impl_each_object(void *objspace_ptr, void (*func)(VALUE obj, void *data), void *data); // Finalizers @@ -172,7 +176,7 @@ GC_IMPL_FN unsigned long long rb_gc_impl_get_total_time(void *objspace_ptr); GC_IMPL_FN size_t rb_gc_impl_gc_count(void *objspace_ptr); GC_IMPL_FN VALUE rb_gc_impl_latest_gc_info(void *objspace_ptr, VALUE key); /* A NULL objspace requests cumulative process-wide statistics. Collectors that - * do not support this scope must raise NotImplementedError. */ + * do not support them must raise NotImplementedError. */ GC_IMPL_FN VALUE rb_gc_impl_stat(void *objspace_ptr, VALUE hash_or_sym); GC_IMPL_FN VALUE rb_gc_impl_stat_heap(void *objspace_ptr, VALUE heap_name, VALUE hash_or_sym); GC_IMPL_FN const char *rb_gc_impl_active_gc_name(void);