inject Rack::Lock if config.eager_load is false

If code is not eager loaded constants are loaded on demand. Constant
autoloading is not thread-safe, so if eager loading is not enabled
multi-threading should not be allowed.

This showed up in certain Capybara scenarios: Most Capybara drivers
other than Rack::Test need a web server. In particular, drivers for
JavaScript support. Capybara launches WEBrick in its own thread for
those but that per se is fine, because the spec thread and the server
thread are coordinated.

Problem comes if the page being served in the spec makes Ajax calls.
Those may hit WEBrick in parallel, and since WEBrick is multi-threaded
and allow_concurrency? returns true in the test environment before
this patch, threads are spawned to serve those parallel requests. On
the other hand, since eager_load is false by default in the test
environment, constants are not preloaded.

So the suite is autoloading constants in a multi-threaded set. That's
a receipt for paracetamol. The symptom is random obscure errors whose
messages point somehow to constant autoloading.

As a consequence of this fix for allow_concurrency? WEBrick in
Capybara scenarios no longer runs in multi-threaded mode.

Fixes #15089.

3 files changed, 20 insertions, 3 deletions

diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md
index 646d84f304..db9dc09e35 100644
--- a/railties/CHANGELOG.md
+++ b/railties/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Inject `Rack::Lock` if `config.eager_load` is false.
+
+    Fixes #15089.
+
+    *Xavier Noria*
+
 *   Change the path of dummy app location in plugin's test_helper.rb for cases
     you specify dummy_path option.
 
diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb
index a00afe008c..d1789192ef 100644
--- a/railties/lib/rails/application/default_middleware_stack.rb
+++ b/railties/lib/rails/application/default_middleware_stack.rb
@@ -66,7 +66,11 @@ module Rails
         end
 
         def allow_concurrency?
-          config.allow_concurrency.nil? ? config.cache_classes : config.allow_concurrency
+          if config.allow_concurrency.nil?
+            config.cache_classes && config.eager_load
+          else
+            config.allow_concurrency
+          end
         end
 
         def load_rack_cache
diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb
index a905598d80..caef39d16f 100644
--- a/railties/test/application/middleware_test.rb
+++ b/railties/test/application/middleware_test.rb
@@ -94,13 +94,20 @@ module ApplicationTests
       assert !middleware.include?("ActiveRecord::Migration::CheckPending")
     end
 
-    test "removes lock if cache classes is set" do
+    test "includes lock if cache_classes is set but eager_load is not" do
       add_to_config "config.cache_classes = true"
       boot!
+      assert middleware.include?("Rack::Lock")
+    end
+
+    test "does not include lock if cache_classes is set and so is eager_load" do
+      add_to_config "config.cache_classes = true"
+      add_to_config "config.eager_load = true"
+      boot!
       assert !middleware.include?("Rack::Lock")
     end
 
-    test "removes lock if allow concurrency is set" do
+    test "does not include lock if allow_concurrency is set" do
       add_to_config "config.allow_concurrency = true"
       boot!
       assert !middleware.include?("Rack::Lock")