From 2c7ec3d155ba9d3e0589f716c1522f2c26371586 Mon Sep 17 00:00:00 2001
From: Peter Zhu <peter@peterzhu.ca>
Date: Wed, 20 Aug 2025 10:53:18 -0400
Subject: [PATCH] Fix race condition in method invalidation for Ractors

We lock the VM to invalidate method entries. However, we do not lock the
VM to call methods, so it's possible that during a method call the method
entry gets invalidated. We only check that the method entry in the callcache
is not invalidated at the beginning of the method call, which makes it
possible to have race conditions. This causes crashes like:

    vm_callinfo.h:421: Assertion Failed: vm_cc_cme:cc->klass != Qundef || !vm_cc_markable(cc)
    vm_insnhelper.c:2200: Assertion Failed: vm_lookup_cc:!METHOD_ENTRY_INVALIDATED(vm_cc_cme(ccs_cc))

This commit adds a VM barrier to method cache invalidation to ensure that
other Ractors are stopped at a safe-point before invalidating the method
entry.
---
 bootstraptest/test_ractor.rb | 27 +++++++++++++++++++++++++++
 vm_callinfo.h                |  2 +-
 vm_method.c                  |  4 ++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb
index b1e9e3a79d..4a58ece8ac 100644
--- a/bootstraptest/test_ractor.rb
+++ b/bootstraptest/test_ractor.rb
@@ -1573,6 +1573,33 @@ assert_equal 'true', %q{
   rs.map{|r| r.value} == Array.new(RN){n}
 }
 
+# check method cache invalidation
+assert_equal 'true', %q{
+  class Foo
+    def hello = nil
+  end
+
+  r1 = Ractor.new do
+    1000.times do
+      class Foo
+        def hello = nil
+      end
+    end
+  end
+
+  r2 = Ractor.new do
+    1000.times do
+      o = Foo.new
+      o.hello
+    end
+  end
+
+  r1.value
+  r2.value
+
+  true
+}
+
 # check experimental warning
 assert_match /\Atest_ractor\.rb:1:\s+warning:\s+Ractor is experimental/, %q{
   Warning[:experimental] = $VERBOSE = true
diff --git a/vm_callinfo.h b/vm_callinfo.h
index 79ccbfa7ab..e52b2f9b1a 100644
--- a/vm_callinfo.h
+++ b/vm_callinfo.h
@@ -613,7 +613,7 @@ static inline bool
 vm_cc_check_cme(const struct rb_callcache *cc, const rb_callable_method_entry_t *cme)
 {
     bool valid;
-    RB_VM_LOCKING() {
+    RB_VM_LOCKING_NO_BARRIER() {
         valid = vm_cc_cme(cc) == cme ||
             (cme->def->iseq_overload && vm_cc_cme(cc) == rb_vm_lookup_overloaded_cme(cme));
     }
diff --git a/vm_method.c b/vm_method.c
index 722daf0a6a..73a431ce93 100644
--- a/vm_method.c
+++ b/vm_method.c
@@ -428,6 +428,8 @@ clear_method_cache_by_id_in_class(VALUE klass, ID mid)
     if (rb_objspace_garbage_object_p(klass)) return;
 
     RB_VM_LOCKING() {
+        rb_vm_barrier();
+
         if (LIKELY(RCLASS_SUBCLASSES_FIRST(klass) == NULL)) {
             // no subclasses
             // check only current class
@@ -1752,6 +1754,8 @@ cached_callable_method_entry(VALUE klass, ID mid)
             return ccs->cme;
         }
         else {
+            rb_vm_barrier();
+
             rb_managed_id_table_delete(cc_tbl, mid);
             rb_vm_ccs_invalidate_and_free(ccs);
         }