summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Zhu <peter@peterzhu.ca>2025-08-20 10:53:18 -0400
committerPeter Zhu <peter@peterzhu.ca>2025-08-20 15:53:00 -0400
commit2c7ec3d155ba9d3e0589f716c1522f2c26371586 (patch)
tree51e68ce0c0ac261efa55d8a01eae3ded99478e7c
parent5c96bbf36a73e29561b2b1a0e42c6f3341a4e542 (diff)
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.
-rw-r--r--bootstraptest/test_ractor.rb27
-rw-r--r--vm_callinfo.h2
-rw-r--r--vm_method.c4
3 files changed, 32 insertions, 1 deletions
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);
}