summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKoichi Sasada <ko1@atdot.net>2020-12-01 11:14:36 +0900
committerKoichi Sasada <ko1@atdot.net>2020-12-01 13:18:32 +0900
commit8247b8eddeb2a504a5c9776d1f77d413c8146897 (patch)
treea0464f31f918b06b6d5a377aed8a3d173a61f9dc
parentd2cfb5228a89678a712efd381e049391800373e1 (diff)
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
Notes
Notes: Merged: https://github.com/ruby/ruby/pull/3831
-rw-r--r--array.c28
-rw-r--r--bootstraptest/test_ractor.rb7
-rw-r--r--internal/array.h2
-rw-r--r--ractor.c5
4 files changed, 29 insertions, 13 deletions
diff --git a/array.c b/array.c
index ba13ac54e5..d8e76b050b 100644
--- a/array.c
+++ b/array.c
@@ -562,34 +562,33 @@ rb_ary_modify_check(VALUE ary)
}
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)
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 7ab6a08875..2ae6602c82 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
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 c9f2b47145..a7bf6d3868 100644
--- a/internal/array.h
+++ b/internal/array.h
@@ -29,6 +29,8 @@ VALUE rb_ary_tmp_new_fill(long capa);
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 4a75b41fc5..693bbe274d 100644
--- a/ractor.c
+++ b/ractor.c
@@ -2314,7 +2314,7 @@ obj_traverse_replace_i(VALUE obj, struct obj_traverse_replace_data *data)
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)
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) {