This project is archived and is in readonly mode.

#3482 ✓stale
Brian Moran

Activesupport caching can't fetch cached boolean false, always re-calculates

Reported by Brian Moran | November 12th, 2009 @ 12:26 AM

Activesupport cache doesn't correctly fetch cached boolean false values. It caches boolean 'true' correctly. It's caused by the incorrect usage of the cached value as a test for the value's existence. The exist? method should be used instead to determine whether the value has been cached.

The incorrect code:

def fetch(key, options = {}, &block)
if !options[:force] && value = read(key, options)


elsif block_given?

 << execute the block, etc >>

Suggested code:

def fetch(key, options = {}, &block)
if !options[:force] && exist?(key, options)

read(key, options)

elsif block_given?

A patch is supplied with this change, and the test case added for this particular case.

Comments and changes to this ticket

  • Chris Kampmeier

    Chris Kampmeier November 15th, 2009 @ 12:28 AM

    Hey Brian, nice catch. I agree that the current behavior isn't correct.

    I noticed a few problems with the patch, though:

    1. You'd need to add stubs for #exist? to the rest of the test_fetch_* tests. I found that the test suite would hang when I didn't have memcached running, otherwise.
    2. I think there's a subtle race condition with adding #exist?: since you're doing two reads, you could get a cache hit for the #exist? call, but a miss for the #read call (because it could be expired or LRU'd out of the cache just between the two calls). So I think it's important to only make one reading call to the cache, even though the #exist? version is obviously more expressive. Also, it seems like only reading from the cache once for hits would be preferable for performance reasons.

    Here's a new patch that maintains the current code, but explicitly checks for nil to fix the problem with false.

  • Rohit Arondekar

    Rohit Arondekar October 6th, 2010 @ 06:36 AM

    • State changed from “new” to “stale”
    • Importance changed from “” to “”

    Marking ticket as stale. If this is still an issue please leave a comment with suggested changes, creating a patch with tests, rebasing an existing patch or just confirming the issue on a latest release or master/branches.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href=""></a>

Referenced by