From 5a5af9a4f9a05897d816c74dae45c64da8150f43 Mon Sep 17 00:00:00 2001 From: Brian Noguchi Date: Mon, 12 Mar 2012 22:17:13 -0700 Subject: [PATCH 1/3] Add comma to package.json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ac9f0c522b..01ee929671 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ }, "optionalDependencies": { "hiredis": "*" - } + }, "repository": { "type": "git", "url": "git://github.com/mranney/node_redis.git" From 2afa0e5acca0051749beb4b4eb19b1d70418b5ed Mon Sep 17 00:00:00 2001 From: Brian Noguchi Date: Mon, 12 Mar 2012 22:59:38 -0700 Subject: [PATCH 2/3] Add failing test. The test demonstrates failure for the following scenario. A single-subscription client calls unsubscribe immediately followed by a subscribe. It will fail when it tries to receive the next pmessage/message because the client will be in false pub_sub_mode. Here is why it is false: First, the 2nd subscribe sets pub_sub_mode to true during send_command. Next, the unsubscribe's return_reply sets pub_sub_mode to false. The 2nd subscribe's return_reply does not re-set pub_sub_mode back to true. So the result is a client with false pub_sub_mode that fails upon receipt of the next message or pmessage. --- tests/re_sub_test.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 tests/re_sub_test.js diff --git a/tests/re_sub_test.js b/tests/re_sub_test.js new file mode 100644 index 0000000000..64b8f31287 --- /dev/null +++ b/tests/re_sub_test.js @@ -0,0 +1,14 @@ +var client = require('../index').createClient() + , client2 = require('../index').createClient() + , assert = require('assert'); + +client.once('subscribe', function (channel, count) { + client.unsubscribe('x'); + client.subscribe('x', function () { + client.quit(); + client2.quit(); + }); + client2.publish('x', 'hi'); +}); + +client.subscribe('x'); From 64a0e689278d0d2621445420d814ad13f39f925f Mon Sep 17 00:00:00 2001 From: Brian Noguchi Date: Mon, 12 Mar 2012 23:03:46 -0700 Subject: [PATCH 3/3] Add fix for last test (re-establish pub_sub_mode = true) --- index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/index.js b/index.js index eb27628b97..1ace00ed0c 100644 --- a/index.js +++ b/index.js @@ -570,6 +570,8 @@ RedisClient.prototype.return_reply = function (reply) { if (this.debug_mode) { console.log("All subscriptions removed, exiting pub/sub mode"); } + } else { + this.pub_sub_mode = true; } // subscribe commands take an optional callback and also emit an event, but only the first response is included in the callback // TODO - document this or fix it so it works in a more obvious way