ruby-changes:51183
From: normal <ko1@a...>
Date: Thu, 10 May 2018 14:10:17 +0900 (JST)
Subject: [ruby-changes:51183] normal:r63390 (trunk): revert r63387 and r63389 for now
normal 2018-05-10 14:10:13 +0900 (Thu, 10 May 2018) New Revision: 63390 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=63390 Log: revert r63387 and r63389 for now I can't reproduce the problem on my 32-bit machine, and I have connectivity problems to my 64-bit systems at the moment. Will revisit in a few hours hopefully. Modified files: trunk/test/ruby/test_autoload.rb trunk/variable.c Index: test/ruby/test_autoload.rb =================================================================== --- test/ruby/test_autoload.rb (revision 63389) +++ test/ruby/test_autoload.rb (revision 63390) @@ -311,42 +311,6 @@ p Foo::Bar https://github.com/ruby/ruby/blob/trunk/test/ruby/test_autoload.rb#L311 end end if Process.respond_to?(:fork) - def test_autoload_same_file - Dir.mktmpdir('autoload') do |tmpdir| - File.write("#{tmpdir}/b.rb", "#{<<~'begin;'}\n#{<<~'end;'}") - begin; - module Foo; end - module Bar; end - end; - 3.times do # timing-dependent, needs a few times to hit [Bug #14742] - assert_separately(%W[-I #{tmpdir}], "#{<<-'begin;'}\n#{<<-'end;'}") - begin; - autoload :Foo, 'b' - autoload :Bar, 'b' - t1 = Thread.new do Foo end - t2 = Thread.new do Bar end - t1.join - t2.join - bug = '[ruby-core:86935] [Bug #14742]' - assert_instance_of Module, t1.value, bug - assert_instance_of Module, t2.value, bug - end; - end - end - end - - def test_no_leak - assert_no_memory_leak([], '', <<~'end;', 'many autoloads', timeout: 30) - 200000.times do |i| - m = Module.new - m.instance_eval do - autoload :Foo, 'x' - autoload :Bar, i.to_s - end - end - end; - end - def add_autoload(path) (@autoload_paths ||= []) << path ::Object.class_eval {autoload(:AutoloadTest, path)} Index: variable.c =================================================================== --- variable.c (revision 63389) +++ variable.c (revision 63390) @@ -25,7 +25,6 @@ https://github.com/ruby/ruby/blob/trunk/variable.c#L25 static struct rb_id_table *rb_global_tbl; static ID autoload, classpath, tmp_classpath, classid; -static VALUE autoload_featuremap; /* feature => autoload_i */ static void check_before_mod_set(VALUE, ID, VALUE, const char *); static void setup_const_entry(rb_const_entry_t *, VALUE, VALUE, rb_const_flag_t); @@ -1843,42 +1842,31 @@ autoload_data(VALUE mod, ID id) https://github.com/ruby/ruby/blob/trunk/variable.c#L1842 return (VALUE)val; } -struct autoload_const { - struct list_node cnode; /* <=> autoload_data_i.constants */ - VALUE mod; - VALUE ad; /* autoload_data_i */ - VALUE value; - ID id; - int safe_level; - rb_const_flag_t flag; -}; - /* always on stack, no need to mark */ struct autoload_state { - struct autoload_const *ac; + struct autoload_data_i *ele; + VALUE mod; VALUE result; + ID id; VALUE thread; struct list_node waitq; }; struct autoload_data_i { VALUE feature; + int safe_level; + rb_const_flag_t flag; + VALUE value; struct autoload_state *state; /* points to on-stack struct */ rb_serial_t fork_gen; - struct list_head constants; /* <=> autoload_const.cnode */ }; static void autoload_i_mark(void *ptr) { struct autoload_data_i *p = ptr; - rb_gc_mark(p->feature); - - /* allow GC to free us if no modules refer to this via autoload_const.ad */ - if (list_empty(&p->constants)) { - rb_hash_delete(autoload_featuremap, p->feature); - } + rb_gc_mark(p->value); } static size_t @@ -1893,49 +1881,16 @@ static const rb_data_type_t autoload_dat https://github.com/ruby/ruby/blob/trunk/variable.c#L1881 0, 0, RUBY_TYPED_FREE_IMMEDIATELY }; -static void -autoload_c_mark(void *ptr) -{ - struct autoload_const *ac = ptr; - - rb_gc_mark(ac->mod); - rb_gc_mark(ac->ad); - rb_gc_mark(ac->value); -} - -static void -autoload_c_free(void *ptr) -{ - struct autoload_const *ac = ptr; - list_del(&ac->cnode); - xfree(ac); -} - -static size_t -autoload_c_memsize(const void *ptr) -{ - return sizeof(struct autoload_const); -} - -static const rb_data_type_t autoload_const_type = { - "autoload_const", - {autoload_c_mark, autoload_c_free, autoload_c_memsize,}, - 0, 0, RUBY_TYPED_FREE_IMMEDIATELY -}; - static struct autoload_data_i * -get_autoload_data(VALUE acv, struct autoload_const **acp) +get_autoload_data(VALUE av) { - struct autoload_const *ac = rb_check_typeddata(acv, &autoload_const_type); - struct autoload_data_i *ele; + struct autoload_data_i *ele = rb_check_typeddata(av, &autoload_data_i_type); - ele = rb_check_typeddata(ac->ad, &autoload_data_i_type); /* do not reach across stack for ->state after forking: */ if (ele && ele->state && ele->fork_gen != GET_VM()->fork_gen) { ele->state = 0; ele->fork_gen = 0; } - if (acp) *acp = ac; return ele; } @@ -1985,42 +1940,17 @@ rb_autoload_str(VALUE mod, ID id, VALUE https://github.com/ruby/ruby/blob/trunk/variable.c#L1940 DATA_PTR(av) = tbl = st_init_numtable(); } + ad = TypedData_Make_Struct(0, struct autoload_data_i, &autoload_data_i_type, ele); if (OBJ_TAINTED(file)) { file = rb_str_dup(file); FL_UNSET(file, FL_TAINT); } - file = rb_fstring(file); - if (!autoload_featuremap) { - autoload_featuremap = rb_hash_new_compare_by_id(); - rb_obj_hide(autoload_featuremap); - rb_gc_register_mark_object(autoload_featuremap); - } - ad = rb_hash_aref(autoload_featuremap, file); - if (NIL_P(ad)) { - ad = TypedData_Make_Struct(0, struct autoload_data_i, - &autoload_data_i_type, ele); - ele->feature = file; - ele->state = 0; - list_head_init(&ele->constants); - rb_hash_aset(autoload_featuremap, file, ad); - } - else { - ele = rb_check_typeddata(ad, &autoload_data_i_type); - } - { - VALUE acv; - struct autoload_const *ac; - acv = TypedData_Make_Struct(0, struct autoload_const, - &autoload_const_type, ac); - ac->mod = mod; - ac->id = id; - ac->value = Qundef; - ac->safe_level = rb_safe_level(); - ac->flag = CONST_PUBLIC; - ac->ad = ad; - list_add_tail(&ele->constants, &ac->cnode); - st_insert(tbl, (st_data_t)id, (st_data_t)acv); - } + ele->feature = rb_fstring(file); + ele->safe_level = rb_safe_level(); + ele->value = Qundef; + ele->state = 0; + ele->flag = CONST_PUBLIC; + st_insert(tbl, (st_data_t)id, (st_data_t)ad); } static void @@ -2030,19 +1960,8 @@ autoload_delete(VALUE mod, ID id) https://github.com/ruby/ruby/blob/trunk/variable.c#L1960 if (st_lookup(RCLASS_IV_TBL(mod), (st_data_t)autoload, &val)) { struct st_table *tbl = check_autoload_table((VALUE)val); - struct autoload_data_i *ele; - struct autoload_const *ac; st_delete(tbl, &n, &load); - ele = get_autoload_data((VALUE)load, &ac); - VM_ASSERT(!list_empty(&ele->constants)); - - /* - * we must delete here to avoid "already initialized" warnings - * with parallel autoload. list_del_init makes list_del in - * autoload_c_free idempotent - */ - list_del_init(&ac->cnode); if (tbl->num_entries == 0) { n = autoload; @@ -2068,13 +1987,12 @@ reset_safe(VALUE safe) https://github.com/ruby/ruby/blob/trunk/variable.c#L1987 static VALUE check_autoload_required(VALUE mod, ID id, const char **loadingpath) { - VALUE file; - VALUE load = autoload_data(mod, id); + VALUE file, load; struct autoload_data_i *ele; const char *loading; int safe; - if (!load || !(ele = get_autoload_data(load, 0))) { + if (!(load = autoload_data(mod, id)) || !(ele = get_autoload_data(load))) { return 0; } file = ele->feature; @@ -2109,21 +2027,19 @@ check_autoload_required(VALUE mod, ID id https://github.com/ruby/ruby/blob/trunk/variable.c#L2027 MJIT_FUNC_EXPORTED int rb_autoloading_value(VALUE mod, ID id, VALUE* value, rb_const_flag_t *flag) { - VALUE load = autoload_data(mod, id); + VALUE load; struct autoload_data_i *ele; - struct autoload_const *ac; - if (!load || !(ele = get_autoload_data(load, &ac))) { - return 0; + if (!(load = autoload_data(mod, id)) || !(ele = get_autoload_data(load))) { + return 0; } - if (ele->state && ele->state->thread == rb_thread_current()) { - if (ac->value != Qundef) { + if (ele->value != Qundef) { if (value) { - *value = ac->value; + *value = ele->value; } if (flag) { - *flag = ac->flag; + *flag = ele->flag; } return 1; } @@ -2142,16 +2058,23 @@ autoload_defined_p(VALUE mod, ID id) https://github.com/ruby/ruby/blob/trunk/variable.c#L2058 return !rb_autoloading_value(mod, id, NULL, NULL); } -static void const_tbl_update(struct autoload_const *); +struct autoload_const_set_args { + VALUE mod; + ID id; + VALUE value; + rb_const_flag_t flag; +}; + +static void const_tbl_update(struct autoload_const_set_args *); static VALUE autoload_const_set(VALUE arg) { - struct autoload_const *ac = (struct autoload_const *)arg; - VALUE klass = ac->mod; - ID id = ac->id; - check_before_mod_set(klass, id, ac->value, "constant"); - const_tbl_update(ac); + struct autoload_const_set_args* args = (struct autoload_const_set_args *)arg; + VALUE klass = args->mod; + ID id = args->id; + check_before_mod_set(klass, id, args->value, "constant"); + const_tbl_update(args); return 0; /* ignored */ } @@ -2159,13 +2082,10 @@ static VALUE https://github.com/ruby/ruby/blob/trunk/variable.c#L2082 autoload_require(VALUE arg) { struct autoload_state *state = (struct autoload_state *)arg; - struct autoload_const *ac = state->ac; - struct autoload_data_i *ele; - ele = rb_check_typeddata(ac->ad, &autoload_data_i_type); /* this may release GVL and switch threads: */ state->result = rb_funcall(rb_vm_top_self(), rb_intern("require"), 1, - ele->feature); + state->ele->feature); return state->result; } @@ -2175,27 +2095,26 @@ autoload_reset(VALUE arg) https://github.com/ruby/ruby/blob/trunk/variable.c#L2095 { struct autoload_state *state = (struct autoload_state *)arg; int need_wakeups = 0; - struct autoload_const *ac = state->ac; - struct autoload_data_i *ele; - ele = rb_check_typeddata(ac->ad, &autoload_data_i_type); - if (ele->state == state) { + if (state->ele->state == state) { need_wakeups = 1; - ele->state = 0; - ele->fork_gen = 0; + state->ele->state = 0; + state->ele->fork_gen = 0; } /* At the last, move a value defined in autoload to constant table */ - if (RTEST(state->result)) { - struct autoload_const *next; - int safe_backup = rb_safe_level(); - - list_for_each_safe(&ele->constants, ac, next, cnode) { - if (ac->value != Qundef) { - rb_ensure(autoload_const_set, (VALUE)ac, - reset_safe, (VALUE)safe_backup); - } - } + if (RTEST(state->result) && state->ele->value != Qundef) { + int safe_backup; + struct autoload_const_set_args args; + + args.mod = state->mod; + args.id = state->id; + args.value = state->ele->value; + args.flag = state->ele->flag; + safe_backup = rb_safe_level(); + rb_set_safe_level_force(state->ele->safe_level); + rb_ensure(autoload_const_set, (VALUE)&args, + reset_safe, (VALUE)safe_backup); } /* wakeup any waiters we had */ @@ -2253,7 +2172,6 @@ rb_autoload_load(VALUE mod, ID id) https://github.com/ruby/ruby/blob/trunk/variable.c#L2172 VALUE load, result; const char *loading = 0, *src; struct autoload_data_i *ele; - struct autoload_const *ac; struct autoload_state state; if (!autoload_defined_p(mod, id)) return Qfalse; @@ -2263,10 +2181,13 @@ rb_autoload_load(VALUE mod, ID id) https://github.com/ruby/ruby/blob/trunk/variable.c#L2181 if (src && loading && strcmp(src, loading) == 0) return Qfalse; /* set ele->state for a marker of autoloading thread */ - if (!(ele = get_autoload_data(load, &ac))) { + if (!(ele = get_autoload_data(load))) { return Qfalse; } - state.ac = ac; + + state.ele = ele; + state.mod = mod; + state.id = id; state.thread = rb_thread_current(); if (!ele->state) { ele->state = &state; @@ -2308,7 +2229,7 @@ rb_autoload_p(VALUE mod, ID id) https://github.com/ruby/ruby/blob/trunk/variable.c#L2229 } load = check_autoload_required(mod, id, 0); if (!load) return Qnil; - return (ele = get_autoload_data(load, 0)) ? ele->feature : Qnil; + return (ele = get_autoload_data(load)) ? ele->feature : Qnil; } MJIT_FUNC_EXPORTED void @@ -2689,12 +2610,12 @@ rb_const_set(VALUE klass, ID id, VALUE v https://github.com/ruby/ruby/blob/trunk/variable.c#L2610 setup_const_entry(ce, klass, val, CONST_PUBLIC); } else { - struct autoload_const ac; - ac.mod = klass; - ac.id = id; - ac.value = val; - ac.flag = CONST_PUBLIC; - const_tbl_update(&ac); + struct autoload_const_set_args args; + args.mod = klass; + args.id = id; + args.value = val; + args.flag = CONST_PUBLIC; + const_tbl_update(&args); } /* * Resolve and cache class name immediately to resolve ambiguity @@ -2726,12 +2647,12 @@ rb_const_set(VALUE klass, ID id, VALUE v https://github.com/ruby/ruby/blob/trunk/variable.c#L2647 } static struct autoload_data_i * -current_autoload_data(VALUE mod, ID id, struct autoload_const **acp) +current_autoload_data(VALUE mod, ID id) { struct autoload_data_i *ele; VALUE load = autoload_data(mod, id); if (!load) return 0; - ele = get_autoload_data(load, acp); + ele = get_autoload_data(load); if (!ele) return 0; /* for autoloading thread, keep the defined value to autoloading storage */ if (ele->state && (ele->state->thread == rb_thread_current())) { @@ -2741,25 +2662,25 @@ current_autoload_data(VALUE mod, ID id, https://github.com/ruby/ruby/blob/trunk/variable.c#L2662 } static void -const_tbl_update(struct autoload_const *ac) +const_tbl_update(struct autoload_const_set_args *args) { VALUE value; - VALUE klass = ac->mod; - VALUE val = ac->value; - ID id = ac->id; + VALUE klass = args->mod; + VALUE val = args->value; + ID id = args->id; struct rb_id_table *tbl = RCLASS_CONST_TBL(klass); - rb_const_flag_t visibility = ac->flag; + rb_const_flag_t visibility = args->flag; rb_const_entry_t *ce; if (rb_id_table_lookup(tbl, id, &value)) { ce = (rb_const_entry_t *)value; if (ce->value == Qundef) { - struct autoload_data_i *ele = current_autoload_data(klass, id, &ac); + struct autoload_data_i *ele = current_autoload_data(klass, id); if (ele) { rb_clear_constant_cache(); - ac->value = val; /* autoload_i is non-WB-protected */ + ele->value = val; /* autoload_i is non-WB-protected */ return; } /* otherwise, allow to override */ @@ -2832,7 +2753,6 @@ set_const_visibility(VALUE mod, int argc https://github.com/ruby/ruby/blob/trunk/variable.c#L2753 } for (i = 0; i < argc; i++) { - struct autoload_const *ac; VALUE val = argv[i]; id = rb_check_id(&val); if (!id) { @@ -2847,12 +2767,10 @@ set_const_visibility(VALUE mod, int argc https://github.com/ruby/ruby/blob/trunk/variable.c#L2767 ce->flag &= ~mask; ce->flag |= flag; if (ce->value == Qundef) { - struct autoload_data_i *ele; - - ele = current_autoload_data(mod, id, &ac); + struct autoload_data_i *ele = current_autoload_data(mod, id); if (ele) { - ac->flag &= ~mask; - ac->flag |= flag; + ele->flag &= ~mask; + ele->flag |= flag; } } } -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/