Skip to content
This repository
Browse code

Change @env_config to @app_env_config

Moral of the story: One must be careful about lazily initializing
instance variables when subclassing.

I would like to draw your attention to #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.
  • Loading branch information...
commit 8aadc6f0f4316d96397ef07876b1c0f9ff7dcf6c 1 parent ef5faeb
Evan Phoenix authored

Showing 1 changed file with 1 addition and 1 deletion. Show diff stats Hide diff stats

  1. 2  railties/lib/rails/application.rb
2  railties/lib/rails/application.rb
@@ -167,7 +167,7 @@ def load_console(app=self)
167 167
     # These parameters will be used by middlewares and engines to configure themselves.
168 168
     #
169 169
     def env_config
170  
-      @env_config ||= super.merge({
  170
+      @app_env_config ||= super.merge({
171 171
         "action_dispatch.parameter_filter" => config.filter_parameters,
172 172
         "action_dispatch.secret_token" => config.secret_token,
173 173
         "action_dispatch.show_exceptions" => config.action_dispatch.show_exceptions,

0 notes on commit 8aadc6f

Please sign in to comment.
Something went wrong with that request. Please try again.