diff options
| author | Peter Zhu <peter@peterzhu.ca> | 2025-08-20 10:53:18 -0400 |
|---|---|---|
| committer | Peter Zhu <peter@peterzhu.ca> | 2025-08-20 15:53:00 -0400 |
| commit | 2c7ec3d155ba9d3e0589f716c1522f2c26371586 (patch) | |
| tree | 51e68ce0c0ac261efa55d8a01eae3ded99478e7c | |
| parent | 5c96bbf36a73e29561b2b1a0e42c6f3341a4e542 (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.rb | 27 | ||||
| -rw-r--r-- | vm_callinfo.h | 2 | ||||
| -rw-r--r-- | vm_method.c | 4 |
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); } |
