[前][次][番号順一覧][スレッド一覧]

ruby-changes:63811

From: Koichi <ko1@a...>
Date: Tue, 1 Dec 2020 13:18:51 +0900 (JST)
Subject: [ruby-changes:63811] 8247b8edde (master): should not use rb_ary_modify()

https://git.ruby-lang.org/ruby.git/commit/?id=8247b8edde

From 8247b8eddeb2a504a5c9776d1f77d413c8146897 Mon Sep 17 00:00:00 2001
From: Koichi Sasada <ko1@a...>
Date: Tue, 1 Dec 2020 11:14:36 +0900
Subject: should not use rb_ary_modify()

ractor_copy() used rb_ary_modify() to make sure this array is not
sharing anything, but it also checks frozen flag. So frozen arrays
raises an error. To solve this issue, this patch introduces new
function rb_ary_cancel_sharing() which makes sure the array does not
share another array and it doesn't check frozen flag.
[Bug #17343]

A test is quoted from https://github.com/ruby/ruby/pull/3817

diff --git a/array.c b/array.c
index ba13ac5..d8e76b0 100644
--- a/array.c
+++ b/array.c
@@ -562,34 +562,33 @@ rb_ary_modify_check(VALUE ary) https://github.com/ruby/ruby/blob/trunk/array.c#L562
 }
 
 void
-rb_ary_modify(VALUE ary)
+rb_ary_cancel_sharing(VALUE ary)
 {
-    rb_ary_modify_check(ary);
     if (ARY_SHARED_P(ary)) {
-	long shared_len, len = RARRAY_LEN(ary);
+        long shared_len, len = RARRAY_LEN(ary);
         VALUE shared_root = ARY_SHARED_ROOT(ary);
 
         ary_verify(shared_root);
 
         if (len <= RARRAY_EMBED_LEN_MAX) {
-	    const VALUE *ptr = ARY_HEAP_PTR(ary);
+            const VALUE *ptr = ARY_HEAP_PTR(ary);
             FL_UNSET_SHARED(ary);
             FL_SET_EMBED(ary);
-	    MEMCPY((VALUE *)ARY_EMBED_PTR(ary), ptr, VALUE, len);
+            MEMCPY((VALUE *)ARY_EMBED_PTR(ary), ptr, VALUE, len);
             rb_ary_decrement_share(shared_root);
             ARY_SET_EMBED_LEN(ary, len);
         }
         else if (ARY_SHARED_ROOT_OCCUPIED(shared_root) && len > ((shared_len = RARRAY_LEN(shared_root))>>1)) {
             long shift = RARRAY_CONST_PTR_TRANSIENT(ary) - RARRAY_CONST_PTR_TRANSIENT(shared_root);
-	    FL_UNSET_SHARED(ary);
+            FL_UNSET_SHARED(ary);
             ARY_SET_PTR(ary, RARRAY_CONST_PTR_TRANSIENT(shared_root));
-	    ARY_SET_CAPA(ary, shared_len);
+            ARY_SET_CAPA(ary, shared_len);
             RARRAY_PTR_USE_TRANSIENT(ary, ptr, {
-		MEMMOVE(ptr, ptr+shift, VALUE, len);
-	    });
+                MEMMOVE(ptr, ptr+shift, VALUE, len);
+            });
             FL_SET_EMBED(shared_root);
             rb_ary_decrement_share(shared_root);
-	}
+        }
         else {
             VALUE *ptr = ary_heap_alloc(ary, len);
             MEMCPY(ptr, ARY_HEAP_PTR(ary), VALUE, len);
@@ -598,11 +597,18 @@ rb_ary_modify(VALUE ary) https://github.com/ruby/ruby/blob/trunk/array.c#L597
             ARY_SET_PTR(ary, ptr);
         }
 
-	rb_gc_writebarrier_remember(ary);
+        rb_gc_writebarrier_remember(ary);
     }
     ary_verify(ary);
 }
 
+void
+rb_ary_modify(VALUE ary)
+{
+    rb_ary_modify_check(ary);
+    rb_ary_cancel_sharing(ary);
+}
+
 static VALUE
 ary_ensure_room_for_push(VALUE ary, long add_len)
 {
diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb
index 7ab6a08..2ae6602 100644
--- a/bootstraptest/test_ractor.rb
+++ b/bootstraptest/test_ractor.rb
@@ -1022,6 +1022,13 @@ assert_equal 'can not make a Proc shareable because it accesses outer variables https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_ractor.rb#L1022
   end
 }
 
+# Ractor deep copies frozen objects
+assert_equal '[true, false]', %q{
+  Ractor.new([[]].freeze) { |ary|
+    [ary.frozen?, ary.first.frozen? ]
+  }.take
+}
+
 ###
 ### Synchronization tests
 ###
diff --git a/internal/array.h b/internal/array.h
index c9f2b47..a7bf6d3 100644
--- a/internal/array.h
+++ b/internal/array.h
@@ -29,6 +29,8 @@ VALUE rb_ary_tmp_new_fill(long capa); https://github.com/ruby/ruby/blob/trunk/internal/array.h#L29
 VALUE rb_ary_at(VALUE, VALUE);
 size_t rb_ary_memsize(VALUE);
 VALUE rb_to_array_type(VALUE obj);
+void rb_ary_cancel_sharing(VALUE ary);
+
 static inline VALUE rb_ary_entry_internal(VALUE ary, long offset);
 static inline bool ARY_PTR_USING_P(VALUE ary);
 static inline void RARY_TRANSIENT_SET(VALUE ary);
diff --git a/ractor.c b/ractor.c
index 4a75b41..693bbe2 100644
--- a/ractor.c
+++ b/ractor.c
@@ -2314,7 +2314,7 @@ obj_traverse_replace_i(VALUE obj, struct obj_traverse_replace_data *data) https://github.com/ruby/ruby/blob/trunk/ractor.c#L2314
 
       case T_ARRAY:
         {
-            rb_ary_modify(obj);
+            rb_ary_cancel_sharing(obj);
 #if USE_TRANSIENT_HEAP
             if (data->move) rb_ary_transient_heap_evacuate(obj, TRUE);
 #endif
@@ -2537,7 +2537,8 @@ copy_leave(VALUE obj, struct obj_traverse_replace_data *data) https://github.com/ruby/ruby/blob/trunk/ractor.c#L2537
     return traverse_cont;
 }
 
-static VALUE ractor_copy(VALUE obj)
+static VALUE
+ractor_copy(VALUE obj)
 {
     VALUE val = rb_obj_traverse_replace(obj, copy_enter, copy_leave, false);
     if (val != Qundef) {
-- 
cgit v0.10.2


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]