From 8aadc6f0f4316d96397ef07876b1c0f9ff7dcf6c Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Mon, 18 Mar 2013 19:21:13 -0700 Subject: Change @env_config to @app_env_config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moral of the story: One must be careful about lazily initializing instance variables when subclassing. I would like to draw your attention to https://github.com/rails/rails/issues/4652 where the reader will see that there appears to be some kind of initialization issue in rails. The source of this issue is that: 1) Engine#env_config contains "@env_config ||= ..." 2) Application#env_config contains "@env_config ||= ..." 3) Threads are in the picture 4) Thread A calls Application#env_config, which super's to Engine#env_config 5) After Engine#env_config returns but before Application#env_config sets @env_config again, Thread B begins running 6) Thread B calls Application#env_config 7) Thread B finds @env_config to contain a value (the one set by Engine#env_config) and returns it 8) Thread B blows up because key set by Application#env_config are there. 9) People report bugs with puma, thin, rainbows, webrick, etc 10) Evan becomes tired of seeing these bugs 11) Evan pours himself a stiff drink, puts on Top Gear(tm), and begins debugging 12) Evan finds the source of the bug 13) Evan authors a PR 14) RIGHT NOW. The bug is fixed by simply using a different ivar name in the methods. Alternately, Engine#env_config could just return a new Hash each time, not memoizing into @env_config. I bid you adieu. --- railties/lib/rails/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 854ac2cbbc..4f695159ea 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -167,7 +167,7 @@ module Rails # These parameters will be used by middlewares and engines to configure themselves. # def env_config - @env_config ||= super.merge({ + @app_env_config ||= super.merge({ "action_dispatch.parameter_filter" => config.filter_parameters, "action_dispatch.secret_token" => config.secret_token, "action_dispatch.show_exceptions" => config.action_dispatch.show_exceptions, -- cgit v1.2.3