https://git.ruby-lang.org/ruby.git/commit/?id=39a2ba5cc5

From 39a2ba5cc559900c30c3143da32446c2f20a7484 Mon Sep 17 00:00:00 2001
From: Alan Wu <XrXr@u...>
Date: Tue, 11 May 2021 12:05:06 -0400
Subject: 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(-)

diff --git a/class.c b/class.c
index 6784540..9ac2b3f 100644
--- a/class.c
+++ b/class.c
@@ -1181,10 +1181,12 @@ cache_clear_refined_method(ID key, VALUE value, void *data) https://github.com/ruby/ruby/blob/trunk/class.c#L1181
 {
     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 c364de4..b386c3a 100644
--- a/test/ruby/test_refinement.rb
+++ b/test/ruby/test_refinement.rb
@@ -2488,6 +2488,55 @@ class TestRefinement < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_refinement.rb#L2488
     }
   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/vm_method.c b/vm_method.c
index be0c1ef..34200dc 100644
--- a/vm_method.c
+++ b/vm_method.c
@@ -8,6 +8,7 @@ https://github.com/ruby/ruby/blob/trunk/vm_method.c#L8
 
 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
@@ -187,9 +188,17 @@ clear_method_cache_by_id_in_class(VALUE klass, ID mid) https://github.com/ruby/ruby/blob/trunk/vm_method.c#L188
                     // 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_cme_invalidate((rb_callable_method_entry_t *)cme);
-- 
cgit v1.1


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/