#3482 Activesupport caching can't fetch cached boolean false, always re-calculates - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#3482 ✓stale

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)

value

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.

allow-rails-caching-boolean-false.diff 1.9 KB

Comments and changes to this ticket

You flagged this item as spam.
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.
- dont-interpret-false-as-cache-miss.patch 1.6 KB
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.