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/