ruby-changes:69341
From: Koichi <ko1@a...>
Date: Sat, 23 Oct 2021 01:33:11 +0900 (JST)
Subject: [ruby-changes:69341] acb23454e5 (master): allow to access ivars of classes/modules
https://git.ruby-lang.org/ruby.git/commit/?id=acb23454e5 From acb23454e57e1bbe828e7f3114430cab2d5db44c Mon Sep 17 00:00:00 2001 From: Koichi Sasada <ko1@a...> Date: Fri, 22 Oct 2021 17:24:34 +0900 Subject: allow to access ivars of classes/modules if an ivar of a class/module refer to a shareable object, this ivar can be read from non-main Ractors. --- benchmark/vm_ivar_of_class.yml | 12 +++++++ bootstraptest/test_ractor.rb | 49 ++++++++++++++++++++++++++- bootstraptest/test_yjit.rb | 4 +-- variable.c | 76 ++++++++++++++++++++++++++++++++++++------ 4 files changed, 128 insertions(+), 13 deletions(-) create mode 100644 benchmark/vm_ivar_of_class.yml diff --git a/benchmark/vm_ivar_of_class.yml b/benchmark/vm_ivar_of_class.yml new file mode 100644 index 0000000000..172e28b2fd --- /dev/null +++ b/benchmark/vm_ivar_of_class.yml @@ -0,0 +1,12 @@ https://github.com/ruby/ruby/blob/trunk/benchmark/vm_ivar_of_class.yml#L1 +prelude: | + class C + @a = 1 + def self.a + _a = @a; _a = @a; _a = @a; _a = @a; _a = @a; + _a = @a; _a = @a; _a = @a; _a = @a; _a = @a; + end + end +benchmark: + vm_ivar_of_class: | + a = C.a +loop_count: 30000000 diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 4da348df67..ee3f13cad9 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -928,7 +928,7 @@ assert_equal 'ArgumentError', %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_ractor.rb#L928 } # ivar in shareable-objects are not allowed to access from non-main Ractor -assert_equal 'can not access instance variables of classes/modules from non-main Ractors', %q{ +assert_equal "can not get unshareable values from instance variables of classes/modules from non-main Ractors", %q{ class C @iv = 'str' end @@ -1022,6 +1022,53 @@ assert_equal '11', %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_ractor.rb#L1022 }.join } +# and instance variables of classes/modules are accessible if they refer shareable objects +assert_equal '333', %q{ + class C + @int = 1 + @str = '-1000'.dup + @fstr = '100'.freeze + + def self.int = @int + def self.str = @str + def self.fstr = @fstr + end + + module M + @int = 2 + @str = '-2000'.dup + @fstr = '200'.freeze + + def self.int = @int + def self.str = @str + def self.fstr = @fstr + end + + a = Ractor.new{ C.int }.take + b = Ractor.new do + C.str.to_i + rescue Ractor::IsolationError + 10 + end.take + c = Ractor.new do + C.fstr.to_i + end.take + + d = Ractor.new{ M.int }.take + e = Ractor.new do + M.str.to_i + rescue Ractor::IsolationError + 20 + end.take + f = Ractor.new do + M.fstr.to_i + end.take + + + # 1 + 10 + 100 + 2 + 20 + 200 + a + b + c + d + e + f +} + # cvar in shareable-objects are not allowed to access from non-main Ractor assert_equal 'can not access class variables from non-main Ractors', %q{ class C diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index 20db784a39..183b19c3eb 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -2023,14 +2023,14 @@ assert_normal_exit %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_yjit.rb#L2023 foo([1]) rescue nil } -# test ractor exception on when getting ivar +# test ractor exception on when setting ivar assert_equal '42', %q{ class A def self.foo _foo = 1 _bar = 2 begin - @bar + @bar = _foo + _bar rescue Ractor::IsolationError 42 end diff --git a/variable.c b/variable.c index 6cfc3f68f2..87d53cdb90 100644 --- a/variable.c +++ b/variable.c @@ -908,7 +908,7 @@ IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(ID id) https://github.com/ruby/ruby/blob/trunk/variable.c#L908 { if (UNLIKELY(!rb_ractor_main_p())) { if (rb_is_instance_id(id)) { // check only normal ivars - rb_raise(rb_eRactorIsolationError, "can not access instance variables of classes/modules from non-main Ractors"); + rb_raise(rb_eRactorIsolationError, "can not set instance variables of classes/modules by non-main Ractors"); } } } @@ -1185,6 +1185,54 @@ gen_ivtbl_count(const struct gen_ivtbl *ivtbl) https://github.com/ruby/ruby/blob/trunk/variable.c#L1185 return n; } +static int +lock_st_lookup(st_table *tab, st_data_t key, st_data_t *value) +{ + int r; + RB_VM_LOCK_ENTER(); + { + r = st_lookup(tab, key, value); + } + RB_VM_LOCK_LEAVE(); + return r; +} + +static int +lock_st_delete(st_table *tab, st_data_t *key, st_data_t *value) +{ + int r; + RB_VM_LOCK_ENTER(); + { + r = st_delete(tab, key, value); + } + RB_VM_LOCK_LEAVE(); + return r; +} + +static int +lock_st_is_member(st_table *tab, st_data_t key) +{ + int r; + RB_VM_LOCK_ENTER(); + { + r = st_is_member(tab, key); + } + RB_VM_LOCK_LEAVE(); + return r; +} + +static int +lock_st_insert(st_table *tab, st_data_t key, st_data_t value) +{ + int r; + RB_VM_LOCK_ENTER(); + { + r = st_insert(tab, key, value); + } + RB_VM_LOCK_LEAVE(); + return r; +} + VALUE rb_ivar_lookup(VALUE obj, ID id, VALUE undef) { @@ -1211,10 +1259,15 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef) https://github.com/ruby/ruby/blob/trunk/variable.c#L1259 { st_data_t val; - IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id); if (RCLASS_IV_TBL(obj) && - st_lookup(RCLASS_IV_TBL(obj), (st_data_t)id, &val)) { - return (VALUE)val; + lock_st_lookup(RCLASS_IV_TBL(obj), (st_data_t)id, &val)) { + if (rb_is_instance_id(id) && + UNLIKELY(!rb_ractor_main_p()) && + !rb_ractor_shareable_p(val)) { + rb_raise(rb_eRactorIsolationError, + "can not get unshareable values from instance variables of classes/modules from non-main Ractors"); + } + return val; } else { break; @@ -1270,7 +1323,7 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) https://github.com/ruby/ruby/blob/trunk/variable.c#L1323 IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id); if (RCLASS_IV_TBL(obj)) { st_data_t id_data = (st_data_t)id, val; - if (st_delete(RCLASS_IV_TBL(obj), &id_data, &val)) { + if (lock_st_delete(RCLASS_IV_TBL(obj), &id_data, &val)) { return (VALUE)val; } } @@ -1554,8 +1607,7 @@ rb_ivar_defined(VALUE obj, ID id) https://github.com/ruby/ruby/blob/trunk/variable.c#L1607 break; case T_CLASS: case T_MODULE: - IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id); - if (RCLASS_IV_TBL(obj) && st_is_member(RCLASS_IV_TBL(obj), (st_data_t)id)) + if (RCLASS_IV_TBL(obj) && lock_st_is_member(RCLASS_IV_TBL(obj), (st_data_t)id)) return Qtrue; break; default: @@ -1747,7 +1799,11 @@ rb_ivar_foreach(VALUE obj, rb_ivar_foreach_callback_func *func, st_data_t arg) https://github.com/ruby/ruby/blob/trunk/variable.c#L1799 case T_MODULE: IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(0); if (RCLASS_IV_TBL(obj)) { - st_foreach_safe(RCLASS_IV_TBL(obj), func, arg); + RB_VM_LOCK_ENTER(); + { + st_foreach_safe(RCLASS_IV_TBL(obj), func, arg); + } + RB_VM_LOCK_LEAVE(); } break; default: @@ -1909,7 +1965,7 @@ rb_obj_remove_instance_variable(VALUE obj, VALUE name) https://github.com/ruby/ruby/blob/trunk/variable.c#L1965 case T_MODULE: IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id); n = id; - if (RCLASS_IV_TBL(obj) && st_delete(RCLASS_IV_TBL(obj), &n, &v)) { + if (RCLASS_IV_TBL(obj) && lock_st_delete(RCLASS_IV_TBL(obj), &n, &v)) { return (VALUE)v; } break; @@ -3708,7 +3764,7 @@ rb_class_ivar_set(VALUE obj, ID key, VALUE value) https://github.com/ruby/ruby/blob/trunk/variable.c#L3764 } st_table *tbl = RCLASS_IV_TBL(obj); - int result = st_insert(tbl, (st_data_t)key, (st_data_t)value); + int result = lock_st_insert(tbl, (st_data_t)key, (st_data_t)value); RB_OBJ_WRITTEN(obj, Qundef, value); return result; } -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/