Bug in recursive partial "Stack Level Too Deep" fix #12521

Closed
wyaeld opened this Issue Oct 14, 2013 · 2 comments

Comments

Projects
None yet
2 participants
Contributor

wyaeld commented Oct 14, 2013

The changes made in 09f6fe1 to fix #11340 are an incomplete fix. Subsequent cache reads will use an incorrect key that includes the "false" value used as a temporary placeholder.

Please see comments and logs added to 09f6fe1 where the code is discussed

wyaeld referenced this issue Oct 14, 2013

Fix "Stack Level Too Deep" error when rendering recursive partials
When rendering recursive partial Action View is trying to generate the
view digest infinitly causing a stack level error.

Fixes #11340
Contributor

thedarkone commented Oct 14, 2013

This was introduced in #11768 (my fault, I wasn't careful enough with my code review).

What is happening is the new code doesn't necessarily clean-up the stop-the-recursion false value if ActionView::Resolver.caching? is disabled.

@wyaeld do you think you could write a test and a fix? What you need to do is to make the end of compute_and_store_digest look like it was before while incorporating the ActionView::Resolver.caching? check.

You'd replace this:

  # Store the actual digest if config.cache_template_loading is true
  klass.new(name, format, finder, options).digest.tap do |digest|
    @@cache[cache_key] = digest if ActionView::Resolver.caching?
  end
rescue Exception
  @@cache.delete_pair(cache_key, false) if pre_stored # something went wrong, make sure not to corrupt the @@cache
  raise

with this;

  digest = klass.new(name, format, finder, options).digest
  # Store the actual digest if config.cache_template_loading is true
  @@cache[cache_key] = digest if ActionView::Resolver.caching?
  digest
ensure
  # something went wrong or ActionView::Resolver.caching? is false, make sure not to corrupt the @@cache
  @@cache.delete_pair(cache_key, false) if pre_stored && !digest 
Contributor

wyaeld commented Oct 14, 2013

I'll give it a try at work tomorrow and report back

wyaeld added a commit to wyaeld/rails that referenced this issue Oct 14, 2013

Add 2 tests, 1 of which fails, to isolate the digest_caching behaviou…
…r causing #12521

If config.action_view.cache_template_loading = false, most likely in a development
configuration if config.cache_classes = false &
config.action_controller.perform_caching = true.

config.action_view.cache_template_loading defaults to the value of config.cache_classes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment