summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornagachika <nagachika@ruby-lang.org>2021-05-29 14:35:12 +0900
committernagachika <nagachika@ruby-lang.org>2021-05-29 14:35:12 +0900
commitd47df50678b00bd622e6be474031204ed2e52b31 (patch)
tree5f69112b413dca78a9aa9165a79d688e912e55f0
parent7b6a2ad04a3272a31323493133498dfc60d77d76 (diff)
merge revision(s) 39a2ba5cc559900c30c3143da32446c2f20a7484: [Backport #17806]
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] --- class.c | 4 +++- test/ruby/test_refinement.rb | 49 ++++++++++++++++++++++++++++++++++++++++++++ vm_method.c | 13 ++++++++++-- 3 files changed, 63 insertions(+), 3 deletions(-)
-rw-r--r--class.c4
-rw-r--r--test/ruby/test_refinement.rb49
-rw-r--r--version.h2
-rw-r--r--vm_method.c13
4 files changed, 64 insertions, 4 deletions
diff --git a/class.c b/class.c
index 47f35b1f90..b607135e2f 100644
--- a/class.c
+++ b/class.c
@@ -1134,10 +1134,12 @@ cache_clear_refined_method(ID key, VALUE value, void *data)
{
rb_method_entry_t *me = (rb_method_entry_t *) value;
- if (me->def->type == VM_METHOD_TYPE_REFINED) {
+ if (me->def->type == VM_METHOD_TYPE_REFINED && me->def->body.refined.orig_me) {
VALUE klass = (VALUE)data;
rb_clear_method_cache(klass, me->called_id);
}
+ // Refined method entries without an orig_me is going to stay in the method
+ // table of klass, like before the move, so no need to clear the cache.
return ID_TABLE_CONTINUE;
}
diff --git a/test/ruby/test_refinement.rb b/test/ruby/test_refinement.rb
index c364de46e4..b386c3a8d7 100644
--- a/test/ruby/test_refinement.rb
+++ b/test/ruby/test_refinement.rb
@@ -2488,6 +2488,55 @@ class TestRefinement < Test::Unit::TestCase
}
end
+ def test_defining_after_cached
+ klass = Class.new
+ refinement = Module.new { refine(klass) { def foo; end } }
+ klass.new.foo rescue nil # cache the refinement method entry
+ klass.define_method(:foo) { 42 }
+ assert_equal(42, klass.new.foo)
+ end
+
+ # [Bug #17806]
+ def test_two_refinements_for_prepended_class
+ assert_normal_exit %q{
+ module R1
+ refine Hash do
+ def foo; :r1; end
+ end
+ end
+
+ class Hash
+ prepend(Module.new)
+ end
+
+ class Hash
+ def foo; end
+ end
+
+ {}.method(:foo) # put it on pCMC
+
+ module R2
+ refine Hash do
+ def foo; :r2; end
+ end
+ end
+
+ {}.foo
+ }
+ end
+
+ # [Bug #17806]
+ def test_redefining_refined_for_prepended_class
+ klass = Class.new { def foo; end }
+ _refinement = Module.new do
+ refine(klass) { def foo; :refined; end }
+ end
+ klass.prepend(Module.new)
+ klass.new.foo # cache foo
+ klass.define_method(:foo) { :second }
+ assert_equal(:second, klass.new.foo)
+ end
+
private
def eval_using(mod, s)
diff --git a/version.h b/version.h
index 70e00e7b80..a9ea2b3e39 100644
--- a/version.h
+++ b/version.h
@@ -12,7 +12,7 @@
# define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR
#define RUBY_VERSION_TEENY 2
#define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR
-#define RUBY_PATCHLEVEL 91
+#define RUBY_PATCHLEVEL 92
#define RUBY_RELEASE_YEAR 2021
#define RUBY_RELEASE_MONTH 5
diff --git a/vm_method.c b/vm_method.c
index 453cbed499..a467ac173c 100644
--- a/vm_method.c
+++ b/vm_method.c
@@ -8,6 +8,7 @@
static int vm_redefinition_check_flag(VALUE klass);
static void rb_vm_check_redefinition_opt_method(const rb_method_entry_t *me, VALUE klass);
+static inline rb_method_entry_t *lookup_method_table(VALUE klass, ID id);
#define object_id idObject_id
#define added idMethod_added
@@ -173,9 +174,17 @@ clear_method_cache_by_id_in_class(VALUE klass, ID mid)
// invalidate cc by invalidating cc->cme
VALUE owner = cme->owner;
VM_ASSERT(BUILTIN_TYPE(owner) == T_CLASS);
+ VALUE klass_housing_cme;
+ if (cme->def->type == VM_METHOD_TYPE_REFINED && !cme->def->body.refined.orig_me) {
+ klass_housing_cme = owner;
+ }
+ else {
+ klass_housing_cme = RCLASS_ORIGIN(owner);
+ }
+ // replace the cme that will be invalid
+ VM_ASSERT(lookup_method_table(klass_housing_cme, mid) == (const rb_method_entry_t *)cme);
const rb_method_entry_t *new_cme = rb_method_entry_clone((const rb_method_entry_t *)cme);
- VALUE origin = RCLASS_ORIGIN(owner);
- rb_method_table_insert(origin, RCLASS_M_TBL(origin), mid, new_cme);
+ rb_method_table_insert(klass_housing_cme, RCLASS_M_TBL(klass_housing_cme), mid, new_cme);
}
vm_me_invalidate_cache((rb_callable_method_entry_t *)cme);