diff options
author | Koichi Sasada <ko1@atdot.net> | 2020-12-11 16:37:20 +0900 |
---|---|---|
committer | Koichi Sasada <ko1@atdot.net> | 2020-12-12 06:19:18 +0900 |
commit | d741c77b5fd976300815c1ea987e76e92b71122f (patch) | |
tree | 852e8cc6d6d66193cbb54c0ac9d2b6e7a1a239d6 | |
parent | 31e8de2920935d500105949bda931f3ca22cdbff (diff) |
fix ivar with shareable objects issue
Instance variables of sharable objects are accessible only from
main ractor, so we need to check it correctly.
Notes
Notes:
Merged: https://github.com/ruby/ruby/pull/3887
-rw-r--r-- | bootstraptest/test_ractor.rb | 47 | ||||
-rw-r--r-- | variable.c | 17 | ||||
-rw-r--r-- | variable.h | 1 | ||||
-rw-r--r-- | vm_insnhelper.c | 82 |
4 files changed, 105 insertions, 42 deletions
diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 2bbf823f37..81bea95e01 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -809,6 +809,53 @@ assert_equal 'can not access instance variables of shareable objects from non-ma 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{ + 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.take + rescue Ractor::RemoteError => e + e.cause.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{ + class Ractor + def setup + @foo = '' + end + end + + shared = Ractor.new{} + + r = Ractor.new shared do |shared| + p shared.setup + end + + begin + r.take + rescue Ractor::RemoteError => e + e.cause.message + end +} + # But a shareable object is frozen, it is allowed to access ivars from non-main Ractor assert_equal '11', %q{ [Object.new, [], ].map{|obj| diff --git a/variable.c b/variable.c index 81c0a7da2f..45385fb9e0 100644 --- a/variable.c +++ b/variable.c @@ -926,6 +926,8 @@ generic_ivtbl(VALUE obj, ID id, bool force_check_ractor) !RB_OBJ_FROZEN_RAW(obj) && UNLIKELY(!rb_ractor_main_p()) && UNLIKELY(rb_ractor_shareable_p(obj))) { + + // TODO: RuntimeError? rb_raise(rb_eRuntimeError, "can not access instance variables of shareable objects from non-main Ractors"); } return generic_iv_tbl_; @@ -961,6 +963,21 @@ rb_ivar_generic_ivtbl_lookup(VALUE obj, struct gen_ivtbl **ivtbl) return gen_ivtbl_get(obj, 0, ivtbl); } +MJIT_FUNC_EXPORTED VALUE +rb_ivar_generic_lookup_with_index(VALUE obj, ID id, uint32_t index) +{ + struct gen_ivtbl *ivtbl; + + if (gen_ivtbl_get(obj, id, &ivtbl)) { + if (LIKELY(index < ivtbl->numiv)) { + VALUE val = ivtbl->ivptr[index]; + return val; + } + } + + return Qundef; +} + static VALUE generic_ivar_delete(VALUE obj, ID id, VALUE undef) { diff --git a/variable.h b/variable.h index 4d71f87bc5..bfe1be2d47 100644 --- a/variable.h +++ b/variable.h @@ -17,5 +17,6 @@ struct gen_ivtbl { }; int rb_ivar_generic_ivtbl_lookup(VALUE obj, struct gen_ivtbl **); +VALUE rb_ivar_generic_lookup_with_index(VALUE obj, ID id, uint32_t index); #endif /* RUBY_TOPLEVEL_VARIABLE_H */ diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 9205273199..9a94b1070b 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1095,6 +1095,21 @@ iv_index_tbl_lookup(struct st_table *iv_index_tbl, ID id, struct rb_iv_index_tbl return found ? true : false; } +ALWAYS_INLINE(static void fill_ivar_cache(const rb_iseq_t *iseq, IVC ic, const struct rb_callcache *cc, int is_attr, struct rb_iv_index_tbl_entry *ent)); + +static inline void +fill_ivar_cache(const rb_iseq_t *iseq, IVC ic, const struct rb_callcache *cc, int is_attr, struct rb_iv_index_tbl_entry *ent) +{ + // fill cache + if (!is_attr) { + ic->entry = ent; + RB_OBJ_WRITTEN(iseq, Qundef, ent->class_value); + } + else { + vm_cc_attr_index_set(cc, (int)ent->index + 1); + } +} + ALWAYS_INLINE(static VALUE vm_getivar(VALUE, ID, const rb_iseq_t *, IVC, const struct rb_callcache *, int)); static inline VALUE vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_callcache *cc, int is_attr) @@ -1116,38 +1131,38 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call if (LIKELY(BUILTIN_TYPE(obj) == T_OBJECT) && LIKELY(index < ROBJECT_NUMIV(obj))) { val = ROBJECT_IVPTR(obj)[index]; + + VM_ASSERT(rb_ractor_shareable_p(obj) ? rb_ractor_shareable_p(val) : true); } else if (FL_TEST_RAW(obj, FL_EXIVAR)) { - struct gen_ivtbl *ivtbl; - - if (LIKELY(rb_ivar_generic_ivtbl_lookup(obj, &ivtbl)) && - LIKELY(index < ivtbl->numiv)) { - val = ivtbl->ivptr[index]; - } + val = rb_ivar_generic_lookup_with_index(obj, id, index); } + goto ret; } else { - struct st_table *iv_index_tbl; - st_index_t numiv; - VALUE *ivptr; + struct rb_iv_index_tbl_entry *ent; if (BUILTIN_TYPE(obj) == T_OBJECT) { - iv_index_tbl = ROBJECT_IV_INDEX_TBL(obj); - numiv = ROBJECT_NUMIV(obj); - ivptr = ROBJECT_IVPTR(obj); - goto fill; - } - else if (FL_TEST_RAW(obj, FL_EXIVAR)) { - struct gen_ivtbl *ivtbl; - if (LIKELY(rb_ivar_generic_ivtbl_lookup(obj, &ivtbl))) { - numiv = ivtbl->numiv; - ivptr = ivtbl->ivptr; - iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj)); - goto fill; + struct st_table *iv_index_tbl = ROBJECT_IV_INDEX_TBL(obj); + + if (iv_index_tbl && iv_index_tbl_lookup(iv_index_tbl, id, &ent)) { + fill_ivar_cache(iseq, ic, cc, is_attr, ent); + + // get value + if (ent->index < ROBJECT_NUMIV(obj)) { + val = ROBJECT_IVPTR(obj)[ent->index]; + + VM_ASSERT(rb_ractor_shareable_p(obj) ? rb_ractor_shareable_p(val) : true); + } } - else { - goto ret; + } + else if (FL_TEST_RAW(obj, FL_EXIVAR)) { + struct st_table *iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj)); + + if (iv_index_tbl && iv_index_tbl_lookup(iv_index_tbl, id, &ent)) { + fill_ivar_cache(iseq, ic, cc, is_attr, ent); + val = rb_ivar_generic_lookup_with_index(obj, id, ent->index); } } else { @@ -1155,25 +1170,6 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call goto general_path; } - fill: - if (iv_index_tbl) { - struct rb_iv_index_tbl_entry *ent; - - if (iv_index_tbl_lookup(iv_index_tbl, id, &ent)) { - if (!is_attr) { - ic->entry = ent; - RB_OBJ_WRITTEN(iseq, Qundef, ent->class_value); - } - else { /* call_info */ - vm_cc_attr_index_set(cc, (int)ent->index + 1); - } - - if (ent->index < numiv) { - val = ivptr[ent->index]; - } - } - } - ret: if (LIKELY(val != Qundef)) { return val; @@ -1204,6 +1200,8 @@ vm_setivar(VALUE obj, ID id, VALUE val, const rb_iseq_t *iseq, IVC ic, const str VALUE klass = RBASIC(obj)->klass; uint32_t index; + VM_ASSERT(!rb_ractor_shareable_p(obj)); + if (LIKELY( (!is_attr && RB_DEBUG_COUNTER_INC_UNLESS(ivar_set_ic_miss_serial, ic->entry && ic->entry->class_serial == RCLASS_SERIAL(klass))) || ( is_attr && RB_DEBUG_COUNTER_INC_UNLESS(ivar_set_ic_miss_unset, vm_cc_attr_index(cc) > 0)))) { |