Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Memcached adapter store bug in internalSetItems method for PHP 7.0 #66

Closed
malinink opened this issue Feb 3, 2016 · 7 comments · Fixed by #67
Closed

Memcached adapter store bug in internalSetItems method for PHP 7.0 #66

malinink opened this issue Feb 3, 2016 · 7 comments · Fixed by #67
Labels
Milestone

Comments

@malinink
Copy link

malinink commented Feb 3, 2016

There is a bug (may be it is a feature)

When I store multiple keys at once instead of storing values (for example int(0) ) it stores something like

i:0;

The reason is in https://github.com/zendframework/zend-cache/blob/master/src/Storage/Adapter/Memcached.php#L403

Code below fixes issue

$namespacedKeyValuePairs[$this->namespacePrefix . $normalizedKey] = $value;

It seems to me that it may be related to all other adapters cause all of them have the same code.

I could provide test if it is necessary.

@ghost
Copy link

ghost commented Feb 3, 2016

hi @malinink

As you describe it looks like you the value gets serialized which has nothing to do with the variable by reference you linked.

Please provide a code example incl. how you setup the storage adapter and call to write/read with data as var_dump.

@malinink
Copy link
Author

malinink commented Feb 4, 2016

@DCMNMarc
I add test into MemcachedTest
All connection options are default (from phpunit.xml.dist)

    public function testMultipleKeySetIncrement()
    {
        $this->_storage->setItems(['key' => 0]);
        $value = $this->_storage->getItem('key');
        var_dump($value);
        $this->_storage->incrementItem('key', 1);
    }

That is test screen:

./vendor/bin/phpunit --filter testMultipleKeySetIncrement
PHPUnit 4.8.22 by Sebastian Bergmann and contributors.

Eint(0)


Time: 167 ms, Memory: 8.00Mb

There was 1 error:

1) ZendTest\Cache\Storage\Adapter\MemcachedTest::testMultipleKeySetIncrement
Zend\Cache\Exception\RuntimeException: CLIENT ERROR

zend-cache/src/Storage/Adapter/Memcached.php:700
zend-cache/src/Storage/Adapter/Memcached.php:578
zend-cache/src/Storage/Adapter/AbstractAdapter.php:1272
zend-cache/test/Storage/Adapter/MemcachedTest.php:206

FAILURES!
Tests: 1, Assertions: 2, Errors: 1.

It seems like adapter could get and set such kind of values but it store them as none integer values, so increment doesn't work.

That is real value stored in memcached

echo "get zfcache:key" | nc localhost 11211
VALUE zfcache:key 4 4
i:0;
END

Normally that value must be (and that happens in setItem() method)

echo "get zfcache:key" | nc localhost 11211
VALUE zfcache:key 1 1
0
END

That is error itself

echo "incr zfcache:key 1" | nc localhost 11211
CLIENT_ERROR cannot increment or decrement non-numeric value

@marc-mabe
Copy link
Member

@malinink Thanks for your help. I can reproduce the bug with Memcached and Apc and created a fix with #67
It's probably related the internal changes how PHP-7 handles variables by reference.

@malinink
Copy link
Author

malinink commented Feb 7, 2016

@marc-mabe
That PR completely fixes the issue. Thanks!

@malinink malinink closed this as completed Feb 7, 2016
@marc-mabe
Copy link
Member

@malinink nice but the issue should be closed on merging that PR - thanks

@marc-mabe marc-mabe reopened this Feb 7, 2016
@marc-mabe
Copy link
Member

link #74

@malinink I just noticed that currently there is no release (no stable, no beta, no alpha) for php memcached that supports PHP-7 (see #74) how did you install that?

@malinink
Copy link
Author

@marc-mabe
I have installed php7.0-fpm package from that ppa https://launchpad.net/~ondrej/+archive/ubuntu/php
But as you can see there is no php7.0-memcached package present there.
I could install it as usual with php-memcached name.
I also install php-redis and other packages that are not prefixed with php7.0 (I could provide all list), and have not found any problems (So I decided that these packages are fully compatible).

That is one thing I don't think about.

@marc-mabe marc-mabe added this to the 2.7.1 milestone Apr 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants