diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 4158556c06b..5bead4ba9f6 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -888,50 +888,47 @@ assert_equal "can not get unshareable values from instance variables of classes/ end RUBY -# ivar in shareable-objects are not allowed to access from non-main Ractor -assert_equal 'can not access instance variables of shareable objects from non-main Ractors', %q{ +# setting an ivar on a shareable but unfrozen object is not allowed, by instance_variable_set +assert_equal "can't modify instance variables of a shareable Ractor", %q{ shared = Ractor.new{} - shared.instance_variable_set(:@iv, 'str') - - r = Ractor.new shared do |shared| - p shared.instance_variable_get(:@iv) - end begin - r.value - rescue Ractor::RemoteError => e - e.cause.message + shared.instance_variable_set(:@iv, 'str') + rescue Ractor::IsolationError => e + e.message end } -# ivar in shareable-objects are not allowed to access from non-main Ractor, by @iv (get) -assert_equal 'can not access instance variables of shareable objects from non-main Ractors', %q{ +# setting an ivar on a shareable but unfrozen object is not allowed, by @iv = ... +assert_equal "can't modify instance variables of a shareable Ractor", %q{ class Ractor def setup @foo = '' end - - def foo - @foo - end end shared = Ractor.new{} - shared.setup - - r = Ractor.new shared do |shared| - p shared.foo - end begin - r.value - rescue Ractor::RemoteError => e - e.cause.message + shared.setup + rescue Ractor::IsolationError => e + e.message end } -# ivar in shareable-objects are not allowed to access from non-main Ractor, by @iv (set) -assert_equal 'can not access instance variables of shareable objects from non-main Ractors', %q{ +# ivars of a shareable object are frozen, so they can be read from a non-main Ractor +assert_equal 'nil', %q{ + shared = Ractor.new{} + + r = Ractor.new shared do |shared| + shared.instance_variable_get(:@iv) + end + + r.value.inspect +} + +# setting an ivar on a shareable object is not allowed from a non-main Ractor either +assert_equal "can't modify instance variables of a shareable Ractor", %q{ class Ractor def setup @foo = '' @@ -941,7 +938,7 @@ assert_equal 'can not access instance variables of shareable objects from non-ma shared = Ractor.new{} r = Ractor.new shared do |shared| - p shared.setup + shared.setup end begin diff --git a/eval.c b/eval.c index 17a3f2743ff..89460c55e67 100644 --- a/eval.c +++ b/eval.c @@ -82,6 +82,7 @@ ruby_setup(void) rb_w32_init_long_paths(); #endif Init_BareVM(); + Init_default_shapes(); rb_vm_encoded_insn_data_table_init(); Init_enable_box(); Init_vm_objects(); diff --git a/hash.c b/hash.c index bc7bd5b4a36..279f540d379 100644 --- a/hash.c +++ b/hash.c @@ -36,6 +36,7 @@ #include "internal/hash.h" #include "internal/object.h" #include "internal/proc.h" +#include "internal/ractor.h" #include "internal/st.h" #include "internal/symbol.h" #include "internal/thread.h" diff --git a/inits.c b/inits.c index e4323cc4a84..95ef7402edf 100644 --- a/inits.c +++ b/inits.c @@ -20,7 +20,6 @@ static void Init_builtin_prelude(void); void rb_call_inits(void) { - CALL(default_shapes); CALL(Thread_Mutex); CALL(RandomSeedCore); CALL(encodings); diff --git a/internal/inits.h b/internal/inits.h index be73dac1dcb..4fcced17426 100644 --- a/internal/inits.h +++ b/internal/inits.h @@ -29,6 +29,9 @@ int Init_enc_set_filesystem_encoding(void); /* newline.c */ void Init_newline(void); +/* shape.c */ +void Init_default_shapes(void); + /* vm.c */ void Init_BareVM(void); void Init_vm_objects(void); diff --git a/internal/variable.h b/internal/variable.h index 47d4c86090f..2c832819671 100644 --- a/internal/variable.h +++ b/internal/variable.h @@ -50,6 +50,7 @@ void rb_obj_replace_fields(VALUE obj, VALUE fields_obj); VALUE rb_obj_complex_fields_build(VALUE obj); VALUE rb_obj_field_get(VALUE obj, shape_id_t target_shape_id); void rb_ivar_set_internal(VALUE obj, ID id, VALUE val); +void rb_check_ivar_modifiable(VALUE obj); void rb_ivar_foreach_buffered(VALUE obj, int (*func)(ID name, VALUE val, st_data_t arg), st_data_t arg); attr_index_t rb_ivar_set_index(VALUE obj, ID id, VALUE val); attr_index_t rb_obj_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE val); diff --git a/ractor.c b/ractor.c index 3c2b54425ce..110294a94a3 100644 --- a/ractor.c +++ b/ractor.c @@ -715,8 +715,7 @@ ractor_alloc(VALUE klass) { rb_ractor_t *r; VALUE rv = TypedData_Make_Struct(klass, rb_ractor_t, &ractor_data_type, r); - FL_SET_RAW(rv, RUBY_FL_SHAREABLE); - rb_gc_obj_became_shareable(rv); + RB_OBJ_SET_SHAREABLE(rv); r->pub.self = rv; r->next_ec_serial = 1; VM_ASSERT(ractor_status_p(r, ractor_created)); @@ -831,8 +830,7 @@ void rb_ractor_main_setup(rb_vm_t *vm, rb_ractor_t *r, rb_thread_t *th) { VALUE rv = r->pub.self = TypedData_Wrap_Struct(rb_cRactor, &ractor_data_type, r); - FL_SET_RAW(r->pub.self, RUBY_FL_SHAREABLE); - rb_gc_obj_became_shareable(r->pub.self); + RB_OBJ_SET_SHAREABLE(r->pub.self); ractor_init(r, Qnil, Qnil); r->threads.main = th; rb_ractor_living_threads_insert(r, th); @@ -1469,6 +1467,16 @@ rb_obj_set_shareable_no_assert(VALUE obj) FL_SET_RAW(obj, FL_SHAREABLE); rb_gc_obj_became_shareable(obj); + /* Ivars on a shareable object would be mutable shared state, so freeze them + * (not obj itself). A T_IMEMO has no shape id to transition. */ + bool froze_ivars = false; + if (!RB_OBJ_FROZEN_RAW(obj) && !RB_TYPE_P(obj, T_IMEMO) && + !RB_TYPE_P(obj, T_CLASS) && !RB_TYPE_P(obj, T_MODULE) && !RB_TYPE_P(obj, T_ICLASS)) { + + RBASIC_SET_SHAPE_ID(obj, rb_shape_transition_frozen(RBASIC_SHAPE_ID(obj))); + froze_ivars = true; + } + /* A T_OBJECT can have a fields imemo too (too_complex and friends), and an imemo * born while its owner was unshareable stays unshareable * (imemo_fields_complex_from_obj), so align it here. */ @@ -1481,6 +1489,8 @@ rb_obj_set_shareable_no_assert(VALUE obj) // no recursive mark FL_SET_RAW(fields, FL_SHAREABLE); rb_gc_obj_became_shareable(fields); + // the imemo carries its owner's shape id, frozen bit included + if (froze_ivars) RBASIC_SET_SHAPE_ID(fields, RBASIC_SHAPE_ID(obj)); // Field values the traversal never reaches (hidden internal ivars, say) // can stay unshareable, so record their shrefs to keep the shareable // fields imemo's edges correct. @@ -3422,9 +3432,12 @@ ractor_native_shallow_copy(VALUE obj) } /* The traversal rewrites the children inside the copy with raw stores, so the frozen - * bit can be set now: by the time leave runs the original is out of sight. */ + * bit can be set now: by the time leave runs the original is out of sight. The shape + * has to be transitioned along with the flag, because field writes are refused based + * on the shape (see rb_check_ivar_modifiable). */ if (OBJ_FROZEN(obj)) { RB_FL_SET_RAW(copy, RUBY_FL_FREEZE); + RBASIC_SET_SHAPE_ID(copy, rb_obj_shape_transition_frozen(copy)); } return copy; } diff --git a/ractor_sync.c b/ractor_sync.c index 3eb83a6550b..047ea540b53 100644 --- a/ractor_sync.c +++ b/ractor_sync.c @@ -1016,8 +1016,7 @@ rb_ractor_setup_default_port(rb_ractor_t *r) { VM_ASSERT(r->sync.default_port_value == Qfalse); r->sync.default_port_value = ractor_port_new(r); - FL_SET_RAW(r->sync.default_port_value, RUBY_FL_SHAREABLE); // only default ports are shareable - rb_gc_obj_became_shareable(r->sync.default_port_value); + RB_OBJ_SET_SHAREABLE(r->sync.default_port_value); } // Ractor#value diff --git a/shape.h b/shape.h index 2a416b1b126..de83d06b876 100644 --- a/shape.h +++ b/shape.h @@ -29,7 +29,8 @@ STATIC_ASSERT(shape_id_num_bits, SHAPE_ID_NUM_BITS == sizeof(shape_id_t) * CHAR_ // 26 SHAPE_ID_FL_COMPLEX // The object is backed by a `st_table`. // 27 SHAPE_ID_FL_FROZEN -// Whether the object is frozen or not. +// Whether field writes are refused: either the object is frozen, or it +// became shareable while unfrozen (see rb_obj_set_shareable in ractor.c). // 28 SHAPE_ID_FL_HAS_OBJECT_ID // Whether the object has an `SHAPE_OBJ_ID` transition. // 29-30 SHAPE_ID_LAYOUT_MASK diff --git a/test/ruby/test_env.rb b/test/ruby/test_env.rb index 48409f84356..3e928a5b52b 100644 --- a/test/ruby/test_env.rb +++ b/test/ruby/test_env.rb @@ -1474,25 +1474,13 @@ class TestEnv < Test::Unit::TestCase end; end - def test_ivar_in_env_should_not_be_access_from_non_main_ractors + def test_ivar_in_env_is_not_allowed + # ENV is shareable but can never be frozen, so it may not carry instance + # variables at all: they would be unshareable values reachable from any ractor. assert_ractor <<~RUBY - ENV.instance_eval{ @a = "hello" } - assert_equal "hello", ENV.instance_variable_get(:@a) - - r_get = Ractor.new do - ENV.instance_variable_get(:@a) - rescue Ractor::IsolationError => e - e - end - assert_equal Ractor::IsolationError, r_get.value.class - - r_get = Ractor.new do - ENV.instance_eval{ @a } - rescue Ractor::IsolationError => e - e - end - - assert_equal Ractor::IsolationError, r_get.value.class + assert_raise(Ractor::IsolationError) { ENV.instance_eval{ @a = "hello" } } + assert_nil ENV.instance_variable_get(:@a) + assert_equal [], ENV.instance_variables r_set = Ractor.new do ENV.instance_eval{ @b = "hello" } @@ -1501,6 +1489,13 @@ class TestEnv < Test::Unit::TestCase end assert_equal Ractor::IsolationError, r_set.value.class + + # Reads are allowed: since writes are forbidden, there is nothing + # unshareable to read. + r_get = Ractor.new do + ENV.instance_variable_get(:@a) + end + assert_nil r_get.value RUBY end diff --git a/variable.c b/variable.c index 9eab776a554..a5e5d465fc5 100644 --- a/variable.c +++ b/variable.c @@ -30,6 +30,7 @@ #include "internal/object.h" #include "internal/gc.h" #include "internal/re.h" +#include "internal/string.h" #include "internal/struct.h" #include "internal/symbol.h" #include "internal/thread.h" @@ -1280,19 +1281,16 @@ cvar_read_ractor_check(VALUE klass, ID id, VALUE val) } static inline void -ivar_ractor_check(VALUE obj, ID id) +ivar_ractor_assert(VALUE obj, ID id) { - if (LIKELY(rb_is_instance_id(id)) /* not internal ID */ && - !RB_OBJ_FROZEN_RAW(obj) && - 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"); - } + RUBY_ASSERT(!rb_is_instance_id(id) /* internal ID */ || + SPECIAL_CONST_P(obj) || + !rb_ractor_shareable_p(obj) || + RB_OBJ_FROZEN_RAW(obj) || + RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE) || + RB_TYPE_P(obj, T_ICLASS) || RB_TYPE_P(obj, T_IMEMO) || + rb_shape_frozen_p(RBASIC_SHAPE_ID(obj)), + "shareable object must not have writable instance variables"); } struct st_table * @@ -1378,7 +1376,7 @@ obj_use_generic_fields_tbl_p(VALUE obj) VALUE rb_obj_fields(VALUE obj, ID field_name) { - ivar_ractor_check(obj, field_name); + ivar_ractor_assert(obj, field_name); switch (BUILTIN_TYPE(obj)) { case T_IMEMO: @@ -1467,7 +1465,7 @@ rb_free_generic_ivar(VALUE obj) static void rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fields_obj) { - ivar_ractor_check(obj, field_name); + ivar_ractor_assert(obj, field_name); if (!fields_obj) { RUBY_ASSERT(original_fields_obj); @@ -2035,6 +2033,25 @@ obj_ivar_set(VALUE obj, ID id, VALUE val) return obj_field_set(obj, target_shape_id, id, val); } +void +rb_check_ivar_modifiable(VALUE obj) +{ + if (UNLIKELY(!RB_FL_ABLE(obj) || rb_shape_frozen_p(RBASIC_SHAPE_ID(obj)))) { + rb_check_frozen(obj); + + RUBY_ASSERT(RB_OBJ_SHAREABLE_P(obj), "unfrozen object with a frozen shape must be shareable"); + + rb_raise(rb_eRactorIsolationError, + "can't modify instance variables of a shareable %"PRIsVALUE, + rb_obj_class(obj)); + } + else if (UNLIKELY(CHILLED_STRING_P(obj))) { + CHILLED_STRING_MUTATED(obj); + } + + RUBY_ASSERT(!RB_OBJ_FROZEN_RAW(obj), "frozen object with an unfrozen shape"); +} + /* Set the instance variable +val+ on object +obj+ at ivar name +id+. * This function only works with T_OBJECT objects, so make sure * +obj+ is of type T_OBJECT before using this function. @@ -2042,7 +2059,7 @@ obj_ivar_set(VALUE obj, ID id, VALUE val) VALUE rb_vm_set_ivar_id(VALUE obj, ID id, VALUE val) { - rb_check_frozen(obj); + rb_check_ivar_modifiable(obj); obj_ivar_set(obj, id, val); return val; } @@ -2101,7 +2118,7 @@ ivar_set(VALUE obj, ID id, VALUE val) VALUE rb_ivar_set(VALUE obj, ID id, VALUE val) { - rb_check_frozen(obj); + rb_check_ivar_modifiable(obj); ivar_set(obj, id, val); return val; } diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 1f9d8acc805..026b93a6f5a 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1394,7 +1394,7 @@ vm_setivar_slowpath(VALUE obj, ID id, VALUE val, const rb_iseq_t *iseq, IVC ic, #if OPT_IC_FOR_IVAR RB_DEBUG_COUNTER_INC(ivar_set_ic_miss); - rb_check_frozen(obj); + rb_check_ivar_modifiable(obj); shape_id_t previous_shape_id = RBASIC_SHAPE_ID(obj); attr_index_t index = rb_ivar_set_index(obj, id, val);