Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP7 cas doesn't work #263

Closed
bolknote opened this issue Jul 13, 2016 · 4 comments
Closed

PHP7 cas doesn't work #263

bolknote opened this issue Jul 13, 2016 · 4 comments

Comments

@bolknote
Copy link

bolknote commented Jul 13, 2016

Last php70-php-pecl-memcached.x86_64 from remi70 (3.0.0-dev):

$m = new \Memcached();
$m->addServer('127.0.0.1', 11211);

$m->delete('cas_test');
$cas_token = null;
$m->set('cas_test', 10);
$v = $m->get('cas_test', null, $cas_token);

var_dump($cas_token); // NULL
var_dump($v); // 10

var_dump($m->getResultCode()); // 10 (MEMCACHED_SERVER_ERROR)

$m->getDelayed(['cas_test'], true);
var_dump($m->fetchAll()[0]['cas']); // int(1057)

2016-07-13 19 47 00

I think you should use 'gets' instead of 'get' if we ask cas token.

@TysonAndre
Copy link
Contributor

It seems as though the CAS param was omitted from the PHP7 implementation. cas_token was passed to zend_parse_parameters in php_memc_get_impl in php5, but not in php7.

In php5 (with CAS return value): https://github.com/php-memcached-dev/php-memcached/blob/master/php_memcached.c#L570

/* {{{ -- php_memc_get_impl */
static void php_memc_get_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key)
{
    char *key = NULL;
    int key_len = 0;
    char *server_key = NULL;
    int   server_key_len = 0;
    uint32_t flags = 0;
    uint64_t cas = 0;
    zval *cas_token = NULL;
    zval *udf_flags = NULL;
    zend_fcall_info fci = empty_fcall_info;
    zend_fcall_info_cache fcc = empty_fcall_info_cache;
    // other vars omitted

    if (by_key) {
        if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ss|f!zz", &server_key,
                                  &server_key_len, &key, &key_len, &fci, &fcc, &cas_token, &udf_flags) == FAILURE) {
            return;
        }
    } else {
        if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|f!zz", &key, &key_len,
                                  &fci, &fcc, &cas_token, &udf_flags) == FAILURE) {
            return;
        }
    }

In php7 (without CAS return value): https://github.com/php-memcached-dev/php-memcached/blob/php7/php_memcached.c#L1386

static
void php_memc_get_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key)
{
    php_memc_get_ctx_t context = {};
    php_memc_keys_t keys = {0}; 
    zend_long get_flags = 0; 
    zend_string *key;
    zend_string *server_key = NULL;
    zend_bool mget_status;
    memcached_return status = MEMCACHED_SUCCESS;
    zend_fcall_info fci = empty_fcall_info;
    zend_fcall_info_cache fcc = empty_fcall_info_cache;
    MEMC_METHOD_INIT_VARS;

    if (by_key) {
        if (zend_parse_parameters(ZEND_NUM_ARGS(), "SS|f!l", &server_key, &key, &fci, &fcc, &get_flags) == FAILURE) {
            return;
        }
    } else {
        if (zend_parse_parameters(ZEND_NUM_ARGS(), "S|f!l", &key, &fci, &fcc, &get_flags) == FAILURE) {
            return;
        }
    }   

@TysonAndre
Copy link
Contributor

Oh. This was mentioned in #214 (comment) .
They suggest using Memcached::GET_EXTENDED.

The API docs would need to be updated for get() as well.
See #229

@TysonAndre
Copy link
Contributor

TysonAndre commented Jul 26, 2016

@bolknote , I assume you'd need to use something like this in any library that needs to work in multiple memcached versions. Not sure if this is completely correct, tell me if there are bugs.

<?php
/**
 * Utilities to deal with Memcached API breaking changes in 3.0 in Memcached->get() and Memcached->getMulti().
 * TODO getMulti
 */
class MemcachedUtil {
    /**
     * Returns data stored in cache at $key.
     *
     * @param Memcached $m
     * @param string $key
     * @param callable|null $v (Must be null for now, see https://github.com/php-memcached-dev/php-memcached/issues/248 )
     * @param int|string &$token token for CAS(returned by reference)
     * @return mixed|false the value in memcached, or false
     */
    public static function get_with_token($m, $key, $v = null, &$token = null) {
        if ($v !== null) {
             throw new Exception("Providing read-through callback may or may not have bugs in php7");
        }
        if (self::does_get_return_cas_by_reference($m)) {
            return $m->get($key, $v, $token);
        } else {
            // $values has fields value, cas, and flags
            $values = $m->get($key, $v, Memcached::GET_EXTENDED);
            if (is_array($values)) {
                $token = $values['cas'];
                return $values['value'];
            }
            return false;
        }
    }

    /**
     * @param Memcached $m
     * @return boolean Do get() and getMulti() return $token by reference, or do you have to pass Memcached::GET_EXTENDED as a bit flag in that position instead.
     */
    public static function does_get_return_cas_by_reference($m) {
        // TODO Logic for handling mocks
        static $returnsReference;
        if ($returnsReference === null) {
            // memcached < 3.0.0a1 (approx) returns the CAS token by reference.
            $returnsReference = version_compare(phpversion("memcached"), '3.0.0a1', '<');
        }
        return $returnsReference;
    }
}

EDIT: version_compare was in the wrong order

@bolknote
Copy link
Author

Thank you! It works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants