From f72a3177a30286c392b347807f2fe10b8482c022 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Mon, 10 Mar 2014 13:18:19 -0700 Subject: Fix autorehashing in RedisArray This commit fixes auto rehashing in RedisArray as well as fixes a couple of memory leaks found along the way Addresses #442 and #294 --- redis_array.c | 91 ++++++++++++++++++++++++++++-------------------------- redis_array_impl.c | 4 +++ 2 files changed, 52 insertions(+), 43 deletions(-) diff --git a/redis_array.c b/redis_array.c index 2c431424..2a36c563 100644 --- a/redis_array.c +++ b/redis_array.c @@ -32,6 +32,12 @@ #include "redis_array.h" #include "redis_array_impl.h" +/* Simple macro to detect failure in a RedisArray call */ +#define RA_CALL_FAILED(rv, cmd) \ + ((Z_TYPE_P(rv) == IS_BOOL && Z_BVAL_P(rv) == 0) || \ + (Z_TYPE_P(rv) == IS_ARRAY && zend_hash_num_elements(Z_ARRVAL_P(rv)) == 0) || \ + (Z_TYPE_P(rv) == IS_LONG && Z_LVAL_P(rv) == 0 && !strcasecmp(cmd, "TYPE"))) \ + extern zend_class_entry *redis_ce; zend_class_entry *redis_array_ce; @@ -76,41 +82,48 @@ zend_function_entry redis_array_functions[] = { {NULL, NULL, NULL} }; -int le_redis_array; -void redis_destructor_redis_array(zend_rsrc_list_entry * rsrc TSRMLS_DC) -{ - int i; - RedisArray *ra = (RedisArray*)rsrc->ptr; +static void redis_array_free(RedisArray *ra) { + int i; - /* delete Redis objects */ - for(i = 0; i < ra->count; ++i) { - zval_dtor(ra->redis[i]); - efree(ra->redis[i]); + // Redis objects + for(i=0;icount;i++) { + zval_dtor(ra->redis[i]); + efree(ra->redis[i]); + efree(ra->hosts[i]); + } + efree(ra->redis); + efree(ra->hosts); - /* remove host too */ - efree(ra->hosts[i]); - } - efree(ra->redis); - efree(ra->hosts); + /* delete hash function */ + if(ra->z_fun) { + zval_dtor(ra->z_fun); + efree(ra->z_fun); + } - /* delete function */ - if(ra->z_fun) { - zval_dtor(ra->z_fun); - efree(ra->z_fun); - } + /* Distributor */ + if(ra->z_dist) { + zval_dtor(ra->z_dist); + efree(ra->z_dist); + } - /* delete distributor */ - if(ra->z_dist) { - zval_dtor(ra->z_dist); - efree(ra->z_dist); - } + /* Delete pur commands */ + zval_dtor(ra->z_pure_cmds); + efree(ra->z_pure_cmds); + + // Free structure itself + efree(ra); +} - /* delete list of pure commands */ - zval_dtor(ra->z_pure_cmds); - efree(ra->z_pure_cmds); +int le_redis_array; +void redis_destructor_redis_array(zend_rsrc_list_entry * rsrc TSRMLS_DC) +{ + RedisArray *ra = (RedisArray*)rsrc->ptr; - /* free container */ - efree(ra); + /* Free previous ring if it's set */ + if(ra->prev) redis_array_free(ra->prev); + + /* Free parent array */ + redis_array_free(ra); } /** @@ -280,6 +293,7 @@ PHP_METHOD(RedisArray, __construct) if(ra) { ra->auto_rehash = b_autorehash; + if(ra->prev) ra->prev->auto_rehash = b_autorehash; #if PHP_VERSION_ID >= 50400 id = zend_list_insert(ra, le_redis_array TSRMLS_CC); #else @@ -302,7 +316,6 @@ ra_forward_call(INTERNAL_FUNCTION_PARAMETERS, RedisArray *ra, const char *cmd, i HashTable *h_args; int argc; - int failed; zend_bool b_write_cmd = 0; h_args = Z_ARRVAL_P(z_args); @@ -366,23 +379,15 @@ ra_forward_call(INTERNAL_FUNCTION_PARAMETERS, RedisArray *ra, const char *cmd, i } else { /* call directly through. */ call_user_function(&redis_ce->function_table, &redis_inst, &z_fun, return_value, argc, z_callargs TSRMLS_CC); - failed = 0; - if((Z_TYPE_P(return_value) == IS_BOOL && Z_BVAL_P(return_value) == 0) || - (Z_TYPE_P(return_value) == IS_ARRAY && zend_hash_num_elements(Z_ARRVAL_P(return_value)) == 0) || - (Z_TYPE_P(return_value) == IS_LONG && Z_LVAL_P(return_value) == 0 && !strcasecmp(cmd, "TYPE"))) - - { - failed = 1; - } - /* check if we have an error. */ - if(failed && ra->prev && !b_write_cmd) { /* there was an error reading, try with prev ring. */ - /* ERROR, FALLBACK TO PREVIOUS RING and forward a reference to the first redis instance we were looking at. */ + if(RA_CALL_FAILED(return_value,cmd) && ra->prev && !b_write_cmd) { /* there was an error reading, try with prev ring. */ + /* ERROR, FALLBACK TO PREVIOUS RING and forward a reference to the first redis instance we were looking at. */ ra_forward_call(INTERNAL_FUNCTION_PARAM_PASSTHRU, ra->prev, cmd, cmd_len, z_args, z_new_target?z_new_target:redis_inst); } - if(!failed && !b_write_cmd && z_new_target && ra->auto_rehash) { /* move key from old ring to new ring */ - ra_move_key(key, key_len, redis_inst, z_new_target TSRMLS_CC); + /* Autorehash if the key was found on the previous node if this is a read command and auto rehashing is on */ + if(!RA_CALL_FAILED(return_value,cmd) && !b_write_cmd && z_new_target && ra->auto_rehash) { /* move key from old ring to new ring */ + ra_move_key(key, key_len, redis_inst, z_new_target TSRMLS_CC); } } diff --git a/redis_array_impl.c b/redis_array_impl.c index 6b125a4c..09f7c8b5 100644 --- a/redis_array_impl.c +++ b/redis_array_impl.c @@ -273,6 +273,7 @@ RedisArray *ra_load_array(const char *name TSRMLS_DC) { /* create RedisArray object */ ra = ra_make_array(hHosts, z_fun, z_dist, hPrev, b_index, b_pconnect, l_retry_interval, b_lazy_connect TSRMLS_CC); ra->auto_rehash = b_autorehash; + if(ra->prev) ra->prev->auto_rehash = b_autorehash; /* cleanup */ zval_dtor(z_params_hosts); @@ -605,6 +606,7 @@ ra_index_key(const char *key, int key_len, zval *z_redis TSRMLS_DC) { /* don't dtor z_ret, since we're returning z_redis */ efree(z_args[0]); + zval_dtor(z_args[1]); efree(z_args[1]); } @@ -967,6 +969,7 @@ ra_move_string(const char *key, int key_len, zval *z_from, zval *z_to, long ttl ZVAL_STRINGL(z_args[0], key, key_len, 0); ZVAL_LONG(z_args[1], ttl); ZVAL_STRINGL(z_args[2], Z_STRVAL(z_ret), Z_STRLEN(z_ret), 1); /* copy z_ret to arg 1 */ + zval_dtor(&z_ret); /* free memory from our previous call */ call_user_function(&redis_ce->function_table, &z_to, &z_fun_set, &z_ret, 3, z_args TSRMLS_CC); /* cleanup */ efree(z_args[1]); @@ -977,6 +980,7 @@ ra_move_string(const char *key, int key_len, zval *z_from, zval *z_to, long ttl ZVAL_STRINGL(&z_fun_set, "SET", 3, 0); ZVAL_STRINGL(z_args[0], key, key_len, 0); ZVAL_STRINGL(z_args[1], Z_STRVAL(z_ret), Z_STRLEN(z_ret), 1); /* copy z_ret to arg 1 */ + zval_dtor(&z_ret); /* free memory from our previous return value */ call_user_function(&redis_ce->function_table, &z_to, &z_fun_set, &z_ret, 2, z_args TSRMLS_CC); /* cleanup */ zval_dtor(z_args[1]); -- cgit v1.2.3