Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Pub sub mode fix #190

Merged
merged 3 commits into from Apr 30, 2012

Conversation

Projects
None yet
3 participants
Contributor

bnoguchi commented Mar 13, 2012

Failure will occur in the following scenario:

A single-subscription client (i.e., a client that called subscribe just once before) calls unsubscribe immediately followed by a subscribe (the 2nd call to 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 will have false pub_sub_mode:

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.

This pull request adds both a test to demonstrate the failing scenario and a fix for it that sets pub_sub_mode to true upon receipt of a subscribe reply.

bnoguchi added some commits Mar 13, 2012

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.
Contributor

DTrejo commented Apr 28, 2012

Thank you brian!
Test did fail, and your change did fix the problem. All tests still passing.
@mranney, ok to merge?
D

mranney added a commit that referenced this pull request Apr 30, 2012

@mranney mranney merged commit 9e76387 into NodeRedis:master Apr 30, 2012

Contributor

mranney commented Apr 30, 2012

Thanks for the fix, @bnoguchi.

mranney added a commit that referenced this pull request Apr 30, 2012

Many contributed fixes. Thank you, contributors.
* [GH-190] - pub/sub mode fix (Brian Noguchi)
* [GH-165] - parser selection fix (TEHEK)
* numerous documentation and examples updates
* auth errors emit Errors instead of Strings (David Trejo)

DTrejo added a commit that referenced this pull request Apr 30, 2012

test.js: Switch to pubsub mode when the number of channels is > 0.
Tests for a bug where the client unsubscribes
and then subscribes to a single channel. If the
subscription is sent before the response to the
unsubscribe is received, then the client would
leave pubsub mode when it received the unsubscribe
response and then fail to enter when the subsequent
subscription is processed. This is another test for #190:
#190

Signed-off-by: David Trejo <david.daniel.trejo@gmail.com>

@DTrejo DTrejo referenced this pull request Apr 30, 2012

Closed

Fix a bug in pubsub. #171

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment