-
-
Save xaviershay/8bb23db32d861cbe952f to your computer and use it in GitHub Desktop.
Remove LoadedFeaturesProxy from my require patch.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diff --git a/load.c b/load.c | |
index 776dd4d..c5e10ca 100644 | |
--- a/load.c | |
+++ b/load.c | |
@@ -52,8 +52,6 @@ VALUE rb_get_expanded_load_path(); | |
static VALUE rb_cLoadedFeaturesProxy; | |
static void rb_rehash_loaded_features(); | |
static VALUE rb_loaded_features_hook(int, VALUE*, VALUE); | |
-static void define_loaded_features_proxy(); | |
-static VALUE loaded_features_proxy_new(VALUE); | |
static st_table * get_loading_table(void); | |
static st_table * get_loaded_features_hash(void); | |
@@ -93,6 +91,16 @@ rb_file_is_ruby(VALUE path) | |
return ext && IS_RBEXT(ext); | |
} | |
+/* Attempts to detect whether $LOADED_FEATURES has been manipulated by | |
+ * a process other than `require`, such as by directly adding an entry to | |
+ * it. This is typically done by tests and C-extensions. | |
+ */ | |
+static int | |
+rb_loaded_features_changed() | |
+{ | |
+ return GET_VM()->expected_loaded_features_count != RARRAY_LEN(get_loaded_features()); | |
+} | |
+ | |
static int | |
rb_file_has_been_required(VALUE expanded_path) | |
{ | |
@@ -100,6 +108,11 @@ rb_file_has_been_required(VALUE expanded_path) | |
st_data_t path_key = (st_data_t)RSTRING_PTR(expanded_path); | |
st_table *loaded_features_hash = get_loaded_features_hash(); | |
+ if (rb_loaded_features_changed()) { | |
+ rb_rehash_loaded_features(); | |
+ rb_clear_cached_expansions(); | |
+ } | |
+ | |
return st_lookup(loaded_features_hash, path_key, &data); | |
} | |
@@ -337,49 +350,13 @@ rb_set_cached_expansion(VALUE filename, VALUE expanded) | |
st_insert(filename_expansion_hash, path_key, data); | |
} | |
- | |
-/* | |
- * $LOADED_FEATURES is exposed publically as an array, but under the covers | |
- * we also store this data in a hash for fast lookups. So that we can rebuild | |
- * the hash whenever $LOADED_FEATURES is changed, we wrap the Array class | |
- * in a proxy that intercepts all data-modifying methods and rebuilds the | |
- * hash. | |
- * | |
- * Note that the list of intercepted methods is currently non-comprehensive | |
- * --- it only covers modifications made by the ruby and rubyspec test suites. | |
- */ | |
-static void | |
-define_loaded_features_proxy() | |
-{ | |
- const char* methods_to_hook[] = {"<<", "push", "clear", "replace", "delete"}; | |
- unsigned int i; | |
- | |
- rb_cLoadedFeaturesProxy = rb_define_class("LoadedFeaturesProxy", rb_cArray); | |
- for (i = 0; i < CHAR_ARRAY_LEN(methods_to_hook); ++i) { | |
- rb_define_method( | |
- rb_cLoadedFeaturesProxy, | |
- methods_to_hook[i], | |
- rb_loaded_features_hook, | |
- -1); | |
- } | |
-} | |
- | |
-static VALUE | |
-rb_loaded_features_hook(int argc, VALUE *argv, VALUE self) | |
-{ | |
- VALUE ret; | |
- ret = rb_call_super(argc, argv); | |
- rb_rehash_loaded_features(); | |
- rb_clear_cached_expansions(); | |
- return ret; | |
-} | |
- | |
static void | |
rb_rehash_loaded_features() | |
{ | |
int i; | |
VALUE features; | |
VALUE feature; | |
+ VALUE directory, basename, extension; | |
st_table* loaded_features_hash = get_loaded_features_hash(); | |
@@ -387,12 +364,38 @@ rb_rehash_loaded_features() | |
features = get_loaded_features(); | |
+ GET_VM()->expected_loaded_features_count = RARRAY_LEN(features); | |
for (i = 0; i < RARRAY_LEN(features); ++i) { | |
feature = RARRAY_PTR(features)[i]; | |
- st_insert( | |
- loaded_features_hash, | |
- (st_data_t)ruby_strdup(RSTRING_PTR(feature)), | |
- (st_data_t)rb_barrier_new()); | |
+ if (rb_path_is_absolute(feature)) { | |
+ st_insert( | |
+ loaded_features_hash, | |
+ (st_data_t)ruby_strdup(RSTRING_PTR(feature)), | |
+ (st_data_t)rb_barrier_new()); | |
+ } else { | |
+ st_insert( | |
+ loaded_features_hash, | |
+ (st_data_t)ruby_strdup(RSTRING_PTR(feature)), | |
+ (st_data_t)rb_barrier_new()); | |
+ extension = rb_file_extension(feature); | |
+ | |
+ if (RSTRING_LEN(extension) > 0) { | |
+ directory = rb_file_dirname(feature); | |
+ | |
+ basename = rb_funcall(rb_cFile, rb_intern("basename"), 2, | |
+ feature, extension); | |
+ | |
+ if (RSTRING_PTR(directory)[0] == '.') { | |
+ feature = basename; | |
+ } else { | |
+ feature = rb_funcall(rb_cFile, rb_intern("join"), 2, directory, basename); | |
+ } | |
+ st_insert( | |
+ loaded_features_hash, | |
+ (st_data_t)ruby_strdup(RSTRING_PTR(feature)), | |
+ (st_data_t)rb_barrier_new()); | |
+ } | |
+ } | |
} | |
} | |
@@ -429,21 +432,6 @@ get_filename_expansion_hash(void) | |
return filename_expansion_hash; | |
} | |
-/* This is cargo-culted from ary_alloc in array.c. I am sure there is | |
- * a much better way to instantiate this class, I just don't know what it is. | |
- */ | |
-static VALUE | |
-loaded_features_proxy_new(VALUE klass) | |
-{ | |
- NEWOBJ(ary, struct RArray); | |
- OBJSETUP(ary, klass, T_ARRAY); | |
- FL_SET((ary), RARRAY_EMBED_FLAG); | |
- RBASIC(ary)->flags &= ~RARRAY_EMBED_LEN_MASK; | |
- RBASIC(ary)->flags |= (0) << RARRAY_EMBED_LEN_SHIFT; | |
- | |
- return (VALUE)ary; | |
-} | |
- | |
static const char *const loadable_ext[] = { | |
".rb", DLEXT, | |
#ifdef DLEXT2 | |
@@ -1030,6 +1018,10 @@ rb_require_safe(VALUE fname, int safe) | |
rb_set_safe_level_force(safe); | |
FilePathValue(fname); | |
rb_set_safe_level_force(0); | |
+ if (rb_file_has_been_required(fname)) { | |
+ /* Checks of features provided manually (such as by C-extensions) */ | |
+ result = Qfalse; | |
+ } else { | |
path = rb_locate_file(fname); | |
if (safe >= 1 && OBJ_TAINTED(path)) { | |
@@ -1055,10 +1047,12 @@ rb_require_safe(VALUE fname, int safe) | |
rb_ary_push(ruby_dln_librefs, LONG2NUM(handle)); | |
} | |
rb_provide_feature(path); | |
+ GET_VM()->expected_loaded_features_count++; | |
result = Qtrue; | |
} | |
} | |
} | |
+ } | |
} | |
POP_TAG(); | |
load_unlock(ftptr, !state); | |
@@ -1212,9 +1206,8 @@ Init_load() | |
rb_define_virtual_variable("$\"", get_loaded_features, 0); | |
rb_define_virtual_variable("$LOADED_FEATURES", get_loaded_features, 0); | |
- define_loaded_features_proxy(); | |
- | |
- vm->loaded_features = loaded_features_proxy_new(rb_cLoadedFeaturesProxy); | |
+ vm->loaded_features = rb_ary_new(); | |
+ vm->expected_loaded_features_count = 0; | |
rb_define_global_function("load", rb_f_load, -1); | |
rb_define_global_function("require", rb_f_require, 1); | |
diff --git a/test/ruby/test_require.rb b/test/ruby/test_require.rb | |
index 49a7952..40b4549 100644 | |
--- a/test/ruby/test_require.rb | |
+++ b/test/ruby/test_require.rb | |
@@ -413,4 +413,36 @@ class TestRequire < Test::Unit::TestCase | |
$:.replace(load_path) | |
$".replace(loaded) | |
end | |
+ | |
+ def test_load_c_provided_feature | |
+ assert !require("enumerator") | |
+ end | |
+ | |
+ def test_load_manually_provided_rb | |
+ # This technique is used in ext/extmk.rb for reasons unknown | |
+ loaded = $".dup | |
+ $".clear | |
+ $" << 'mkmf.rb' | |
+ %w( | |
+ mkmf.rb | |
+ mkmf | |
+ ).each do |file| | |
+ assert !require("#{file}"), "Require of #{file} expected to return false" | |
+ end | |
+ ensure | |
+ $".replace(loaded) | |
+ end | |
+ | |
+ def test_load_manually_provided_extension | |
+ loaded = $".dup | |
+ $" << 'c_feature.so' | |
+ %w( | |
+ c_feature.so | |
+ c_feature | |
+ ).each do |file| | |
+ assert !require("#{file}"), "Require of #{file} expected to return false" | |
+ end | |
+ ensure | |
+ $".replace(loaded) | |
+ end | |
end | |
diff --git a/vm_core.h b/vm_core.h | |
index 7818ec0..388934a 100644 | |
--- a/vm_core.h | |
+++ b/vm_core.h | |
@@ -324,8 +324,12 @@ typedef struct rb_vm_struct { | |
* objects so do *NOT* mark this when you GC. | |
*/ | |
struct RArray at_exit; | |
+ | |
+ /* The following three variables are used in load.c as part | |
+ * of requiring files */ | |
struct st_table *loaded_features_hash; | |
struct st_table *filename_expansion_hash; | |
+ long expected_loaded_features_count; | |
} rb_vm_t; | |
typedef struct { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment