From e1ed7a4717768bc5795f75e384e6f842417d3616 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 26 Jan 2018 08:59:33 -0500 Subject: [PATCH 1/2] Add failing test for infinite loop when unloading autoloaded modules when an error occured during the load. --- activesupport/test/dependencies_test.rb | 55 +++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index d636da46d2..513ce181d3 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -185,6 +185,61 @@ def test_require_dependency_interaction_with_autoloading end end + # Regression see https://github.com/rails/rails/issues/31694 + def test_included_constant_that_changes_to_have_exception_then_back_does_not_loop_forever + # This constant references a nested constant whose namespace will be auto-generated + parent_constant = <<-RUBY + class ConstantReloadError + AnotherConstant::ReloadError + end + RUBY + + # This constant's namespace will be auto-generated, + # also, we'll edit it to contain an error at load-time + child_constant = <<-RUBY + class AnotherConstant::ReloadError + # no_such_method_as_this + end + RUBY + + # Create a version which contains an error during loading + child_constant_with_error = child_constant.sub("# no_such_method_as_this", "no_such_method_as_this") + + fixtures_path = File.join(__dir__, "autoloading_fixtures") + Dir.mktmpdir(nil, fixtures_path) do |tmpdir| + # Set up the file structure where constants will be loaded from + child_constant_path = "#{tmpdir}/another_constant/reload_error.rb" + File.write("#{tmpdir}/constant_reload_error.rb", parent_constant) + Dir.mkdir("#{tmpdir}/another_constant") + File.write(child_constant_path, child_constant_with_error) + + tmpdir_name = tmpdir.split("/").last + with_loading("autoloading_fixtures/#{tmpdir_name}") do + # Load the file, with the error: + assert_raises(NameError) { + ConstantReloadError + } + + Timeout.timeout(0.1) do + # Remove the constant, as if Rails development middleware is reloading changed files: + ActiveSupport::Dependencies.remove_unloadable_constants! + refute defined?(AnotherConstant::ReloadError) + end + + # Change the file, so that it is **correct** this time: + File.write(child_constant_path, child_constant) + + # Again: Remove the constant, as if Rails development middleware is reloading changed files: + ActiveSupport::Dependencies.remove_unloadable_constants! + refute defined?(AnotherConstant::ReloadError) + + # Now, reload the _fixed_ constant: + assert ConstantReloadError + assert AnotherConstant::ReloadError + end + end + end + def test_module_loading with_autoloading_fixtures do assert_kind_of Module, A From d92fb27885ddcb0a92ac67f69bf0eb8c912f4dc7 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 26 Jan 2018 09:41:27 -0500 Subject: [PATCH 2/2] Remove duplicates after autoloading modules --- activesupport/lib/active_support/dependencies.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index abc648e0c6..0f59558bb5 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -447,6 +447,7 @@ def autoload_module!(into, const_name, qualified_name, path_suffix) mod = Module.new into.const_set const_name, mod autoloaded_constants << qualified_name unless autoload_once_paths.include?(base_path) + autoloaded_constants.uniq! mod end