summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKoichi Sasada <ko1@atdot.net>2020-12-11 16:37:20 +0900
committerKoichi Sasada <ko1@atdot.net>2020-12-12 06:19:18 +0900
commitd741c77b5fd976300815c1ea987e76e92b71122f (patch)
tree852e8cc6d6d66193cbb54c0ac9d2b6e7a1a239d6
parent31e8de2920935d500105949bda931f3ca22cdbff (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.rb47
-rw-r--r--variable.c17
-rw-r--r--variable.h1
-rw-r--r--vm_insnhelper.c82
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)))) {