Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PING not allowed while subscribing #351

Closed
michael-grunder opened this issue Aug 6, 2015 · 2 comments · Fixed by #1027
Closed

PING not allowed while subscribing #351

michael-grunder opened this issue Aug 6, 2015 · 2 comments · Fixed by #1027
Assignees

Comments

@michael-grunder
Copy link
Collaborator

This is in reference to redis/redis#420, specifically redis/redis#420 (comment)

The reason hiredis silently ignores the ping response is because it stores subscribe and psubscribe callbacks in separate dictionaries in order to deliver messages to the correct handler.

It looks for a redis publish message, which can take the following forms (with pong being new):

SUBSCRIBE PSUBSCRIBE PONG
1) "message" 1) pmessage 1) pong
2) "foo" 2) "foo*" 2) pongarg
3) "bar" 3) "foo"
4) "bar"

Hiredis is only accounting for message or pmessage, and specifically just checks the first character of the first multibulk reply string to determine which one it is, which is done here. Given that pong starts with p it looks in the psubscribe dict.

It uses the second argument (which for subscribe or psubscribe will be the channel/pattern) to search, but in the case of pong can now be literally anything.

You can actually "trick" hiredis into executing your callback by doing something like this:

int main (int argc, char **argv) {
    signal(SIGPIPE, SIG_IGN);

    redisAsyncContext *c = redisAsyncConnect("127.0.0.1", 6379);

    redisLibevAttach(EV_DEFAULT_ c);
    redisAsyncSetConnectCallback(c,connectCallback);
    redisAsyncSetDisconnectCallback(c,disconnectCallback);

    redisAsyncCommand(c, subscribeCallback, NULL, "PSUBSCRIBE pong*");

    ev_loop(EV_DEFAULT_ 0);
    return 0;
}
void subscribeCallback(redisAsyncContext *c, void *r, void *privdata) {
    printf("In subscribeCallback...\n");

    redisReply *reply = r;
    int i;

    if (reply == NULL) return;

    for (i = 0; i < reply->elements; i++) {
        if (reply->element[i]->type == REDIS_REPLY_STRING) {
            printf("%i: %s\n", i, reply->element[i]->str);
        }
    }
    redisAsyncCommand(c, subscribeCallback, NULL, "PING pong*");
}

This is obviously not a solution of any kind but is what's going on. Hiredis could be made to handle the new message types by detecting "pong" specifically, and using the argument as either the channel name or pattern. This would mean that you couldn't add any other kind of context to the ping, but would allow you to use it as a mechanism to detect the connection was still live.

@echoma
Copy link

echoma commented Sep 16, 2015

This works but a little buggy.
When the connection closed, subscribeCallback() was called many many times with r=NULL. It seems that hiredis remembers all "PING pong*" command and called them when close. I am afraid that hiredis will allocate more memory for the remember work.

@echoma
Copy link

echoma commented Sep 16, 2015

I tried "PSUBSCRIBE pong_" instead of "PING pong_", it works fine in subscribe mode.
And in fact, you can subscribe anything as PING, I use tcpdump and can see network data when subsctibe again and again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants