Redis 2.4.10

a memory leak with replication on two or more dbs including >= db10

00-RELEASENOTES

@@ -18,6 +18,15 @@ to modify your program in order to use Redis 2.4.
 CHANGELOG
 ---------
 
+What's new in Redis 2.4.10
+==========================
+
+UPGRADE URGENCY: High for users using replication with a DB identifier higher than 9, or chaining replication so that slaves are attached to other slaves. Low for all the other users.
+
+* [FEATURE] Redis --test-meory improved with new test testing addressing.
+* [BUGFIX] Fixed a memory leak: happens when replication is used with DB > 9.
+* [BUGFIX] Fixed a replication bug that happens in chained replicaiton when a slave is attached to a master that is later turned into a slave. Now the chained slave reconnection is properly forced.
+
 What's new in Redis 2.4.9
 =========================
 

src/memtest.c

@@ -56,6 +56,33 @@ void memtest_progress_step(size_t curr, size_t size, char c) {
     fflush(stdout);
 }
 
+/* Test that addressing is fine. Every location is populated with its own
+ * address, and finally verified. This test is very fast but may detect
+ * ASAP big issues with the memory subsystem. */
+void memtest_addressing(unsigned long *l, size_t bytes) {
+    unsigned long words = bytes/sizeof(unsigned long);
+    unsigned long j, *p;
+
+    /* Fill */
+    p = l;
+    for (j = 0; j < words; j++) {
+        *p = (unsigned long)p;
+        p++;
+        if ((j & 0xffff) == 0) memtest_progress_step(j,words*2,'A');
+    }
+    /* Test */
+    p = l;
+    for (j = 0; j < words; j++) {
+        if (*p != (unsigned long)p) {
+            printf("\n*** MEMORY ADDRESSING ERROR: %p contains %lu\n",
+                (void*) p, *p);
+            exit(1);
+        }
+        p++;
+        if ((j & 0xffff) == 0) memtest_progress_step(j+words,words*2,'A');
+    }
+}
+
 /* Fill words stepping a single page at every write, so we continue to
  * touch all the pages in the smallest amount of time reducing the
  * effectiveness of caches, and making it hard for the OS to transfer
@@ -162,6 +189,11 @@ void memtest_test(size_t megabytes, int passes) {
     }
     while (pass != passes) {
         pass++;
+
+        memtest_progress_start("Addressing test",pass);
+        memtest_addressing(m,bytes);
+        memtest_progress_end();
+
         memtest_progress_start("Random fill",pass);
         memtest_fill_random(m,bytes);
         memtest_progress_end();

src/networking.c

@@ -491,6 +491,16 @@ static void freeClientArgv(redisClient *c) {
     c->cmd = NULL;
 }
 
+/* Close all the slaves connections. This is useful in chained replication
+ * when we resync with our own master and want to force all our slaves to
+ * resync with us as well. */
+void disconnectSlaves(void) {
+    while (listLength(server.slaves)) {
+        listNode *ln = listFirst(server.slaves);
+        freeClient((redisClient*)ln->value);
+    }
+}
+
 void freeClient(redisClient *c) {
     listNode *ln;
 
@@ -568,21 +578,12 @@ void freeClient(redisClient *c) {
         server.master = NULL;
         server.replstate = REDIS_REPL_CONNECT;
         server.repl_down_since = time(NULL);
-        /* Since we lost the connection with the master, we should also
-         * close the connection with all our slaves if we have any, so
-         * when we'll resync with the master the other slaves will sync again
-         * with us as well. Note that also when the slave is not connected
-         * to the master it will keep refusing connections by other slaves.
+        /* We lost connection with our master, force our slaves to resync
+         * with us as well to load the new data set.
          *
-         * We do this only if server.masterhost != NULL. If it is NULL this
-         * means the user called SLAVEOF NO ONE and we are freeing our
-         * link with the master, so no need to close link with slaves. */
-        if (server.masterhost != NULL) {
-            while (listLength(server.slaves)) {
-                ln = listFirst(server.slaves);
-                freeClient((redisClient*)ln->value);
-            }
-        }
+         * If server.masterhost is NULL the user called SLAVEOF NO ONE so
+         * slave resync is not needed. */
+        if (server.masterhost != NULL) disconnectSlaves();
     }
     /* Release memory */
     zfree(c->argv);

src/redis.c

@@ -773,16 +773,11 @@ void createSharedObjects(void) {
     shared.space = createObject(REDIS_STRING,sdsnew(" "));
     shared.colon = createObject(REDIS_STRING,sdsnew(":"));
     shared.plus = createObject(REDIS_STRING,sdsnew("+"));
-    shared.select0 = createStringObject("select 0\r\n",10);
-    shared.select1 = createStringObject("select 1\r\n",10);
-    shared.select2 = createStringObject("select 2\r\n",10);
-    shared.select3 = createStringObject("select 3\r\n",10);
-    shared.select4 = createStringObject("select 4\r\n",10);
-    shared.select5 = createStringObject("select 5\r\n",10);
-    shared.select6 = createStringObject("select 6\r\n",10);
-    shared.select7 = createStringObject("select 7\r\n",10);
-    shared.select8 = createStringObject("select 8\r\n",10);
-    shared.select9 = createStringObject("select 9\r\n",10);
+
+    for (j = 0; j < REDIS_SHARED_SELECT_CMDS; j++) {
+        shared.select[j] = createObject(REDIS_STRING,
+            sdscatprintf(sdsempty(),"select %d\r\n", j));
+    }
     shared.messagebulk = createStringObject("$7\r\nmessage\r\n",13);
     shared.pmessagebulk = createStringObject("$8\r\npmessage\r\n",14);
     shared.subscribebulk = createStringObject("$9\r\nsubscribe\r\n",15);

src/redis.h

@@ -47,6 +47,7 @@
 #define REDIS_EXPIRELOOKUPS_PER_CRON    10 /* lookup 10 expires per loop */
 #define REDIS_MAX_WRITE_PER_EVENT (1024*64)
 #define REDIS_REQUEST_MAX_SIZE (1024*1024*256) /* max bytes in inline command */
+#define REDIS_SHARED_SELECT_CMDS 10
 #define REDIS_SHARED_INTEGERS 10000
 #define REDIS_REPLY_CHUNK_BYTES (5*1500) /* 5 TCP packets with default MTU */
 #define REDIS_INLINE_MAX_SIZE   (1024*64) /* Max size of inline reads */
@@ -372,8 +373,7 @@ struct sharedObjectsStruct {
     *colon, *nullbulk, *nullmultibulk, *queued,
     *emptymultibulk, *wrongtypeerr, *nokeyerr, *syntaxerr, *sameobjecterr,
     *outofrangeerr, *loadingerr, *plus,
-    *select0, *select1, *select2, *select3, *select4,
-    *select5, *select6, *select7, *select8, *select9,
+    *select[REDIS_SHARED_SELECT_CMDS],
     *messagebulk, *pmessagebulk, *subscribebulk, *unsubscribebulk, *mbulk3,
     *mbulk4, *psubscribebulk, *punsubscribebulk,
     *integers[REDIS_SHARED_INTEGERS];
@@ -718,6 +718,7 @@ sds getAllClientsInfoString(void);
 void rewriteClientCommandVector(redisClient *c, int argc, ...);
 unsigned long getClientOutputBufferMemoryUsage(redisClient *c);
 void flushSlavesOutputBuffers(void);
+void disconnectSlaves(void);
 
 #ifdef __GNUC__
 void addReplyErrorFormat(redisClient *c, const char *fmt, ...)

src/replication.c

@@ -25,24 +25,15 @@ void replicationFeedSlaves(list *slaves, int dictid, robj **argv, int argc) {
         if (slave->slaveseldb != dictid) {
             robj *selectcmd;
 
-            switch(dictid) {
-            case 0: selectcmd = shared.select0; break;
-            case 1: selectcmd = shared.select1; break;
-            case 2: selectcmd = shared.select2; break;
-            case 3: selectcmd = shared.select3; break;
-            case 4: selectcmd = shared.select4; break;
-            case 5: selectcmd = shared.select5; break;
-            case 6: selectcmd = shared.select6; break;
-            case 7: selectcmd = shared.select7; break;
-            case 8: selectcmd = shared.select8; break;
-            case 9: selectcmd = shared.select9; break;
-            default:
+            if (dictid >= 0 && dictid < REDIS_SHARED_SELECT_CMDS) {
+                incrRefCount(shared.select[dictid]);
+                selectcmd = shared.select[dictid];
+            } else {
                 selectcmd = createObject(REDIS_STRING,
                     sdscatprintf(sdsempty(),"select %d\r\n",dictid));
-                selectcmd->refcount = 0;
-                break;
             }
             addReply(slave,selectcmd);
+            decrRefCount(selectcmd);
             slave->slaveseldb = dictid;
         }
         addReplyMultiBulkLen(slave,argc);
@@ -507,6 +498,7 @@ void slaveofCommand(redisClient *c) {
         server.masterhost = sdsdup(c->argv[1]->ptr);
         server.masterport = atoi(c->argv[2]->ptr);
         if (server.master) freeClient(server.master);
+        disconnectSlaves(); /* Force our slaves to resync with us as well. */
         if (server.replstate == REDIS_REPL_TRANSFER)
             replicationAbortSyncTransfer();
         server.replstate = REDIS_REPL_CONNECT;

src/version.h

@@ -1 +1 @@
-#define REDIS_VERSION "2.4.9"
+#define REDIS_VERSION "2.4.10"

src/ziplist.c

@@ -489,7 +489,9 @@ static unsigned char *__ziplistInsert(unsigned char *zl, unsigned char *p, unsig
     size_t offset;
     int nextdiff = 0;
     unsigned char encoding = 0;
-    long long value;
+    long long value = 123456789; /* initialized to avoid warning. Using a value
+                                    that is easy to see if for some reason
+                                    we use it uninitialized. */
     zlentry entry, tail;
 
     /* Find out prevlen for the entry that is inserted. */