Properly init/release iterators in zunionInterGenericCommand().

This commit does mainly two things:

1) It fixes zunionInterGenericCommand() by removing mass-initialization
of all the iterators used, so that we don't violate the unsafe iterator
API of dictionaries. This fixes issue #1240.

2) Since the zui* APIs required the allocator to be initialized in the
zsetopsrc structure in order to use non-iterator related APIs, this
commit fixes this strict requirement by accessing objects directly via
the op->subject->ptr pointer we have to the object.
This commit is contained in:
antirez 2013-08-16 15:26:44 +02:00
parent 48cde3fe47
commit cfb9d025c6

View File

@ -1270,20 +1270,20 @@ int zuiLength(zsetopsrc *op) {
return 0; return 0;
if (op->type == REDIS_SET) { if (op->type == REDIS_SET) {
iterset *it = &op->iter.set;
if (op->encoding == REDIS_ENCODING_INTSET) { if (op->encoding == REDIS_ENCODING_INTSET) {
return intsetLen(it->is.is); return intsetLen(op->subject->ptr);
} else if (op->encoding == REDIS_ENCODING_HT) { } else if (op->encoding == REDIS_ENCODING_HT) {
return dictSize(it->ht.dict); dict *ht = op->subject->ptr;
return dictSize(ht);
} else { } else {
redisPanic("Unknown set encoding"); redisPanic("Unknown set encoding");
} }
} else if (op->type == REDIS_ZSET) { } else if (op->type == REDIS_ZSET) {
iterzset *it = &op->iter.zset;
if (op->encoding == REDIS_ENCODING_ZIPLIST) { if (op->encoding == REDIS_ENCODING_ZIPLIST) {
return zzlLength(it->zl.zl); return zzlLength(op->subject->ptr);
} else if (op->encoding == REDIS_ENCODING_SKIPLIST) { } else if (op->encoding == REDIS_ENCODING_SKIPLIST) {
return it->sl.zs->zsl->length; zset *zs = op->subject->ptr;
return zs->zsl->length;
} else { } else {
redisPanic("Unknown sorted set encoding"); redisPanic("Unknown sorted set encoding");
} }
@ -1419,18 +1419,19 @@ int zuiFind(zsetopsrc *op, zsetopval *val, double *score) {
return 0; return 0;
if (op->type == REDIS_SET) { if (op->type == REDIS_SET) {
iterset *it = &op->iter.set;
if (op->encoding == REDIS_ENCODING_INTSET) { if (op->encoding == REDIS_ENCODING_INTSET) {
if (zuiLongLongFromValue(val) && intsetFind(it->is.is,val->ell)) { if (zuiLongLongFromValue(val) &&
intsetFind(op->subject->ptr,val->ell))
{
*score = 1.0; *score = 1.0;
return 1; return 1;
} else { } else {
return 0; return 0;
} }
} else if (op->encoding == REDIS_ENCODING_HT) { } else if (op->encoding == REDIS_ENCODING_HT) {
dict *ht = op->subject->ptr;
zuiObjectFromValue(val); zuiObjectFromValue(val);
if (dictFind(it->ht.dict,val->ele) != NULL) { if (dictFind(ht,val->ele) != NULL) {
*score = 1.0; *score = 1.0;
return 1; return 1;
} else { } else {
@ -1440,19 +1441,19 @@ int zuiFind(zsetopsrc *op, zsetopval *val, double *score) {
redisPanic("Unknown set encoding"); redisPanic("Unknown set encoding");
} }
} else if (op->type == REDIS_ZSET) { } else if (op->type == REDIS_ZSET) {
iterzset *it = &op->iter.zset;
zuiObjectFromValue(val); zuiObjectFromValue(val);
if (op->encoding == REDIS_ENCODING_ZIPLIST) { if (op->encoding == REDIS_ENCODING_ZIPLIST) {
if (zzlFind(it->zl.zl,val->ele,score) != NULL) { if (zzlFind(op->subject->ptr,val->ele,score) != NULL) {
/* Score is already set by zzlFind. */ /* Score is already set by zzlFind. */
return 1; return 1;
} else { } else {
return 0; return 0;
} }
} else if (op->encoding == REDIS_ENCODING_SKIPLIST) { } else if (op->encoding == REDIS_ENCODING_SKIPLIST) {
zset *zs = op->subject->ptr;
dictEntry *de; dictEntry *de;
if ((de = dictFind(it->sl.zs->dict,val->ele)) != NULL) { if ((de = dictFind(zs->dict,val->ele)) != NULL) {
*score = *(double*)dictGetVal(de); *score = *(double*)dictGetVal(de);
return 1; return 1;
} else { } else {
@ -1580,9 +1581,6 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) {
} }
} }
for (i = 0; i < setnum; i++)
zuiInitIterator(&src[i]);
/* sort sets from the smallest to largest, this will improve our /* sort sets from the smallest to largest, this will improve our
* algorithm's performance */ * algorithm's performance */
qsort(src,setnum,sizeof(zsetopsrc),zuiCompareByCardinality); qsort(src,setnum,sizeof(zsetopsrc),zuiCompareByCardinality);
@ -1596,6 +1594,7 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) {
if (zuiLength(&src[0]) > 0) { if (zuiLength(&src[0]) > 0) {
/* Precondition: as src[0] is non-empty and the inputs are ordered /* Precondition: as src[0] is non-empty and the inputs are ordered
* by size, all src[i > 0] are non-empty too. */ * by size, all src[i > 0] are non-empty too. */
zuiInitIterator(&src[0]);
while (zuiNext(&src[0],&zval)) { while (zuiNext(&src[0],&zval)) {
double score, value; double score, value;
@ -1630,12 +1629,14 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) {
} }
} }
} }
zuiClearIterator(&src[0]);
} }
} else if (op == REDIS_OP_UNION) { } else if (op == REDIS_OP_UNION) {
for (i = 0; i < setnum; i++) { for (i = 0; i < setnum; i++) {
if (zuiLength(&src[i]) == 0) if (zuiLength(&src[i]) == 0)
continue; continue;
zuiInitIterator(&src[i]);
while (zuiNext(&src[i],&zval)) { while (zuiNext(&src[i],&zval)) {
double score, value; double score, value;
@ -1672,14 +1673,12 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) {
maxelelen = sdslen(tmp->ptr); maxelelen = sdslen(tmp->ptr);
} }
} }
zuiClearIterator(&src[i]);
} }
} else { } else {
redisPanic("Unknown operator"); redisPanic("Unknown operator");
} }
for (i = 0; i < setnum; i++)
zuiClearIterator(&src[i]);
if (dbDelete(c->db,dstkey)) { if (dbDelete(c->db,dstkey)) {
signalModifiedKey(c->db,dstkey); signalModifiedKey(c->db,dstkey);
touched = 1; touched = 1;