Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Rails.cache.fetch does not work with false boolean as cached value

#1830

If I run


some_bool_value = Rails.cache.fetch("some_key") {expensive_function_returns_false()}

Rails will miss on the fetch every time. However if I do


if Rails.cache.exist?("some_key")
  some_bool_value = Rails.cache.read("some_key")
else
  some_bool_value = Rails.cache.write("some_key", expensive_function_returns_false())
end

The cache will miss the first time and than hit every time after that. I was looking at the code in rails / activesupport / lib / active_support / cache.rb and noticed on line 141 there is the following if statement:


if !options[:force] && value = read(key, options)

value is set to false in the if statement, causing the condition to fail and miss. It would be better if value was read after the if statement and the if statement would be changed to


if !options[:force] && exist?(key)

Reported by Thijs de Vries · January 31st, 2009 @ 10:19 PM

State: stale
Milestone: 3.x
Assigned to: nobody
Importance: none

Activity

  1. Thijs de Vries
    Thijs de Vries
    • Tag set to patch

    Here's a patch that should do it.

    January 31st, 2009 @ 10:32 PM

  2. Thijs de Vries
    Thijs de Vries

    That last patch I sent caused some of the tests to fail. This one won't fail any tests and allows for the new ones to pass.

    January 31st, 2009 @ 11:34 PM

  3. DHH
    DHH

    Won't this lead to a race condition? You have two calls to the separate cache to check for the same key. The key could have come and gone in between those two calls.

    February 5th, 2009 @ 08:01 PM

  4. Derek McLoughlin
    Derek McLoughlin

    Would it be better to have the following:

        if !options[:force] && ((value = read(key, options)) != nil)

    March 12th, 2009 @ 03:01 PM

  5. Thijs de Vries
    Thijs de Vries
    • Tag cleared.

    @@@ruby if !options[:force] && ((value = read(key, options)) != nil)

    
won't let you store nil as a value. In case a really expensive function returns nil, one may still wanna store nil so they don't have to run the really expensive function again. Probably have to work with thread locks to make this work.

    March 12th, 2009 @ 03:10 PM

  6. Derek McLoughlin
    Derek McLoughlin

    True, but nil is currently being used as meaning "the thing isn't in the cache". The above solution works for cached values of 'false'.

    March 13th, 2009 @ 04:17 PM

  7. MatthewRudy
    MatthewRudy
    • Assigned user set to José Valim
    • Tag set to activesupport, cache, false, fetch

    hey,
    what happened to this?

    The "false" case of this bit me and a colleague again this week.
    Spent ages wondering why using memcache was even slower than hitting our DB.

    So here is a patch, just to deal with the "false" case.

    But I feel that there should be a more clever way to deal with a nil store.

    Anyway,
    here is my patch.

    http://github.com/matthewrudy/rude-rails/commit/37d50e8e4d4eec27e06...

    November 28th, 2009 @ 06:09 PM

  8. José Valim
    José Valim
    • Assigned user cleared.

    November 28th, 2009 @ 10:17 PM

  9. MatthewRudy
    MatthewRudy

    living the dream!

    November 29th, 2009 @ 02:05 AM

  10. MatthewRudy
    MatthewRudy

    my patch still works
    http://github.com/matthewrudy/rude-rails/commit/6cb522c735d8fa9895b...

    and I upped the patch
    but now its a bit weird
    (I think because of the rebase)

    March 2nd, 2010 @ 03:59 AM

  11. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.x to 3.x

    May 4th, 2010 @ 06:48 PM

  12. W. Andrew Loe III
    W. Andrew Loe III

    Chris Kampmeier in #3482 Activesupport caching can't fetch cached boolean false, always re-calculates appears to have come up with a patch that does not introduce a race condition.

    June 16th, 2010 @ 09:16 PM

  13. Santiago Pastorino
    Santiago Pastorino
    • State changed from new to open
    • Importance changed from to

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

    February 2nd, 2011 @ 04:37 PM

  14. Santiago Pastorino
    Santiago Pastorino
    • State changed from open to stale

    February 2nd, 2011 @ 04:37 PM