mirror of
https://github.com/ruby/ruby.git
synced 2022-11-09 12:17:21 -05:00
* variable.c: Make autoload thread-safe. See #921.
What's the problem?
autoload is thread unsafe. When we define a constant to be
autoloaded, we expect the constant construction is invariant. But
current autoload implementation allows other threads to access the
constant while the first thread is loading a file.
What's happening inside?
The current implementation uses Qundef as a marker of autoload in
Constant table. Once the first thread find Qundef as a value at
constant lookup, it starts loading a defined feature. Generally a
loaded file overrides the Qundef in Constant table by module/class
declaration at very beginning lines of the file, so other threads
can see the new Module/Class object before feature loading is
finished. It breaks invariant construction.
How to solve?
To ensure invariant constant construction, we need to override
Qundef with defined Object after the feature loading. For keeping
Qundef in Constant table, I expanded autoload_data struct in
Module to have a slot for keeping the defined object while feature
loading. And changed Module's constant lookup/update logic a
little so that the slot is only visible from the thread which
invokes feature loading. (== the first thread which accessed the
autoload constant)
Evaluation?
All test passes (bootstrap test, test-all and RubySpec) and added
8 tests for threading behavior. Extra logics are executed only
when Qundef is found, so no perf drop should happen except
autoloading.
* variable.c (rb_autoload): Prepare new autoload_data struct.
* variable.c (rb_autoload_load): Load feature and update Constant
table after feature loading is finished.
* variable.c (rb_const_get_0): When the fetched constant is under
autoloading, it returns the object only for the thread which starts
autoloading.
* variable.c (rb_const_defined_0): Ditto.
* variable.c (rb_const_set): When the specified constant is under
autoloading, it sets the object only for the thread which starts
autoloading. Otherwise, simply overrides Qundef with constant
override warning.
* vm_insnhelper.c (vm_get_ev_const): Apply same change as
rb_const_get_0 in variable.c.
* test/ruby/test_autoload.rb: Added tests for threading behavior.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@33078 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
This commit is contained in:
nahi
parent
e93d882d96
commit
1e7f99dddf
4 changed files with 331 additions and 32 deletions
56
ChangeLog
56
ChangeLog
|
|
@ -1,3 +1,59 @@
|
|||
Fri Aug 26 19:12:08 2011 Hiroshi Nakamura <nahi@ruby-lang.org>
|
||||
|
||||
* variable.c: Make autoload thread-safe. See #921.
|
||||
|
||||
What's the problem?
|
||||
autoload is thread unsafe. When we define a constant to be
|
||||
autoloaded, we expect the constant construction is invariant. But
|
||||
current autoload implementation allows other threads to access the
|
||||
constant while the first thread is loading a file.
|
||||
|
||||
What's happening inside?
|
||||
The current implementation uses Qundef as a marker of autoload in
|
||||
Constant table. Once the first thread find Qundef as a value at
|
||||
constant lookup, it starts loading a defined feature. Generally a
|
||||
loaded file overrides the Qundef in Constant table by module/class
|
||||
declaration at very beginning lines of the file, so other threads
|
||||
can see the new Module/Class object before feature loading is
|
||||
finished. It breaks invariant construction.
|
||||
|
||||
How to solve?
|
||||
To ensure invariant constant construction, we need to override
|
||||
Qundef with defined Object after the feature loading. For keeping
|
||||
Qundef in Constant table, I expanded autoload_data struct in
|
||||
Module to have a slot for keeping the defined object while feature
|
||||
loading. And changed Module's constant lookup/update logic a
|
||||
little so that the slot is only visible from the thread which
|
||||
invokes feature loading. (== the first thread which accessed the
|
||||
autoload constant)
|
||||
|
||||
Evaluation?
|
||||
All test passes (bootstrap test, test-all and RubySpec) and added
|
||||
8 tests for threading behavior. Extra logics are executed only
|
||||
when Qundef is found, so no perf drop should happen except
|
||||
autoloading.
|
||||
|
||||
* variable.c (rb_autoload): Prepare new autoload_data struct.
|
||||
|
||||
* variable.c (rb_autoload_load): Load feature and update Constant
|
||||
table after feature loading is finished.
|
||||
|
||||
* variable.c (rb_const_get_0): When the fetched constant is under
|
||||
autoloading, it returns the object only for the thread which starts
|
||||
autoloading.
|
||||
|
||||
* variable.c (rb_const_defined_0): Ditto.
|
||||
|
||||
* variable.c (rb_const_set): When the specified constant is under
|
||||
autoloading, it sets the object only for the thread which starts
|
||||
autoloading. Otherwise, simply overrides Qundef with constant
|
||||
override warning.
|
||||
|
||||
* vm_insnhelper.c (vm_get_ev_const): Apply same change as
|
||||
rb_const_get_0 in variable.c.
|
||||
|
||||
* test/ruby/test_autoload.rb: Added tests for threading behavior.
|
||||
|
||||
Fri Aug 26 10:10:37 2011 Eric Hodel <drbrain@segment7.net>
|
||||
|
||||
* lib/rubygems: Update to RubyGems 1.8.10. Fixes security issue in
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
require 'test/unit'
|
||||
require 'tempfile'
|
||||
require 'thread'
|
||||
require_relative 'envutil'
|
||||
|
||||
class TestAutoload < Test::Unit::TestCase
|
||||
|
|
@ -53,4 +55,112 @@ p Foo::Bar
|
|||
assert_equal(tmpfile, b.autoload?(:X), bug4565)
|
||||
}
|
||||
end
|
||||
|
||||
def test_require_explicit
|
||||
file = Tempfile.open(['autoload', '.rb'])
|
||||
file.puts 'class Object; AutoloadTest = 1; end'
|
||||
file.close
|
||||
add_autoload(file.path)
|
||||
begin
|
||||
assert_nothing_raised do
|
||||
assert(require file.path)
|
||||
assert_equal(1, ::AutoloadTest)
|
||||
end
|
||||
ensure
|
||||
remove_autoload_constant
|
||||
end
|
||||
end
|
||||
|
||||
def test_threaded_accessing_constant
|
||||
file = Tempfile.open(['autoload', '.rb'])
|
||||
file.puts 'sleep 0.5; class AutoloadTest; X = 1; end'
|
||||
file.close
|
||||
add_autoload(file.path)
|
||||
begin
|
||||
assert_nothing_raised do
|
||||
t1 = Thread.new { ::AutoloadTest::X }
|
||||
t2 = Thread.new { ::AutoloadTest::X }
|
||||
[t1, t2].each(&:join)
|
||||
end
|
||||
ensure
|
||||
remove_autoload_constant
|
||||
end
|
||||
end
|
||||
|
||||
def test_threaded_accessing_inner_constant
|
||||
file = Tempfile.open(['autoload', '.rb'])
|
||||
file.puts 'class AutoloadTest; sleep 0.5; X = 1; end'
|
||||
file.close
|
||||
add_autoload(file.path)
|
||||
begin
|
||||
assert_nothing_raised do
|
||||
t1 = Thread.new { ::AutoloadTest::X }
|
||||
t2 = Thread.new { ::AutoloadTest::X }
|
||||
[t1, t2].each(&:join)
|
||||
end
|
||||
ensure
|
||||
remove_autoload_constant
|
||||
end
|
||||
end
|
||||
|
||||
def test_nameerror_when_autoload_did_not_define_the_constant
|
||||
file = Tempfile.open(['autoload', '.rb'])
|
||||
file.puts ''
|
||||
file.close
|
||||
add_autoload(file.path)
|
||||
begin
|
||||
assert_raise(NameError) do
|
||||
AutoloadTest
|
||||
end
|
||||
ensure
|
||||
remove_autoload_constant
|
||||
end
|
||||
end
|
||||
|
||||
def test_override_autoload
|
||||
file = Tempfile.open(['autoload', '.rb'])
|
||||
file.puts ''
|
||||
file.close
|
||||
add_autoload(file.path)
|
||||
begin
|
||||
eval %q(class AutoloadTest; end)
|
||||
assert_equal(Class, AutoloadTest.class)
|
||||
ensure
|
||||
remove_autoload_constant
|
||||
end
|
||||
end
|
||||
|
||||
def test_override_while_autoloading
|
||||
file = Tempfile.open(['autoload', '.rb'])
|
||||
file.puts 'class AutoloadTest; sleep 0.5; end'
|
||||
file.close
|
||||
add_autoload(file.path)
|
||||
begin
|
||||
# while autoloading...
|
||||
t = Thread.new { AutoloadTest }
|
||||
sleep 0.1
|
||||
# override it
|
||||
eval %q(AutoloadTest = 1)
|
||||
t.join
|
||||
assert_equal(1, AutoloadTest)
|
||||
ensure
|
||||
remove_autoload_constant
|
||||
end
|
||||
end
|
||||
|
||||
def add_autoload(path)
|
||||
eval <<-END
|
||||
class ::Object
|
||||
autoload :AutoloadTest, #{path.dump}
|
||||
end
|
||||
END
|
||||
end
|
||||
|
||||
def remove_autoload_constant
|
||||
eval <<-END
|
||||
class ::Object
|
||||
remove_const(:AutoloadTest)
|
||||
end
|
||||
END
|
||||
end
|
||||
end
|
||||
|
|
|
|||
194
variable.c
194
variable.c
|
|
@ -1445,12 +1445,63 @@ static const rb_data_type_t autoload_data_type = {
|
|||
#define check_autoload_table(av) \
|
||||
(struct st_table *)rb_check_typeddata((av), &autoload_data_type)
|
||||
|
||||
static VALUE
|
||||
autoload_data(VALUE mod, ID id)
|
||||
{
|
||||
struct st_table *tbl;
|
||||
st_data_t val;
|
||||
|
||||
if (!st_lookup(RCLASS_IV_TBL(mod), autoload, &val) ||
|
||||
!(tbl = check_autoload_table((VALUE)val)) || !st_lookup(tbl, (st_data_t)id, &val)) {
|
||||
return 0;
|
||||
}
|
||||
return (VALUE)val;
|
||||
}
|
||||
|
||||
struct autoload_data_i {
|
||||
VALUE feature;
|
||||
int safe_level;
|
||||
VALUE thread;
|
||||
VALUE value;
|
||||
};
|
||||
|
||||
static void
|
||||
autoload_i_mark(void *ptr)
|
||||
{
|
||||
struct autoload_data_i *p = ptr;
|
||||
rb_gc_mark(p->feature);
|
||||
rb_gc_mark(p->thread);
|
||||
rb_gc_mark(p->value);
|
||||
}
|
||||
|
||||
static void
|
||||
autoload_i_free(void *ptr)
|
||||
{
|
||||
struct autoload_data_i *p = ptr;
|
||||
xfree(p);
|
||||
}
|
||||
|
||||
static size_t
|
||||
autoload_i_memsize(const void *ptr)
|
||||
{
|
||||
return sizeof(struct autoload_data_i);
|
||||
}
|
||||
|
||||
static const rb_data_type_t autoload_data_i_type = {
|
||||
"autoload_i",
|
||||
{autoload_i_mark, autoload_i_free, autoload_i_memsize,},
|
||||
};
|
||||
|
||||
#define check_autoload_data(av) \
|
||||
(struct autoload_data_i *)rb_check_typeddata((av), &autoload_data_i_type)
|
||||
|
||||
void
|
||||
rb_autoload(VALUE mod, ID id, const char *file)
|
||||
{
|
||||
st_data_t av;
|
||||
VALUE fn;
|
||||
VALUE ad, fn;
|
||||
struct st_table *tbl;
|
||||
struct autoload_data_i *ele;
|
||||
|
||||
if (!rb_is_const_id(id)) {
|
||||
rb_raise(rb_eNameError, "autoload must be constant name: %s", rb_id2name(id));
|
||||
|
|
@ -1473,13 +1524,20 @@ rb_autoload(VALUE mod, ID id, const char *file)
|
|||
st_add_direct(tbl, (st_data_t)autoload, av);
|
||||
DATA_PTR(av) = tbl = st_init_numtable();
|
||||
}
|
||||
ad = TypedData_Wrap_Struct(0, &autoload_data_i_type, 0);
|
||||
st_insert(tbl, (st_data_t)id, (st_data_t)ad);
|
||||
DATA_PTR(ad) = ele = ALLOC(struct autoload_data_i);
|
||||
|
||||
fn = rb_str_new2(file);
|
||||
FL_UNSET(fn, FL_TAINT);
|
||||
OBJ_FREEZE(fn);
|
||||
st_insert(tbl, (st_data_t)id, (st_data_t)rb_node_newnode(NODE_MEMO, fn, rb_safe_level(), 0));
|
||||
ele->feature = fn;
|
||||
ele->safe_level = rb_safe_level();
|
||||
ele->thread = Qnil;
|
||||
ele->value = Qundef;
|
||||
}
|
||||
|
||||
static NODE*
|
||||
static void
|
||||
autoload_delete(VALUE mod, ID id)
|
||||
{
|
||||
st_data_t val, load = 0, n = id;
|
||||
|
|
@ -1498,8 +1556,6 @@ autoload_delete(VALUE mod, ID id)
|
|||
st_delete(RCLASS_IV_TBL(mod), &n, &val);
|
||||
}
|
||||
}
|
||||
|
||||
return (NODE *)load;
|
||||
}
|
||||
|
||||
static VALUE
|
||||
|
|
@ -1516,22 +1572,18 @@ reset_safe(VALUE safe)
|
|||
return safe;
|
||||
}
|
||||
|
||||
static NODE *
|
||||
autoload_node(VALUE mod, ID id, const char **loadingpath)
|
||||
static VALUE
|
||||
check_autoload_required(VALUE mod, ID id, const char **loadingpath)
|
||||
{
|
||||
VALUE file;
|
||||
struct st_table *tbl;
|
||||
st_data_t val;
|
||||
NODE *load;
|
||||
VALUE file, load;
|
||||
struct autoload_data_i *ele;
|
||||
const char *loading;
|
||||
int safe;
|
||||
|
||||
if (!st_lookup(RCLASS_IV_TBL(mod), autoload, &val) ||
|
||||
!(tbl = check_autoload_table((VALUE)val)) || !st_lookup(tbl, (st_data_t)id, &val)) {
|
||||
if (!(load = autoload_data(mod, id)) || !(ele = check_autoload_data(load))) {
|
||||
return 0;
|
||||
}
|
||||
load = (NODE *)val;
|
||||
file = load->nd_lit;
|
||||
file = ele->feature;
|
||||
Check_Type(file, T_STRING);
|
||||
if (!RSTRING_PTR(file) || !*RSTRING_PTR(file)) {
|
||||
rb_raise(rb_eArgError, "empty file name");
|
||||
|
|
@ -1550,7 +1602,7 @@ autoload_node(VALUE mod, ID id, const char **loadingpath)
|
|||
}
|
||||
|
||||
static int
|
||||
autoload_node_id(VALUE mod, ID id)
|
||||
autoload_defined_p(VALUE mod, ID id)
|
||||
{
|
||||
struct st_table *tbl = RCLASS_CONST_TBL(mod);
|
||||
st_data_t val;
|
||||
|
|
@ -1561,42 +1613,109 @@ autoload_node_id(VALUE mod, ID id)
|
|||
return 1;
|
||||
}
|
||||
|
||||
int
|
||||
rb_autoloading_value(VALUE mod, ID id, VALUE* value)
|
||||
{
|
||||
VALUE load;
|
||||
struct autoload_data_i *ele;
|
||||
|
||||
if (!(load = autoload_data(mod, id)) || !(ele = check_autoload_data(load))) {
|
||||
return 0;
|
||||
}
|
||||
if (ele->thread == rb_thread_current()) {
|
||||
if (ele->value != Qundef) {
|
||||
if (value) {
|
||||
*value = ele->value;
|
||||
}
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
struct autoload_const_set_args {
|
||||
VALUE mod;
|
||||
ID id;
|
||||
VALUE value;
|
||||
};
|
||||
|
||||
static void
|
||||
autoload_const_set(struct autoload_const_set_args* args)
|
||||
{
|
||||
autoload_delete(args->mod, args->id);
|
||||
rb_const_set(args->mod, args->id, args->value);
|
||||
}
|
||||
|
||||
static VALUE
|
||||
autoload_require(struct autoload_data_i *ele)
|
||||
{
|
||||
return rb_require_safe(ele->feature, ele->safe_level);
|
||||
}
|
||||
|
||||
VALUE
|
||||
rb_autoload_load(VALUE mod, ID id)
|
||||
{
|
||||
VALUE file;
|
||||
NODE *load;
|
||||
VALUE load, result;
|
||||
const char *loading = 0, *src;
|
||||
struct autoload_data_i *ele;
|
||||
int state = 0;
|
||||
|
||||
if (!autoload_node_id(mod, id)) return Qfalse;
|
||||
load = autoload_node(mod, id, &loading);
|
||||
if (!autoload_defined_p(mod, id)) return Qfalse;
|
||||
load = check_autoload_required(mod, id, &loading);
|
||||
if (!load) return Qfalse;
|
||||
src = rb_sourcefile();
|
||||
if (src && loading && strcmp(src, loading) == 0) return Qfalse;
|
||||
file = load->nd_lit;
|
||||
return rb_require_safe(file, (int)load->nd_nth);
|
||||
|
||||
/* set ele->thread for a marker of autoloading thread */
|
||||
if (!(ele = check_autoload_data(load))) {
|
||||
return Qfalse;
|
||||
}
|
||||
if (ele->thread == Qnil) {
|
||||
ele->thread = rb_thread_current();
|
||||
}
|
||||
/* autoload_data_i can be deleted by another thread while require */
|
||||
RB_GC_GUARD(load);
|
||||
result = rb_protect((VALUE(*)(VALUE))autoload_require, (VALUE)ele, &state);
|
||||
if (ele->thread == rb_thread_current()) {
|
||||
ele->thread = Qnil;
|
||||
}
|
||||
if (state) rb_jump_tag(state);
|
||||
|
||||
if (RTEST(result)) {
|
||||
/* At the last, move a value defined in autoload to constant table */
|
||||
if (ele->value != Qundef) {
|
||||
int safe_backup;
|
||||
struct autoload_const_set_args args;
|
||||
args.mod = mod;
|
||||
args.id = id;
|
||||
args.value = ele->value;
|
||||
safe_backup = rb_safe_level();
|
||||
rb_set_safe_level_force(ele->safe_level);
|
||||
rb_ensure((VALUE(*)(VALUE))autoload_const_set, (VALUE)&args, reset_safe, (VALUE)safe_backup);
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
VALUE
|
||||
rb_autoload_p(VALUE mod, ID id)
|
||||
{
|
||||
VALUE file;
|
||||
NODE *load;
|
||||
const char *loading = 0;
|
||||
VALUE load;
|
||||
struct autoload_data_i *ele;
|
||||
|
||||
while (!autoload_node_id(mod, id)) {
|
||||
while (!autoload_defined_p(mod, id)) {
|
||||
mod = RCLASS_SUPER(mod);
|
||||
if (!mod) return Qnil;
|
||||
}
|
||||
load = autoload_node(mod, id, &loading);
|
||||
load = check_autoload_required(mod, id, 0);
|
||||
if (!load) return Qnil;
|
||||
return load && (file = load->nd_lit) ? file : Qnil;
|
||||
return (ele = check_autoload_data(load)) ? ele->feature : Qnil;
|
||||
}
|
||||
|
||||
static VALUE
|
||||
rb_const_get_0(VALUE klass, ID id, int exclude, int recurse, int visibility)
|
||||
{
|
||||
VALUE value, tmp;
|
||||
VALUE value, tmp, av;
|
||||
int mod_retry = 0;
|
||||
|
||||
tmp = klass;
|
||||
|
|
@ -1613,6 +1732,7 @@ rb_const_get_0(VALUE klass, ID id, int exclude, int recurse, int visibility)
|
|||
if (value == Qundef) {
|
||||
if (am == tmp) break;
|
||||
am = tmp;
|
||||
if (rb_autoloading_value(tmp, id, &av)) return av;
|
||||
rb_autoload_load(tmp, id);
|
||||
continue;
|
||||
}
|
||||
|
|
@ -1843,7 +1963,7 @@ rb_const_defined_0(VALUE klass, ID id, int exclude, int recurse, int visibility)
|
|||
if (visibility && ce->flag == CONST_PRIVATE) {
|
||||
return (int)Qfalse;
|
||||
}
|
||||
if (ce->value == Qundef && !autoload_node(tmp, id, 0))
|
||||
if (ce->value == Qundef && !check_autoload_required(tmp, id, 0) && !rb_autoloading_value(tmp, id, 0))
|
||||
return (int)Qfalse;
|
||||
return (int)Qtrue;
|
||||
}
|
||||
|
|
@ -1922,8 +2042,20 @@ rb_const_set(VALUE klass, ID id, VALUE val)
|
|||
|
||||
if (st_lookup(RCLASS_CONST_TBL(klass), (st_data_t)id, &value)) {
|
||||
rb_const_entry_t *ce = (rb_const_entry_t*)value;
|
||||
if (ce->value == Qundef)
|
||||
if (ce->value == Qundef) {
|
||||
VALUE load;
|
||||
struct autoload_data_i *ele;
|
||||
|
||||
load = autoload_data(klass, id);
|
||||
/* for autoloading thread, keep the defined value to autoloading storage */
|
||||
if (load && (ele = check_autoload_data(load)) && (ele->thread == rb_thread_current())) {
|
||||
rb_vm_change_state();
|
||||
ele->value = val;
|
||||
return;
|
||||
}
|
||||
/* otherwise, allow to override */
|
||||
autoload_delete(klass, id);
|
||||
}
|
||||
else {
|
||||
visibility = ce->flag;
|
||||
rb_warn("already initialized constant %s", rb_id2name(id));
|
||||
|
|
|
|||
|
|
@ -1178,7 +1178,7 @@ vm_get_ev_const(rb_thread_t *th, const rb_iseq_t *iseq,
|
|||
cref = cref->nd_next;
|
||||
|
||||
if (!NIL_P(klass)) {
|
||||
VALUE am = 0;
|
||||
VALUE av, am = 0;
|
||||
st_data_t data;
|
||||
search_continue:
|
||||
if (RCLASS_CONST_TBL(klass) &&
|
||||
|
|
@ -1188,6 +1188,7 @@ vm_get_ev_const(rb_thread_t *th, const rb_iseq_t *iseq,
|
|||
if (am == klass) break;
|
||||
am = klass;
|
||||
if (is_defined) return 1;
|
||||
if (rb_autoloading_value(klass, id, &av)) return av;
|
||||
rb_autoload_load(klass, id);
|
||||
goto search_continue;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue