From 2e5bf429e44e7605dae82edfc387979bf049c71f Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Thu, 16 Jul 2026 07:59:01 +0900 Subject: [PATCH] Ractor: restrict class/module modification to the creator Ractor Only the Ractor which created a class/module may modify it: define, remove or undefine methods, alias, include/prepend, refine, define or remove constants, register an autoload, and set the class's own instance variables. Reading is unchanged. This replaces the assorted "main Ractor only" rules with one creator-based rule, and in exchange a Ractor gets full use of the classes it creates itself -- unshareable constant and instance variable values included, which even the main Ractor could not do for its own classes before. Three paths reach a class's tables without going through rb_class_modify_check, so they get the check of their own: Module#refine (which writes its refinement tables into the receiver as well as into the refined class), Class#initialize (which writes the superclass of a not-yet-initialized class, and had no frozen check either), and Module#ruby2_keywords. The constant inline cache has to move with the rule. Its hit condition was "shareable value, or we are the main Ractor", which was sound only because a non-main Ractor could not have an unshareable constant in the first place. Now it can, so an owner priming the cache would let any other Ractor read that value straight out of it, never reaching the owner check in vm_get_ev_const. The entry records the Ractor which filled it instead -- the one that passed the check -- which also gives a non-main owner a fast path it never had. Co-Authored-By: Claude Fable 5 Co-Authored-By: Claude Opus 5 (1M context) --- bootstraptest/test_ractor.rb | 135 +++++++++++++++++++++++++++------ eval.c | 6 ++ object.c | 2 + test/ruby/test_iseq.rb | 26 +++++-- variable.c | 41 +++++----- vm_core.h | 7 +- vm_insnhelper.c | 23 +++--- vm_method.c | 1 + yjit/src/cruby_bindings.inc.rs | 1 + zjit/src/cruby_bindings.inc.rs | 1 + 10 files changed, 183 insertions(+), 60 deletions(-) diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 3fb76118aea..09b9f56b1b5 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -869,8 +869,8 @@ assert_equal '99', %q{ Ractor.new { inner = 99; eval("inner").to_s }.value } -# ivar in shareable-objects are not allowed to access from non-main Ractor -assert_equal "can not get unshareable values from instance variables of classes/modules from non-main Ractors (@iv from C)", <<~'RUBY', frozen_string_literal: false +# ivar in shareable-objects are not allowed to access from non-owner Ractor +assert_equal "can not get unshareable values from instance variables of classes/modules created by another Ractor (@iv from C)", <<~'RUBY', frozen_string_literal: false class C @iv = 'str' end @@ -1175,8 +1175,97 @@ assert_equal 'true', %q{ r.value } +# A shareable object belongs to no single Ractor, so its singleton class is the +# main Ractor's rather than the one which materialized it +assert_equal 'true', %q{ + r = Ractor.new do + begin + Ractor.main.define_singleton_method(:zzz) { :sub } + :not_raised + rescue Ractor::IsolationError + :raised + end + end + raised = r.value + Ractor.main.define_singleton_method(:zzz) { :main } + (raised == :raised && Ractor.main.zzz == :main).to_s +} + +# The constant inline cache is keyed on the Ractor which filled it, so a non-owner +# can not be handed an unshareable constant through a cache the owner primed +assert_equal 'Ractor::IsolationError', %q{ + port = Ractor::Port.new + r = Ractor.new(port) do |port| + k = Class.new + k.const_set(:X, [1, 2, 3]) + m = Module.new + m.const_set(:K, k) + m.module_eval("def self.rd = K::X") + m.rd # the owner fills the cache + port << m + Ractor.receive + end + m = port.receive + res = begin + m.rd + 'no error' + rescue Ractor::IsolationError + 'Ractor::IsolationError' + end + r << nil + r.join + res +} + +# Class#initialize writes the superclass of an uninitialized class, so it is +# owner-only like any other modification +assert_equal 'can not modify K because it is created by another Ractor', %q{ + K = Class.allocate + + r = Ractor.new { K.send(:initialize, Struct) } + + begin + r.join + rescue Ractor::RemoteError => e + e.cause.message + end +} + +# Module#refine writes its refinement tables into the receiver, so the receiver +# must be owned too +assert_equal 'can not modify M because it is created by another Ractor', %q{ + module M; end + + r = Ractor.new do + M.send(:refine, Class.new) { def z = 1 } + end + + begin + r.join + rescue Ractor::RemoteError => e + e.cause.message + end +} + +# ... and so does Module#ruby2_keywords, which rewrites a method definition +assert_equal 'can not modify M because it is created by another Ractor', %q{ + module M + def m(*a) = a + end + + r = Ractor.new do + M.send(:ruby2_keywords, :m) + end + + begin + r.join + rescue Ractor::RemoteError => e + e.cause.message + end +} + # Getting non-shareable objects via constants by other Ractors is not allowed -assert_equal 'can not access non-shareable objects in constant C::CONST by non-main Ractor.', <<~'RUBY', frozen_string_literal: false +assert_equal 'can not access non-shareable objects in constant C::CONST of a class/module created by another Ractor.', <<~'RUBY', frozen_string_literal: false class C CONST = 'str' end @@ -1191,7 +1280,7 @@ assert_equal 'can not access non-shareable objects in constant C::CONST by non-m RUBY # Constant cache should care about non-shareable constants -assert_equal "can not access non-shareable objects in constant Object::STR by non-main Ractor.", <<~'RUBY', frozen_string_literal: false +assert_equal "can not access non-shareable objects in constant Object::STR of a class/module created by another Ractor.", <<~'RUBY', frozen_string_literal: false STR = "hello" def str; STR; end s = str() # fill const cache @@ -1203,7 +1292,7 @@ assert_equal "can not access non-shareable objects in constant Object::STR by no RUBY # The correct constant path shall be reported -assert_equal "can not access non-shareable objects in constant Object::STR by non-main Ractor.", <<~'RUBY', frozen_string_literal: false +assert_equal "can not access non-shareable objects in constant Object::STR of a class/module created by another Ractor.", <<~'RUBY', frozen_string_literal: false STR = "hello" module M def self.str; STR; end @@ -1216,8 +1305,8 @@ assert_equal "can not access non-shareable objects in constant Object::STR by no end RUBY -# Setting non-shareable objects into constants by other Ractors is not allowed -assert_equal 'can not set constants with non-shareable objects by non-main Ractors', <<~'RUBY', frozen_string_literal: false +# Setting constants of classes created by other Ractors is not allowed +assert_equal 'can not set constants of classes/modules created by another Ractor', <<~'RUBY', frozen_string_literal: false class C end r = Ractor.new do @@ -1733,19 +1822,12 @@ assert_equal 'true', %q{ } # check method cache invalidation +# (the owner Ractor redefines methods while another Ractor calls them) 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 @@ -1753,7 +1835,12 @@ assert_equal 'true', %q{ end end - r1.value + 1000.times do + class Foo + def hello = nil + end + end + r2.value true @@ -2603,21 +2690,23 @@ RUBY assert_equal 'ok', <<~'RUBY' begin - CLASSES = 1000.times.map { Class.new }.freeze - + # Each Ractor creates its own class (it can only define bmethods on classes + # it owns) and returns it after defining the bmethod. # This would be better to run in parallel, but there's a bug with lambda # creation and YJIT causing crashes in dev mode - ractors = CLASSES.map do |klass| - Ractor.new(klass) do |klass| + ractors = 1000.times.map do + Ractor.new do + klass = Class.new Ractor.receive klass.define_method(:foo) {} + klass end end - ractors.each do |ractor| + CLASSES = ractors.map do |ractor| ractor << nil - ractor.join - end + ractor.value + end.freeze ractors.clear GC.start diff --git a/eval.c b/eval.c index ddefe300f5d..15a8cad2f4e 100644 --- a/eval.c +++ b/eval.c @@ -456,6 +456,7 @@ rb_class_modify_check(VALUE klass) } rb_error_frozen_object(klass); } + rb_class_owner_check(klass); } NORETURN(static void rb_longjmp(rb_execution_context_t *, enum ruby_tag_type, volatile VALUE, VALUE)); @@ -1636,6 +1637,11 @@ rb_mod_refine(VALUE module, VALUE klass) ensure_class_or_module(klass); + // refine installs refined method entries into the target's method table, and + // rb_refinement_setup writes the refinement tables into the receiver + rb_class_owner_check(module); + rb_class_owner_check(klass); + rb_refinement_setup(&data, module, klass); rb_yield_refine_block(data.refinement, data.refinements); diff --git a/object.c b/object.c index 6327f4aadc4..d76057640cf 100644 --- a/object.c +++ b/object.c @@ -2205,6 +2205,8 @@ rb_class_initialize(int argc, VALUE *argv, VALUE klass) { VALUE super; + // an uninitialized class is still somebody's, and this writes its superclass + rb_class_modify_check(klass); if (RCLASS_SUPER(klass) != 0 || klass == rb_cBasicObject) { rb_raise(rb_eTypeError, "already initialized class"); } diff --git a/test/ruby/test_iseq.rb b/test/ruby/test_iseq.rb index c01bb3dc90b..ffa0cbd5f39 100644 --- a/test/ruby/test_iseq.rb +++ b/test/ruby/test_iseq.rb @@ -878,17 +878,27 @@ class TestISeq < Test::Unit::TestCase def test_mandatory_only_redef assert_separately ['-W0'], <<~RUBY - r = Ractor.new{ - Float(10) - module Kernel - undef Float - def Float(n) - :new - end - end + port = Ractor::Port.new + r = Ractor.new(port){|port| + Float(10) # fill the mandatory only cache + port << :filled + Ractor.receive GC.start Float(30) } + + port.receive + + # Kernel is created by the main Ractor, so only the main Ractor can + # redefine it. The redefinition should be visible from the other Ractor. + module Kernel + undef Float + def Float(n) + :new + end + end + + r << :redefined assert_equal :new, r.value RUBY end diff --git a/variable.c b/variable.c index 2ff49546c8b..ee6e92a7062 100644 --- a/variable.c +++ b/variable.c @@ -1209,12 +1209,11 @@ rb_alias_variable(ID name1, ID name2) } static void -IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(ID id) +class_ivar_set_ractor_check(VALUE klass, ID id) { - if (UNLIKELY(!rb_ractor_main_p())) { - if (rb_is_instance_id(id)) { // check only normal ivars - rb_raise(rb_eRactorIsolationError, "can not set instance variables of classes/modules by non-main Ractors"); - } + if (rb_is_instance_id(id) && // check only normal ivars + UNLIKELY(!rb_class_owned_p(klass))) { + rb_raise(rb_eRactorIsolationError, "can not set instance variables of classes/modules created by another Ractor"); } } @@ -1244,6 +1243,10 @@ ivar_ractor_check(VALUE obj, ID id) UNLIKELY(!rb_ractor_main_p()) && UNLIKELY(rb_ractor_shareable_p(obj))) { + if (RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE)) { + // classes/modules are owner-checked at each read/write site instead + return; + } rb_raise(rb_eRactorIsolationError, "can not access instance variables of shareable objects from non-main Ractors"); } } @@ -1556,11 +1559,11 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef) } if (is_class && val != undef && rb_is_instance_id(id)) { - if (UNLIKELY(!rb_ractor_main_p()) && !rb_ractor_shareable_p(val)) { + if (UNLIKELY(!rb_class_owned_p(obj)) && !rb_ractor_shareable_p(val)) { rb_raise( rb_eRactorIsolationError, - "can not get unshareable values from instance variables of classes/modules from " - "non-main Ractors (%"PRIsVALUE" from %"PRIsVALUE")", + "can not get unshareable values from instance variables of classes/modules " + "created by another Ractor (%"PRIsVALUE" from %"PRIsVALUE")", rb_id2str(id), obj ); @@ -1593,9 +1596,9 @@ rb_ivar_get_at(VALUE obj, attr_index_t index, ID id) VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj); VALUE val = rb_imemo_fields_ptr(fields_obj)[index]; - if (UNLIKELY(!rb_ractor_main_p()) && !rb_ractor_shareable_p(val)) { + if (UNLIKELY(!rb_class_owned_p(obj)) && !rb_ractor_shareable_p(val)) { rb_raise(rb_eRactorIsolationError, - "can not get unshareable values from instance variables of classes/modules from non-main Ractors"); + "can not get unshareable values from instance variables of classes/modules created by another Ractor"); } return val; @@ -1648,7 +1651,7 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) int type = BUILTIN_TYPE(obj); if (type == T_CLASS || type == T_MODULE) { - IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id); + class_ivar_set_ractor_check(obj, id); if (rb_multi_ractor_p()) { concurrent = true; @@ -2042,7 +2045,7 @@ ivar_set(VALUE obj, ID id, VALUE val) case T_CLASS: case T_MODULE: { - IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id); + class_ivar_set_ractor_check(obj, id); bool dontcare; return class_ivar_set(obj, id, val, &dontcare); } @@ -2328,7 +2331,8 @@ rb_field_foreach(VALUE obj, rb_ivar_foreach_callback_func *func, st_data_t arg, case T_CLASS: case T_MODULE: { - IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(0); + // No owner check: every caller of this walk uses the names only for a + // class/module. Values are checked where they are read (rb_ivar_lookup). VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj); if (fields_obj) { imemo_fields_each(fields_obj, func, arg, ivar_only); @@ -2891,6 +2895,8 @@ rb_autoload_str(VALUE module, ID name, VALUE feature) rb_raise(rb_eNameError, "autoload must be constant name: %"PRIsVALUE"", QUOTE_ID(name)); } + rb_class_owner_check(module); + Check_Type(feature, T_STRING); if (!RSTRING_LEN(feature)) { rb_raise(rb_eArgError, "empty feature name"); @@ -3332,9 +3338,9 @@ rb_const_get_0(VALUE klass, ID id, int exclude, int recurse, int visibility) VALUE found_in; VALUE c = rb_const_search(klass, id, exclude, recurse, visibility, &found_in); if (!UNDEF_P(c)) { - if (UNLIKELY(!rb_ractor_main_p())) { + if (UNLIKELY(!rb_class_owned_p(found_in))) { if (!rb_ractor_shareable_p(c)) { - rb_raise(rb_eRactorIsolationError, "can not access non-shareable objects in constant %"PRIsVALUE"::%"PRIsVALUE" by non-main Ractor.", rb_class_path(found_in), rb_id2str(id)); + rb_raise(rb_eRactorIsolationError, "can not access non-shareable objects in constant %"PRIsVALUE"::%"PRIsVALUE" of a class/module created by another Ractor.", rb_class_path(found_in), rb_id2str(id)); } } return c; @@ -3546,6 +3552,7 @@ rb_const_remove(VALUE mod, ID id) rb_const_entry_t *ce; rb_check_frozen(mod); + rb_class_owner_check(mod); ce = rb_const_lookup(mod, id); @@ -3842,8 +3849,8 @@ const_set(VALUE klass, ID id, VALUE val) QUOTE_ID(id)); } - if (!rb_ractor_main_p() && !rb_ractor_shareable_p(val)) { - rb_raise(rb_eRactorIsolationError, "can not set constants with non-shareable objects by non-main Ractors"); + if (UNLIKELY(!rb_class_owned_p(klass))) { + rb_raise(rb_eRactorIsolationError, "can not set constants of classes/modules created by another Ractor"); } check_before_mod_set(klass, id, val, "constant"); diff --git a/vm_core.h b/vm_core.h index fa407563b53..97b1bfb694a 100644 --- a/vm_core.h +++ b/vm_core.h @@ -263,10 +263,13 @@ struct iseq_inline_constant_cache_entry { VALUE value; const rb_cref_t *ic_cref; + /* Ractor that filled this entry. An unshareable value may be handed out again + * only to that Ractor: it is the one that passed the owner check. */ + rb_serial_t ractor_id; }; STATIC_ASSERT(sizeof_iseq_inline_constant_cache_entry, - (offsetof(struct iseq_inline_constant_cache_entry, ic_cref) + - sizeof(const rb_cref_t *)) <= RVALUE_SIZE); + (offsetof(struct iseq_inline_constant_cache_entry, ractor_id) + + sizeof(rb_serial_t)) <= RVALUE_SIZE); struct iseq_inline_constant_cache { struct iseq_inline_constant_cache_entry *entry; diff --git a/vm_insnhelper.c b/vm_insnhelper.c index fa9df3eb9df..3d0b4734406 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1142,10 +1142,10 @@ vm_get_ev_const(rb_execution_context_t *ec, VALUE orig_klass, ID id, bool allow_ return 1; } else { - if (UNLIKELY(!rb_ractor_main_p())) { + if (UNLIKELY(!rb_class_owned_p(klass))) { if (!rb_ractor_shareable_p(val)) { rb_raise(rb_eRactorIsolationError, - "can not access non-shareable objects in constant %"PRIsVALUE"::%"PRIsVALUE" by non-main ractor.", rb_class_path(klass), rb_id2str(id)); + "can not access non-shareable objects in constant %"PRIsVALUE"::%"PRIsVALUE" of a class/module created by another Ractor.", rb_class_path(klass), rb_id2str(id)); } } return val; @@ -1255,10 +1255,10 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call case T_CLASS: case T_MODULE: { - if (UNLIKELY(!rb_ractor_main_p())) { - // For two reasons we can only use the fast path on the main - // ractor. - // First, only the main ractor is allowed to set ivars on classes + if (UNLIKELY(!rb_class_owned_p(obj))) { + // For two reasons we can only use the fast path on the Ractor + // that owns the class. + // First, only the owner Ractor is allowed to set ivars on classes // and modules. So we can skip locking. // Second, other ractors need to check the shareability of the // values returned from the class ivars. @@ -1428,7 +1428,7 @@ NOINLINE(static VALUE vm_setivar_class(VALUE obj, VALUE val, rb_setivar_cache ca static VALUE vm_setivar_class(VALUE obj, VALUE val, rb_setivar_cache cache) { - if (UNLIKELY(!rb_ractor_main_p())) { + if (UNLIKELY(!rb_class_owned_p(obj))) { return Qundef; } @@ -6708,9 +6708,11 @@ vm_ic_track_const_chain(rb_control_frame_t *cfp, IC ic, const ID *segments) // For JIT inlining static inline bool -vm_inlined_ic_hit_p(VALUE flags, VALUE value, const rb_cref_t *ic_cref, const VALUE *reg_ep) +vm_inlined_ic_hit_p(VALUE flags, VALUE value, const rb_cref_t *ic_cref, rb_serial_t ractor_id, const VALUE *reg_ep) { - if ((flags & IMEMO_CONST_CACHE_SHAREABLE) || rb_ractor_main_p()) { + // Not rb_ractor_main_p(): an owner Ractor may now cache an unshareable constant + // of its own class, and only it may be handed that value back. + if ((flags & IMEMO_CONST_CACHE_SHAREABLE) || ractor_id == rb_ractor_id(GET_RACTOR())) { VM_ASSERT(ractor_incidental_shareable_p(flags & IMEMO_CONST_CACHE_SHAREABLE, value)); return (ic_cref == NULL || // no need to check CREF @@ -6723,7 +6725,7 @@ static bool vm_ic_hit_p(const struct iseq_inline_constant_cache_entry *ice, const VALUE *reg_ep) { VM_ASSERT(IMEMO_TYPE_P(ice, imemo_constcache)); - return vm_inlined_ic_hit_p(ice->flags, ice->value, ice->ic_cref, reg_ep); + return vm_inlined_ic_hit_p(ice->flags, ice->value, ice->ic_cref, ice->ractor_id, reg_ep); } // YJIT needs this function to never allocate and never raise @@ -6745,6 +6747,7 @@ vm_ic_update(const rb_iseq_t *iseq, IC ic, VALUE val, const VALUE *reg_ep, const struct iseq_inline_constant_cache_entry *ice = SHAREABLE_IMEMO_NEW(struct iseq_inline_constant_cache_entry, imemo_constcache, 0); RB_OBJ_WRITE(ice, &ice->value, val); ice->ic_cref = vm_get_const_key_cref(reg_ep); + ice->ractor_id = rb_ractor_id(GET_RACTOR()); if (rb_ractor_shareable_p(val)) { RUBY_ASSERT((rb_gc_verify_shareable(val), 1)); diff --git a/vm_method.c b/vm_method.c index e0dc349e19c..3314f0449c7 100644 --- a/vm_method.c +++ b/vm_method.c @@ -3196,6 +3196,7 @@ rb_mod_ruby2_keywords(int argc, VALUE *argv, VALUE module) rb_check_arity(argc, 1, UNLIMITED_ARGUMENTS); rb_check_frozen(module); + rb_class_owner_check(module); for (i = 0; i < argc; i++) { VALUE v = argv[i]; diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 2b688521c80..88d9c043364 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -453,6 +453,7 @@ pub struct iseq_inline_constant_cache_entry { pub flags: VALUE, pub value: VALUE, pub ic_cref: *const rb_cref_t, + pub ractor_id: rb_serial_t, } #[repr(C)] #[derive(Debug, Copy, Clone)] diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index a6a826514a6..e0161a42373 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -593,6 +593,7 @@ pub struct iseq_inline_constant_cache_entry { pub flags: VALUE, pub value: VALUE, pub ic_cref: *const rb_cref_t, + pub ractor_id: rb_serial_t, } #[repr(C)] #[derive(Debug, Copy, Clone)]