From defb5f66a090c614aa70aee9e1923ec571e0e911 Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 11 May 2011 09:49:23 +0200 Subject: [PATCH 1/6] removed assert causing an illegal memory access. This was responsible of crashes during BLPOP and other list blocking operations. --- src/t_list.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/t_list.c b/src/t_list.c index adb0c409..7c1b848a 100644 --- a/src/t_list.c +++ b/src/t_list.c @@ -822,7 +822,6 @@ int handleClientsWaitingListPush(redisClient *c, robj *key, robj *ele) { /* This should remove the first element of the "clients" list. */ unblockClientWaitingData(receiver); - redisAssert(ln != listFirst(clients)); if (dstkey == NULL) { /* BRPOP/BLPOP */ From af9aed25e44cf6e0c38cee2d289c7c31f03fc01f Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 12 May 2011 20:20:25 +0200 Subject: [PATCH 2/6] ZINTERSTORE regressiont test with two sets, intset+hashtable --- tests/unit/type/zset.tcl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/type/zset.tcl b/tests/unit/type/zset.tcl index 4fa7af1b..37668cfe 100644 --- a/tests/unit/type/zset.tcl +++ b/tests/unit/type/zset.tcl @@ -460,6 +460,13 @@ start_server {tags {"zset"}} { basics ziplist basics skiplist + test {ZINTERSTORE regression with two sets, intset+hashtable} { + r del seta setb setc + r sadd set1 a + r sadd set2 10 + r zinterstore set3 2 set1 set2 + } {0} + proc stressers {encoding} { if {$encoding == "ziplist"} { # Little extra to allow proper fuzzing in the sorting stresser From 70bc5f7724364e93c63865c02d517bc0164274d9 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 12 May 2011 20:15:13 +0200 Subject: [PATCH 3/6] replication with expire test modified to produce no or less false failures --- tests/integration/replication.tcl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/replication.tcl b/tests/integration/replication.tcl index 6ca5a6dd..7be4df4b 100644 --- a/tests/integration/replication.tcl +++ b/tests/integration/replication.tcl @@ -27,6 +27,8 @@ start_server {tags {"repl"}} { test {MASTER and SLAVE consistency with expire} { createComplexDataset r 50000 useexpire after 4000 ;# Make sure everything expired before taking the digest + r keys * ;# Force DEL syntesizing to slave + after 1000 ;# Wait another second. Now everything should be file. if {[r debug digest] ne [r -1 debug digest]} { set csv1 [csvdump r] set csv2 [csvdump {r -1}] From dd1eefa4f3c89177cbe4f2e98dbd8f409ff87bc6 Mon Sep 17 00:00:00 2001 From: antirez Date: Sun, 15 May 2011 12:15:54 +0200 Subject: [PATCH 4/6] Fixed SINTER[STORE] problem related to the new copy on write safe iterator --- src/t_set.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/t_set.c b/src/t_set.c index b221e2e9..be083c8b 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -437,6 +437,7 @@ void sinterGenericCommand(redisClient *c, robj **setkeys, unsigned long setnum, si = setTypeInitIterator(sets[0]); while((encoding = setTypeNext(si,&eleobj,&intobj)) != -1) { for (j = 1; j < setnum; j++) { + if (sets[j] == sets[0]) continue; if (encoding == REDIS_ENCODING_INTSET) { /* intset with intset is simple... and fast */ if (sets[j]->encoding == REDIS_ENCODING_INTSET && From d070abe44cdc63ece3533d06986629b5b5c21ca8 Mon Sep 17 00:00:00 2001 From: antirez Date: Sun, 15 May 2011 15:33:01 +0200 Subject: [PATCH 5/6] Fix for a possible bug related to ZINTER/UNIONSTORE called with the same source set more than one time. --- src/t_zset.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/t_zset.c b/src/t_zset.c index 3e76ebd1..8b166209 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -1521,7 +1521,10 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { score = src[0].weight * zval.score; for (j = 1; j < setnum; j++) { - if (zuiFind(&src[j],&zval,&value)) { + if (src[j].subject == src[0].subject) { + value = zval.score*src[j].weight; + zunionInterAggregate(&score,value,aggregate); + } else if (zuiFind(&src[j],&zval,&value)) { value *= src[j].weight; zunionInterAggregate(&score,value,aggregate); } else { From cb16b6c3899d0696a7e633c29f764e06b222b2fe Mon Sep 17 00:00:00 2001 From: antirez Date: Sun, 15 May 2011 17:28:06 +0200 Subject: [PATCH 6/6] Fixed misuse of the new iterator semantics in ZUNIONSTORE --- src/t_zset.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/t_zset.c b/src/t_zset.c index 8b166209..1c55cb97 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -1521,6 +1521,8 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { score = src[0].weight * zval.score; for (j = 1; j < setnum; j++) { + /* It is not safe to access the hash we zset we are + * iterating, so explicitly check for equal object. */ if (src[j].subject == src[0].subject) { value = zval.score*src[j].weight; zunionInterAggregate(&score,value,aggregate); @@ -1564,7 +1566,12 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { /* Because the inputs are sorted by size, it's only possible * for sets at larger indices to hold this element. */ for (j = (i+1); j < setnum; j++) { - if (zuiFind(&src[j],&zval,&value)) { + /* It is not safe to access the hash we zset we are + * iterating, so explicitly check for equal object. */ + if(src[j].subject == src[i].subject) { + value = zval.score*src[j].weight; + zunionInterAggregate(&score,value,aggregate); + } else if (zuiFind(&src[j],&zval,&value)) { value *= src[j].weight; zunionInterAggregate(&score,value,aggregate); }