Skip to content

Commit 469d61c

Browse files
committed
Method cache: fix refinement entry handling
To invalidate some callable method entries, we replace the entry in the class. Most types of method entries are on the method table of the origin class, but refinement entries without an orig_me are housed in the method table of the class itself. They are there because refinements take priority over prepended methods. By unconditionally inserting a copy of the refinement entry into the origin class, clearing the method cache created situations where there are refinement entry duplicates in the lookup chain, leading to infinite loops and other problems. Update the replacement logic to use the right class that houses the method entry. Also be more selective about cache invalidation when moving refinement entries for prepend. This avoids calling clear_method_cache_by_id_in_class() before refinement entries are in the place it expects. [Bug #17806]
1 parent b167d8d commit 469d61c

File tree

3 files changed

+63
-3
lines changed

3 files changed

+63
-3
lines changed

class.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1134,10 +1134,12 @@ cache_clear_refined_method(ID key, VALUE value, void *data)
11341134
{
11351135
rb_method_entry_t *me = (rb_method_entry_t *) value;
11361136

1137-
if (me->def->type == VM_METHOD_TYPE_REFINED) {
1137+
if (me->def->type == VM_METHOD_TYPE_REFINED && me->def->body.refined.orig_me) {
11381138
VALUE klass = (VALUE)data;
11391139
rb_clear_method_cache(klass, me->called_id);
11401140
}
1141+
// Refined method entries without an orig_me is going to stay in the method
1142+
// table of klass, like before the move, so no need to clear the cache.
11411143

11421144
return ID_TABLE_CONTINUE;
11431145
}

test/ruby/test_refinement.rb

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2488,6 +2488,55 @@ def except *args; end
24882488
}
24892489
end
24902490

2491+
def test_defining_after_cached
2492+
klass = Class.new
2493+
refinement = Module.new { refine(klass) { def foo; end } }
2494+
klass.new.foo rescue nil # cache the refinement method entry
2495+
klass.define_method(:foo) { 42 }
2496+
assert_equal(42, klass.new.foo)
2497+
end
2498+
2499+
# [Bug #17806]
2500+
def test_two_refinements_for_prepended_class
2501+
assert_normal_exit %q{
2502+
module R1
2503+
refine Hash do
2504+
def foo; :r1; end
2505+
end
2506+
end
2507+
2508+
class Hash
2509+
prepend(Module.new)
2510+
end
2511+
2512+
class Hash
2513+
def foo; end
2514+
end
2515+
2516+
{}.method(:foo) # put it on pCMC
2517+
2518+
module R2
2519+
refine Hash do
2520+
def foo; :r2; end
2521+
end
2522+
end
2523+
2524+
{}.foo
2525+
}
2526+
end
2527+
2528+
# [Bug #17806]
2529+
def test_redefining_refined_for_prepended_class
2530+
klass = Class.new { def foo; end }
2531+
_refinement = Module.new do
2532+
refine(klass) { def foo; :refined; end }
2533+
end
2534+
klass.prepend(Module.new)
2535+
klass.new.foo # cache foo
2536+
klass.define_method(:foo) { :second }
2537+
assert_equal(:second, klass.new.foo)
2538+
end
2539+
24912540
private
24922541

24932542
def eval_using(mod, s)

vm_method.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
static int vm_redefinition_check_flag(VALUE klass);
1010
static void rb_vm_check_redefinition_opt_method(const rb_method_entry_t *me, VALUE klass);
11+
static inline rb_method_entry_t *lookup_method_table(VALUE klass, ID id);
1112

1213
#define object_id idObject_id
1314
#define added idMethod_added
@@ -173,9 +174,17 @@ clear_method_cache_by_id_in_class(VALUE klass, ID mid)
173174
// invalidate cc by invalidating cc->cme
174175
VALUE owner = cme->owner;
175176
VM_ASSERT(BUILTIN_TYPE(owner) == T_CLASS);
177+
VALUE klass_housing_cme;
178+
if (cme->def->type == VM_METHOD_TYPE_REFINED && !cme->def->body.refined.orig_me) {
179+
klass_housing_cme = owner;
180+
}
181+
else {
182+
klass_housing_cme = RCLASS_ORIGIN(owner);
183+
}
184+
// replace the cme that will be invalid
185+
VM_ASSERT(lookup_method_table(klass_housing_cme, mid) == (const rb_method_entry_t *)cme);
176186
const rb_method_entry_t *new_cme = rb_method_entry_clone((const rb_method_entry_t *)cme);
177-
VALUE origin = RCLASS_ORIGIN(owner);
178-
rb_method_table_insert(origin, RCLASS_M_TBL(origin), mid, new_cme);
187+
rb_method_table_insert(klass_housing_cme, RCLASS_M_TBL(klass_housing_cme), mid, new_cme);
179188
}
180189

181190
vm_me_invalidate_cache((rb_callable_method_entry_t *)cme);

0 commit comments

Comments
 (0)