Fix BRPOPLPUSH behavior for all use cases.

diff --git a/src/t_list.c b/src/t_list.c
index 0da70a040b908fb059f458561dfa262a6ea8c547..ceed70c31697f26d2ab02a2eb2b39993e9c46cdb 100644 (file)
--- a/src/t_list.c
+++ b/src/t_list.c
@@ -748,10 +748,6 @@ void unblockClientWaitingData(redisClient *c) {
         decrRefCount(c->bpop.keys[j]);
     }
 
         decrRefCount(c->bpop.keys[j]);
     }
 

-    if (c->bpop.target != NULL) {
-        decrRefCount(c->bpop.target);
-    }
-

     /* Cleanup the client structure */
     zfree(c->bpop.keys);
     c->bpop.keys = NULL;
     /* Cleanup the client structure */
     zfree(c->bpop.keys);
     c->bpop.keys = NULL;


@@ -789,7 +785,11 @@ int handleClientsWaitingListPush(redisClient *c, robj *key, robj *ele) {
     redisAssert(ln != NULL);
     receiver = ln->value;
 
     redisAssert(ln != NULL);
     receiver = ln->value;
 

-    if (receiver->bpop.target == NULL) {
+    robj *target = receiver->bpop.target;
+
+    unblockClientWaitingData(receiver);
+
+    if (target == NULL) {

         /* BRPOP/BLPOP return a multi-bulk with the name
          * of the popped list */
         addReplyMultiBulkLen(receiver,2);
         /* BRPOP/BLPOP return a multi-bulk with the name
          * of the popped list */
         addReplyMultiBulkLen(receiver,2);


@@ -797,23 +797,23 @@ int handleClientsWaitingListPush(redisClient *c, robj *key, robj *ele) {
         addReplyBulk(receiver,ele);
     } else {
         /* BRPOPLPUSH */
         addReplyBulk(receiver,ele);
     } else {
         /* BRPOPLPUSH */

-        robj *dobj = lookupKeyWrite(receiver->db,receiver->bpop.target);
+        robj *dobj = lookupKeyWrite(receiver->db,target);

         if (dobj && checkType(receiver,dobj,REDIS_LIST)) return 0;
 
         addReplyBulk(receiver,ele);
 
         if (dobj && checkType(receiver,dobj,REDIS_LIST)) return 0;
 
         addReplyBulk(receiver,ele);
 

-        if (!handleClientsWaitingListPush(receiver, receiver->bpop.target, ele)) {
+        if (!handleClientsWaitingListPush(receiver, target, ele)) {

             /* Create the list if the key does not exist */
             if (!dobj) {
                 dobj = createZiplistObject();
             /* Create the list if the key does not exist */
             if (!dobj) {
                 dobj = createZiplistObject();

-                dbAdd(receiver->db, receiver->bpop.target, dobj);
+                dbAdd(receiver->db, target, dobj);

             }
             }

-

             listTypePush(dobj, ele, REDIS_HEAD);
         }
             listTypePush(dobj, ele, REDIS_HEAD);
         }

+
+        decrRefCount(target);

     }
 
     }
 

-    unblockClientWaitingData(receiver);

     return 1;
 }
 
     return 1;
 }
 

diff --git a/tests/unit/type/list.tcl b/tests/unit/type/list.tcl
index 62ea159dd4566edcb8fcd6865c77af486d20276e..3616443344468d2ec998556ebae8812663c9c4ac 100644 (file)
--- a/tests/unit/type/list.tcl
+++ b/tests/unit/type/list.tcl
@@ -191,6 +191,49 @@ start_server {
         assert_equal {foo} [r lrange blist 0 -1]
     }
 
         assert_equal {foo} [r lrange blist 0 -1]
     }
 

+    test "linked BRPOPLPUSH" {
+      set rd1 [redis_deferring_client]
+      set rd2 [redis_deferring_client]
+
+      r del list1 list2 list3
+
+      $rd1 brpoplpush list1 list2 0
+      $rd2 brpoplpush list2 list3 0
+
+      r rpush list1 foo
+
+      assert_equal {} [r lrange list1 0 -1]
+      assert_equal {} [r lrange list2 0 -1]
+      assert_equal {foo} [r lrange list3 0 -1]
+    }
+
+    test "circular BRPOPLPUSH" {
+      set rd1 [redis_deferring_client]
+      set rd2 [redis_deferring_client]
+
+      r del list1 list2
+
+      $rd1 brpoplpush list1 list2 0
+      $rd2 brpoplpush list2 list1 0
+
+      r rpush list1 foo
+
+      assert_equal {foo} [r lrange list1 0 -1]
+      assert_equal {} [r lrange list2 0 -1]
+    }
+
+    test "self-referential BRPOPLPUSH" {
+      set rd [redis_deferring_client]
+
+      r del blist
+
+      $rd brpoplpush blist blist 0
+
+      r rpush blist foo
+
+      assert_equal {foo} [r lrange blist 0 -1]
+    }
+

     test "BRPOPLPUSH inside a transaction" {
         r del xlist target
         r lpush xlist foo
     test "BRPOPLPUSH inside a transaction" {
         r del xlist target
         r lpush xlist foo