Issues

ZF-11524: Fast cache is inadvertently saved with infinite lifetime

Description

The division in Zend_Cache_Backend_TwoLevels::_getFastLifetime() may return 0. For example if $lifetime = 1 and $priority = 8...

{{$fastLifetime = (int) ($lifetime / (11 - $priority));}}

...which results in infinite lifetime for Memcached (http://php.net/memcached.set), APC (http://php.net/apc_store) and XCache.

This is breaks tests which use cache lifetime of 1 and also can cause problems with auto_refresh_fast_cache where 1 can be passed as remaining cache lifetime.

This patch addresses both issues:

Index: library/Zend/Cache/Backend/TwoLevels.php
===================================================================
--- library/Zend/Cache/Backend/TwoLevels.php    (Revision 24185)
+++ library/Zend/Cache/Backend/TwoLevels.php    (Arbeitskopie)
@@ -241,7 +241,7 @@
             $newFastLifetime = $this->_getFastLifetime($array['lifetime'], $array['priority'], time() - $array['expire']);
             // we have the time to refresh the fast cache
             $usage = $this->_getFastFillingPercentage('loading');
-            if (($array['priority'] > 0) && (10 * $array['priority'] >= $usage)) {
+            if (($array['priority'] > 0) && (10 * $array['priority'] >= $usage) && ($newFastLifetime != 0 || $array['lifetime'] == 0)) {
                 // we can refresh the fast cache
                 $preparedData = $this->_prepareData($array['data'], $array['lifetime'], $array['priority']);
                 $this->_fastBackend->save($preparedData, $id, array(), $newFastLifetime);
@@ -481,12 +481,13 @@
      */
     private function _getFastLifetime($lifetime, $priority, $maxLifetime = null)
     {
-        if ($lifetime === null) {
+        if ($lifetime === null || $lifetime == 0) {
             // if lifetime is null, we have an infinite lifetime
             // we need to use arbitrary lifetimes
             $fastLifetime = (int) (2592000 / (11 - $priority));
         } else {
-            $fastLifetime = (int) ($lifetime / (11 - $priority));
+            // prevent computed infinite lifetime (0) by ceil
+            $fastLifetime = ceil($lifetime / (11 - $priority));
         }
         if (($maxLifetime !== null) && ($maxLifetime >= 0)) {
             if ($fastLifetime > $maxLifetime) {

Comments

use noformat to format patch assigned to me

fixed in r24250

I'll merge it to 1.11 branch at the end of this week

merged to 1.11 branch in r24254

This fix has not made it into 1.11.11. Is that intended?

Hm, maybe my comment is wrong. Seems that another error which leads to an infinite lifetime was introduced then:


...
if ($maxLifetime >= 0 && $fastLifetime > $maxLifetime) {
    return $maxLifetime;
}
...

The save() function in TwoLevels cache always calls _getFastLifetime() without 3rd parameter, so it is always null, so if there is a $lifetime > 0 supplied, which is normally the case, it always returns null. Which is an infinite lifetime at least in Memcached.

So this bit which seemed to be there in your patch does work (for me at least):


if (($maxLifetime !== null) && ($maxLifetime >= 0)) {
    if ($fastLifetime > $maxLifetime) {
        return $maxLifetime;
    }
}

Hm, there is another issue that has a strong relation to this one.

I use File as the slow store and Memcached as the fast store. With this setup I can not change the priority that is used in Zend_Cache_Backend_TwoLevels::_getFastLifetime() so I end up with one third of the lifetime in the fast store that the slow store gets and I definitely want to set the same on both!

Here's what's happening:

I'm using "Class" as a frontend and I can set the priority there (even if it would be nice to be able to set it via options). So Zend_Cache_Frontend_Class::setPriority() works.

But:


$this->_backendCapabilities = $this->_backend->getCapabilities();

...
if (($this->_extendedBackend) && ($this->_backendCapabilities['priority'])) {
    $result = $this->_backend->save($data, $id, $tags, $specificLifetime, $priority);
} else {
    $result = $this->_backend->save($data, $id, $tags, $specificLifetime);
}
...

public function getCapabilities()
{
    $slowBackendCapabilities = $this->_slowBackend->getCapabilities();
    return array(
        'automatic_cleaning' => $slowBackendCapabilities['automatic_cleaning'],
        'tags' => $slowBackendCapabilities['tags'],
        'expired_read' => $slowBackendCapabilities['expired_read'],
        'priority' => $slowBackendCapabilities['priority'],
        'infinite_lifetime' => $slowBackendCapabilities['infinite_lifetime'],
        'get_list' => $slowBackendCapabilities['get_list']
    );
}

So it always reads the priority capability from the slow store. Never from the fast store, but in Zend_Cache_Backend_TwoLevels::_getFastLifetime() the lifetime of the fast store IS changed.

So to be able to keep the original lifetime in the fast store currently I'd need a SLOW store with the priority capability, which does not make much sense for me.

A quick workaround would be to set the the default for the priority parameter in Zend_Cache_Backend_TwoLevels::save() to 10 instead of 8, which makes the most sense for me anyway. Why would I want to lower the lifetime to one third in the fast store by default (without beeing able to change this)?

I don't know if there are other bits that would break with this quick fix, maybe something with auto_refresh_fast_cache and load() ...?

At the end of the day a user needs to be able to set the lifetime in both stores to the same value, that's what counts.