Notice unserialize error in CakePHP via Redis

Question

Update:

Per suggestion from the accepted answer below, I tested reading a negative number:

$negativeInt = -1;
Cache::write('my-test-count', $negativeInt, 'short');
$readVal = Cache::read('my-test-count', 'short');
debug($readVal);
exit;

The unseralize error reproduces consistently when trying to read any negative number. It's now an accepted bug, which I assume will be resolved by 2.8.1

---

Original Question:

I keep getting this error, but can't figure out why or even how to further troubleshoot.

The line that throws the error should only get hit if Cache::read() returns false. But, that line would throw an unserialize error itself if I didn't put the @ before it.

Question: How can I reliably use Redis for counts without getting unserialize Notices on a regular basis? If the data in the key is "bad", how can I know that without getting the Notice just from doing a ::read. I've tried making sure my data is (int) (see below), but that doesn't seem to help.

Notice (8): unserialize(): Error at offset 0 of 2 bytes [APP/Vendor/pear-pear.cakephp.org/CakePHP/Cake/Cache/Engine/RedisEngine.php, line 136]

Upon inspection of the error:

> unserialize - [internal], line ??
> RedisEngine::read() - APP/Vendor/pear-pear.cakephp.org/CakePHP/Cake/Cache/Engine/RedisEngine.php, line 136
> Cache::read() - APP/Vendor/pear-pear.cakephp.org/CakePHP/Cake/Cache/Cache.php, line 358
> Cache::remember() - APP/Vendor/pear-pear.cakephp.org/CakePHP/Cake/Cache/Cache.php, line 567
> Item::getCount() - APP/Model/Item.php, line 812

It appears to be coming from this function:

public function getCount($id) {
    $model = $this;

    // here is where the Cache::read() and debug are in the update below

    return Cache::remember('item' . $id. '_count', function() use ($model, $id) {
        $count = $model->find('count', array(
            'conditions' => array(
                $model->alias . '.status' => 1,
                $model->alias . '.id' => $id
            )
        ));
        return ($count === false) ? 0 : (int)$count;
    }, 'my_counts'); // THIS IS LINE 812
}

public function decrementCount($id, $offset = 1) {
    if(empty($id)) return false;
    $count = @Cache::read('item' . $id . '_count', 'my_counts');
    if($count === false) {
        $this->getCount($id);
    } else {
        Cache::decrement('item' . $id . '_count', $offset, 'my_counts');
    }
}

public function incrementCount($id, $offset = 1) {
    if(empty($id)) return false;
    $count = @Cache::read('item' . $id. '_count', 'my_counts');
    if($count === false) {
        $this->getCount($id);
    } else {
        Cache::increment('item' . $id. '_count', $offset, 'my_counts');
    }
}

---

UPDATE:

This function gets run in a loop (through 1-20 items). When I add the following before the Cache::remember(...:

$toReturn = Cache::read('item' . $id. '_count', 'my_counts');
debug($toReturn);

It gives this:

1. debug: (int) 0
2. debug: (int) 0
3. Notice (8): unserialize(): Error at... (from the Cache::read)
4. debug: false
5. Notice (8): unserialize(): Error at... (from the Cache::remember
6. debug: (int) 0
7. Notice (8): unserialize(): Error at... (from the Cache::read)
8. debug: false
9. Notice (8): unserialize(): Error at... (from the Cache::remember
10. 0
11. 3
12. 1
13. 1 ...

Answer 1

It seems to be a core bug. I might be able to answer to why this issue occurs.

I am guessing that this issue is caused by int(-1). When writing data, RedisEngine doesn't serialize the data if it is an integer. Therefore, int(-1) will be saved without calling serialize().

public function write($key, $value, $duration) {
    if (!is_int($value)) {
        $value = serialize($value);
    }
    ...
}

But when reading data, the implementation seems asymmetric. I don't know Redis well, but I am guessing that Redis stores int(-1) as string(-1). As ctype_digit("-1") returns false, string(-1) would be unserialized wrongly.

public function read($key) {
    $value = $this->_Redis->get($key);
    if (ctype_digit($value)) { // ctype_digit("-1") === false
        $value = (int)$value;
    }
    if ($value !== false && is_string($value)) { // is_string("-1") === true
        $value = unserialize($value);
    }
    ...
}

Thus, you will see "Notice (8): unserialize(): Error at offset 0 of 2 bytes". The size of string(-1) is 2 bytes.

What about opening an issue on github, if you could reproduce that writing int(-1) and reading it throws a notice.

Aside from that, a workaround would be stop using Cache::decrement(). On race conditions, the method will store int(-1). You may use Cache::write() instead if it is not an important data. For example:

$count = Cache::read('item' . $id. '_count', 'my_counts');
if ($count === false || 0 > $count - $offset) {
    $this->getCount($id);
} else {
    Cache::write('item' . $id. '_count', $count - $offset, 'my_counts');
}

But if it is an important data, then you may need to implement exclusive lock/unlock.

Thank you for reading this to the end and sorry if I am wrong.

Answer 2

Solution:

return ($count === false) ? 0 : (int)$count;

Long version:

You shouldn't increment/decrement serialized value.

Cache::remember does something like this:

$key = 'key';
$value = 10;
$serialized = serialize($value); // $serialized = 'i:10;'
$redis->set($key, $serialized);

Cache::read does something like this:

$key = 'key';
$serialized = $redis->get($key); // $serialized = 'i:10;'
$value = unserialize($serialized); // $value = (int) 10
return $value;

But Cache::decrement does something like this:

$key = 'key';
$count = 1;
$redis->decrby($key, $count); // Error, decrby expects plain value, "10", not "i:10;" ( http://redis.io/commands/decrby )

Also, if you decrement successfully, then you will have plain value in redis, so result will be:

$key = 'key';
$serialized = $redis->get($key); // $serialized = '10'
$value = unserialize($serialized); // error, cannot unserialize '10'

EDIT: After checking source: https://github.com/cakephp/cakephp/blob/master/src/Cache/Engine/RedisEngine.php

I see that Cake does checks:

    if (!is_int($value)) {
        $value = serialize($value);
    }

So all you need to do to avoid serialization is to make sure that you are storing (int).

Answer 3

Just remove third parameter, it should be work

return Cache::remember('item' . $id. '_count', function() use ($model, $id) {
    $count = $model->find('count', array(
        'conditions' => array(
            $model->alias . '.status' => 1,
            $model->alias . '.id' => $id
        )
    ));
    return ($count === false) ? 0 : (int)$count;
});