Forbid setting ivars on Ractor shareable objects

Previously we had a check that objects which were shareable, but not
frozen on a non-main Ractor would raise an exception if instance
variables were read. This prevented a non-main Ractor reading
non-shareable objects.

This changes the behaviour to forbid all writes to shareable objects
(even when unfrozen). This is done by re-purposing the frozen flag on
the object shape to reflect (FL_FROZEN || FL_SHAREABLE).

With IV writes forbidden on all shareable objects whether or not they
are frozen (with the exception of classes and modules), we can also
remove the check against reading instance variables, since no unsafe
instance variables can be set.

[Bug #22291]

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

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();

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"

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);

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);

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);

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;
 }

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

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

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
 

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;
 }

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);