[pull] master from ruby:master

array.c

@@ -3963,6 +3963,9 @@ append_values_at_single(VALUE result, VALUE ary, long olen, VALUE idx)
     /* check if idx is Range */
     else if (rb_range_beg_len(idx, &beg, &len, olen, 1)) {
         if (len > 0) {
+            // rb_range_beg_len may run arbitrary code that modifies ary, so we
+            // need to re-calculate olen
+            const long olen = RARRAY_LEN(ary);
             const VALUE *const src = RARRAY_CONST_PTR(ary);
             const long end = beg + len;
             const long prevlen = RARRAY_LEN(result);

bootstraptest/test_ractor.rb

@@ -888,50 +888,47 @@ class C
   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
+  end
 
-    def foo
-      @foo
-    end
+  shared = Ractor.new{}
+
+  begin
+    shared.setup
+  rescue Ractor::IsolationError => e
+    e.message
   end
+}
 
+# ivars of a shareable object are frozen, so they can be read from a non-main Ractor
+assert_equal 'nil', %q{
   shared = Ractor.new{}
-  shared.setup
 
   r = Ractor.new shared do |shared|
-    p shared.foo
+    shared.instance_variable_get(:@iv)
   end
 
-  begin
-    r.value
-  rescue Ractor::RemoteError => e
-    e.cause.message
-  end
+  r.value.inspect
 }
 
-# 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{
+# 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 @@ def setup
   shared = Ractor.new{}
 
   r = Ractor.new shared do |shared|
-    p shared.setup
+    shared.setup
   end
 
   begin
@@ -951,6 +948,69 @@ def setup
   end
 }
 
+# a Proc which already has ivars can not be isolated
+assert_equal 'can not isolate a Proc because it has instance variables', %q{
+  pr = Proc.new{}
+  pr.instance_variable_set(:@iv, Object.new)
+
+  begin
+    Ractor.new(&pr)
+  rescue Ractor::IsolationError => e
+    e.message
+  end
+}
+
+# freezing the Proc first does not skip the check
+assert_equal 'can not isolate a Proc because it has instance variables', %q{
+  pr = Proc.new{}
+  pr.instance_variable_set(:@iv, Object.new)
+  pr.freeze
+
+  begin
+    Ractor.new(&pr)
+  rescue Ractor::IsolationError => e
+    e.message
+  end
+}
+
+# an ivar set by Proc#refined is internal, so it does not block isolation
+assert_equal 'r', %q{
+  module M
+    refine String do
+      def foo; 'r'; end
+    end
+  end
+
+  rp = Proc.new{ ''.foo }.refined(M)
+  Ractor.new(&rp).value
+}
+
+# a Proc made shareable by Ractor.new can not be given ivars afterwards
+assert_equal "can't modify instance variables of a shareable Proc", %q{
+  HAX = -> { }
+  Ractor.new(&HAX).join
+
+  begin
+    HAX.instance_variable_set(:@foo, Object.new)
+  rescue Ractor::IsolationError => e
+    e.message
+  end
+}
+
+# freezing a shareable object from another Ractor can not expose an ivar, since
+# none could be set after it became shareable
+assert_equal 'nil', %q{
+  HAX = -> { }
+  Ractor.new(&HAX).join
+
+  r = Ractor.new do
+    HAX.freeze
+    HAX.instance_variable_get(:@foo)
+  end
+
+  r.value.inspect
+}
+
 # But a shareable object is frozen, it is allowed to access ivars from non-main Ractor
 assert_equal '11', %q{
   [Object.new, [], ].map{|obj|

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_array.rb

@@ -2977,6 +2977,18 @@ def test_values_at2
     assert_equal([nil], a.values_at(2**31-1))
   end
 
+  def test_values_at_ary_modify
+    a = (0..100_000).to_a
+    obj = Object.new
+    obj.define_singleton_method(:begin) do
+      a.clear
+      0
+    end
+    obj.define_singleton_method(:end) { 10_000 }
+    obj.define_singleton_method(:exclude_end?) { false }
+    assert_equal(10_001, a.values_at(obj).length)
+  end
+
   def test_select
     assert_equal([0, 2], [0, 1, 2, 3].select {|x| x % 2 == 0 })
   end

test/ruby/test_env.rb

@@ -1474,25 +1474,13 @@ def test_shared_substring_in_ractor
     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 @@ def test_ivar_in_env_should_not_be_access_from_non_main_ractors
     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