Skip to content

Commit 7a058b5

Browse files
committed
Pin refinement CCs to fix SEGV
We carry 9e5ef24 ("Reapply \"Fix check in cc_is_active\"") and b6e3272 ("Reapply \"Simplify iseq mark_and_move CC handling\""), which make refinement CCs actually live and movable, but not upstream master's 8eab317 ("Convert cc_refinement_table into TypedData object"), which replaced the address-keyed cc_refinement_table with an array of declarative weak references. With the address-keyed table, compaction relocating a refinement CC rewrites the set entry's key but leaves it filed under the hash of the old address, so rb_vm_delete_cc_refinement() can no longer find it when the CC is freed and the table is left pointing at a recycled slot. The next using/refine walks that entry: in debug builds it fails "vm_method.c:664: invalidate_cc_refinement:vm_cc_refinement_p(cc)" (seen in test-all as TestRefinement#test_refine_mutual_recursion), otherwise it silently writes Qundef into an unrelated live object. Pin them like we already do for method entries that are keys of the overloaded_cme table, and add a regression test that reproduces the assertion 100% of the time without this change.
1 parent 2f8b396 commit 7a058b5

2 files changed

Lines changed: 48 additions & 0 deletions

File tree

‎imemo.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,12 @@ rb_imemo_mark_and_move(VALUE obj, bool reference_updating)
379379
if ((vm_cc_super_p(cc) || vm_cc_refinement_p(cc))) {
380380
rb_gc_mark_movable((VALUE)cc->cme_);
381381
}
382+
383+
if (vm_cc_refinement_p(cc)) {
384+
// it is a key of the "cc_refinement" table, which looks up
385+
// entries by address, so it should be pinned.
386+
rb_gc_mark(obj);
387+
}
382388
}
383389

384390
break;

‎test/ruby/test_gc_compact.rb‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,48 @@ def walk_ast ast
161161
end;
162162
end
163163

164+
def test_refinement_callcaches_are_not_moved
165+
assert_separately([], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 60)
166+
begin;
167+
module RefinementForCompaction
168+
refine String do
169+
def refined_method; :refined; end
170+
end
171+
end
172+
using RefinementForCompaction
173+
174+
objects = []
175+
10.times do
176+
procs = 200.times.map { eval("proc { 'str'.refined_method }") }
177+
procs.each(&:call)
178+
179+
# Refinement call caches are registered in a VM wide table which looks
180+
# up entries by address, so they must not be moved.
181+
GC.verify_compaction_references(expand_heap: false, toward: :empty)
182+
183+
procs = nil
184+
GC.start
185+
186+
# Reuse the slots of the collected refinement call caches.
187+
500.times do
188+
klass = Class.new { def foo; end }
189+
obj = klass.new
190+
3.times { obj.foo }
191+
objects << obj
192+
end
193+
end
194+
195+
# Invalidates every registered refinement call cache.
196+
module RefinementForCompaction
197+
refine String do
198+
def other_refined_method; :refined; end
199+
end
200+
end
201+
202+
assert_equal(:refined, "str".refined_method)
203+
end;
204+
end
205+
164206
def test_compact_count
165207
count = GC.stat(:compact_count)
166208
GC.compact

0 commit comments

Comments
 (0)