diff options
| -rw-r--r-- | ChangeLog | 53 | ||||
| -rw-r--r-- | file.c | 24 | ||||
| -rw-r--r-- | internal.h | 3 | ||||
| -rw-r--r-- | load.c | 97 | ||||
| -rw-r--r-- | test/ruby/test_require.rb | 110 | ||||
| -rw-r--r-- | vm.c | 1 | ||||
| -rw-r--r-- | vm_core.h | 1 |
7 files changed, 276 insertions, 13 deletions
@@ -1,3 +1,56 @@ +Mon Nov 5 23:28:57 2012 Hiroshi Shirosaki <h.shirosaki@gmail.com> + + * file.c (rb_get_path_check_to_string): extract from + rb_get_path_check(). We change the spec not to call to_path of + String object. + + * file.c (rb_get_path_check_convert): extract from rb_get_path_check(). + + * file.c (rb_get_path_check): follow the above change. + + * file.c (rb_file_expand_path_fast): remove check_expand_path_args(). + Instead we call it in load.c. + + * file.c (rb_find_file_ext_safe): use rb_get_expanded_load_path() to + reduce expand cost. + + * file.c (rb_find_file_safe): ditto. + + * internal.h (rb_get_expanded_load_path): add a declaration. + + * internal.h (rb_get_path_check_to_string, rb_get_path_check_convert): + add declarations. + + * load.c (rb_construct_expanded_load_path): fix for compatibility. + Same checks in rb_get_path_check() are added. We don't replace + $LOAD_PATH and ensure that String object of $LOAD_PATH are frozen. + We don't freeze non String object and expand it every times. We add + arguments for expanding load path partially and checking if load path + have relative paths or non String objects. + + * load.c (load_path_getcwd): get current working directory for checking + if it's changed when getting load path. + + * load.c (rb_get_expanded_load_path): fix for rebuilding cache properly. + We check if current working directory is changed and rebuild expanded + load path cache. We expand paths which start with ~ (User HOME) and + non String objects every times for compatibility. We make this + accessible from other source files. + + * load.c (rb_feature_provided): call rb_get_path() since we changed + rb_file_expand_path_fast() not to call it. + + * load.c (Init_load): initialize vm->load_path_check_cache. + + * vm.c (rb_vm_mark): mark vm->load_path_check_cache for GC. + + * vm_core.h (rb_vm_struct): add vm->load_path_check_cache to store data + to check load path cache validity. + + * test/ruby/test_require.rb (TestRequire): add tests for require + compatibility related to cached expanded load path. + [ruby-core:47970] [Bug #7158] + Mon Nov 5 23:26:05 2012 Greg Price <price@mit.edu> * load.c (rb_get_expanded_load_path): cache the expanded load @@ -167,8 +167,8 @@ check_path_encoding(VALUE str) return enc; } -static VALUE -rb_get_path_check(VALUE obj, int level) +VALUE +rb_get_path_check_to_string(VALUE obj, int level) { VALUE tmp; ID to_path; @@ -177,13 +177,21 @@ rb_get_path_check(VALUE obj, int level) rb_insecure_operation(); } + if (RB_TYPE_P(obj, T_STRING)) { + return obj; + } CONST_ID(to_path, "to_path"); tmp = rb_check_funcall(obj, to_path, 0, 0); if (tmp == Qundef) { tmp = obj; } StringValue(tmp); + return tmp; +} +VALUE +rb_get_path_check_convert(VALUE obj, VALUE tmp, int level) +{ tmp = file_path_convert(tmp); if (obj != tmp && insecure_obj_p(tmp, level)) { rb_insecure_operation(); @@ -195,6 +203,13 @@ rb_get_path_check(VALUE obj, int level) return rb_str_new4(tmp); } +static VALUE +rb_get_path_check(VALUE obj, int level) +{ + VALUE tmp = rb_get_path_check_to_string(obj, level); + return rb_get_path_check_convert(obj, tmp, level); +} + VALUE rb_get_path_no_checksafe(VALUE obj) { @@ -3255,7 +3270,6 @@ rb_file_expand_path(VALUE fname, VALUE dname) VALUE rb_file_expand_path_fast(VALUE fname, VALUE dname) { - check_expand_path_args(fname, dname); return rb_file_expand_path_internal(fname, dname, 0, 0, EXPAND_PATH_BUFFER()); } @@ -5258,7 +5272,7 @@ rb_find_file_ext_safe(VALUE *filep, const char *const *ext, int safe_level) rb_raise(rb_eSecurityError, "loading from non-absolute path %s", f); } - RB_GC_GUARD(load_path) = rb_get_load_path(); + RB_GC_GUARD(load_path) = rb_get_expanded_load_path(); if (!load_path) return 0; fname = rb_str_dup(*filep); @@ -5323,7 +5337,7 @@ rb_find_file_safe(VALUE path, int safe_level) rb_raise(rb_eSecurityError, "loading from non-absolute path %s", f); } - RB_GC_GUARD(load_path) = rb_get_load_path(); + RB_GC_GUARD(load_path) = rb_get_expanded_load_path(); if (load_path) { long i; diff --git a/internal.h b/internal.h index e538de4404..6e6a071f92 100644 --- a/internal.h +++ b/internal.h @@ -108,6 +108,8 @@ void rb_file_const(const char*, VALUE); int rb_file_load_ok(const char *); VALUE rb_file_expand_path_fast(VALUE, VALUE); VALUE rb_file_expand_path_internal(VALUE, VALUE, int, int, VALUE); +VALUE rb_get_path_check_to_string(VALUE, int); +VALUE rb_get_path_check_convert(VALUE, VALUE, int); void Init_File(void); #ifdef _WIN32 @@ -133,6 +135,7 @@ VALUE rb_iseq_clone(VALUE iseqval, VALUE newcbase); /* load.c */ VALUE rb_get_load_path(void); +VALUE rb_get_expanded_load_path(void); NORETURN(void rb_load_fail(VALUE, const char*)); /* math.c */ @@ -33,22 +33,58 @@ rb_get_load_path(void) return load_path; } +enum expand_type { + EXPAND_ALL, + EXPAND_RELATIVE, + EXPAND_HOME, + EXPAND_NON_CACHE +}; + +/* Construct expanded load path and store it to cache. + We rebuild load path partially if the cache is invalid. + We don't cache non string object and expand it every times. We ensure that + string objects in $LOAD_PATH are frozen. + */ static void -rb_construct_expanded_load_path(void) +rb_construct_expanded_load_path(int type, int *has_relative, int *has_non_cache) { rb_vm_t *vm = GET_VM(); VALUE load_path = vm->load_path; + VALUE expanded_load_path = vm->expanded_load_path; VALUE ary; long i; + int level = rb_safe_level(); ary = rb_ary_new2(RARRAY_LEN(load_path)); for (i = 0; i < RARRAY_LEN(load_path); ++i) { VALUE path, as_str, expanded_path; + int is_string, non_cache; + char *as_cstr; as_str = path = RARRAY_PTR(load_path)[i]; - StringValue(as_str); - if (as_str != path) - rb_ary_store(load_path, i, as_str); - rb_str_freeze(as_str); + is_string = RB_TYPE_P(path, T_STRING) ? 1 : 0; + non_cache = !is_string ? 1 : 0; + as_str = rb_get_path_check_to_string(path, level); + as_cstr = RSTRING_PTR(as_str); + + if (!non_cache) { + if ((type == EXPAND_RELATIVE && + rb_is_absolute_path(as_cstr)) || + (type == EXPAND_HOME && + (!as_cstr[0] || as_cstr[0] != '~')) || + (type == EXPAND_NON_CACHE)) { + /* Use cached expanded path. */ + rb_ary_push(ary, RARRAY_PTR(expanded_load_path)[i]); + continue; + } + } + if (!*has_relative && !rb_is_absolute_path(as_cstr)) + *has_relative = 1; + if (!*has_non_cache && non_cache) + *has_non_cache = 1; + /* Freeze only string object. We expand other objects every times. */ + if (is_string) + rb_str_freeze(path); + as_str = rb_get_path_check_convert(path, as_str, level); expanded_path = rb_file_expand_path_fast(as_str, Qnil); rb_str_freeze(expanded_path); rb_ary_push(ary, expanded_path); @@ -59,12 +95,56 @@ rb_construct_expanded_load_path(void) } static VALUE +load_path_getcwd(void) +{ + char *cwd = my_getcwd(); + VALUE cwd_str = rb_filesystem_str_new_cstr(cwd); + xfree(cwd); + return cwd_str; +} + +VALUE rb_get_expanded_load_path(void) { rb_vm_t *vm = GET_VM(); + const VALUE non_cache = Qtrue; + if (!rb_ary_shared_with_p(vm->load_path_snapshot, vm->load_path)) { - /* The load path was modified. Rebuild the expanded load path. */ - rb_construct_expanded_load_path(); + /* The load path was modified. Rebuild the expanded load path. */ + int has_relative = 0, has_non_cache = 0; + rb_construct_expanded_load_path(EXPAND_ALL, &has_relative, &has_non_cache); + if (has_relative) { + vm->load_path_check_cache = load_path_getcwd(); + } + else if (has_non_cache) { + /* Non string object. */ + vm->load_path_check_cache = non_cache; + } + else { + vm->load_path_check_cache = 0; + } + } + else if (vm->load_path_check_cache == non_cache) { + int has_relative = 1, has_non_cache = 1; + /* Expand only non-cacheable objects. */ + rb_construct_expanded_load_path(EXPAND_NON_CACHE, + &has_relative, &has_non_cache); + } + else if (vm->load_path_check_cache) { + int has_relative = 1, has_non_cache = 1; + VALUE cwd = load_path_getcwd(); + if (!rb_str_equal(vm->load_path_check_cache, cwd)) { + /* Current working directory or filesystem encoding was changed. + Expand relative load path and non-cacheable objects again. */ + vm->load_path_check_cache = cwd; + rb_construct_expanded_load_path(EXPAND_RELATIVE, + &has_relative, &has_non_cache); + } + else { + /* Expand only tilde (User HOME) and non-cacheable objects. */ + rb_construct_expanded_load_path(EXPAND_HOME, + &has_relative, &has_non_cache); + } } return vm->expanded_load_path; } @@ -392,7 +472,7 @@ rb_feature_provided(const char *feature, const char **loading) if (*feature == '.' && (feature[1] == '/' || strncmp(feature+1, "./", 2) == 0)) { - fullpath = rb_file_expand_path_fast(rb_str_new2(feature), Qnil); + fullpath = rb_file_expand_path_fast(rb_get_path(rb_str_new2(feature)), Qnil); feature = RSTRING_PTR(fullpath); } if (ext && !strchr(ext, '/')) { @@ -963,6 +1043,7 @@ Init_load() vm->load_path = rb_ary_new(); vm->expanded_load_path = rb_ary_new(); vm->load_path_snapshot = rb_ary_new(); + vm->load_path_check_cache = 0; rb_define_virtual_variable("$\"", get_loaded_features, 0); rb_define_virtual_variable("$LOADED_FEATURES", get_loaded_features, 0); diff --git a/test/ruby/test_require.rb b/test/ruby/test_require.rb index a7c75d06bf..dbbe483578 100644 --- a/test/ruby/test_require.rb +++ b/test/ruby/test_require.rb @@ -435,4 +435,114 @@ class TestRequire < Test::Unit::TestCase $:.replace(loadpath) $".replace(features) end + + def test_require_changed_current_dir + bug7158 = '[ruby-core:47970]' + Dir.mktmpdir {|tmp| + Dir.chdir(tmp) { + Dir.mkdir("a") + Dir.mkdir("b") + open(File.join("a", "foo.rb"), "w") {} + open(File.join("b", "bar.rb"), "w") {|f| + f.puts "p :ok" + } + assert_in_out_err([], <<-INPUT, %w(:ok), [], bug7158) + $: << "." + Dir.chdir("a") + require "foo" + Dir.chdir("../b") + p :ng unless require "bar" + Dir.chdir("..") + p :ng if require "b/bar" + INPUT + } + } + end + + def test_require_not_modified_load_path + bug7158 = '[ruby-core:47970]' + Dir.mktmpdir {|tmp| + Dir.chdir(tmp) { + open("foo.rb", "w") {} + assert_in_out_err([], <<-INPUT, %w(:ok), [], bug7158) + a = Object.new + def a.to_str + "#{tmp}" + end + $: << a + require "foo" + last_path = $:.pop + p :ok if last_path == a && last_path.class == Object + INPUT + } + } + end + + def test_require_changed_home + bug7158 = '[ruby-core:47970]' + Dir.mktmpdir {|tmp| + Dir.chdir(tmp) { + open("foo.rb", "w") {} + Dir.mkdir("a") + open(File.join("a", "bar.rb"), "w") {} + assert_in_out_err([], <<-INPUT, %w(:ok), [], bug7158) + $: << '~' + ENV['HOME'] = "#{tmp}" + require "foo" + ENV['HOME'] = "#{tmp}/a" + p :ok if require "bar" + INPUT + } + } + end + + def test_require_to_path_redefined_in_load_path + bug7158 = '[ruby-core:47970]' + Dir.mktmpdir {|tmp| + Dir.chdir(tmp) { + open("foo.rb", "w") {} + assert_in_out_err([], <<-INPUT, %w(:ok), [], bug7158) + a = Object.new + def a.to_path + "bar" + end + $: << a + begin + require "foo" + p :ng + rescue LoadError + end + def a.to_path + "#{tmp}" + end + p :ok if require "foo" + INPUT + } + } + end + + def test_require_to_str_redefined_in_load_path + bug7158 = '[ruby-core:47970]' + Dir.mktmpdir {|tmp| + Dir.chdir(tmp) { + open("foo.rb", "w") {} + assert_in_out_err([], <<-INPUT, %w(:ok), [], bug7158) + a = Object.new + def a.to_str + "foo" + end + $: << a + begin + require "foo" + p :ng + rescue LoadError + end + def a.to_str + "#{tmp}" + end + p :ok if require "foo" + INPUT + } + } + end end @@ -1513,6 +1513,7 @@ rb_vm_mark(void *ptr) RUBY_MARK_UNLESS_NULL(vm->mark_object_ary); RUBY_MARK_UNLESS_NULL(vm->load_path); RUBY_MARK_UNLESS_NULL(vm->load_path_snapshot); + RUBY_MARK_UNLESS_NULL(vm->load_path_check_cache); RUBY_MARK_UNLESS_NULL(vm->expanded_load_path); RUBY_MARK_UNLESS_NULL(vm->loaded_features); RUBY_MARK_UNLESS_NULL(vm->loaded_features_snapshot); @@ -355,6 +355,7 @@ typedef struct rb_vm_struct { VALUE top_self; VALUE load_path; VALUE load_path_snapshot; + VALUE load_path_check_cache; VALUE expanded_load_path; VALUE loaded_features; VALUE loaded_features_snapshot; |