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

Why does hiredis ignore my callback on UNSUBSCRIBE in RESP3? #967

Closed
stolyaroleh opened this issue Jul 16, 2021 · 2 comments · Fixed by #1012
Closed

Why does hiredis ignore my callback on UNSUBSCRIBE in RESP3? #967

stolyaroleh opened this issue Jul 16, 2021 · 2 comments · Fixed by #1012

Comments

@stolyaroleh
Copy link

When I SUBSCRIBE to a channel with a callback, hiredis calls it with subscribe and message push messages, but not with unsubscribe. Instead, hiredis would send the reply to a push handler. This makes it harder for me to track callback lifetimes in C++, since I no longer have access to void *privdata that I have attached to my subscription.

When running the following modified example-libuv (and manually publishing a message after subscribing), I was expecting the following output:

subscribed
message
unsubscribed

Instead, I get:

subscribed
message
push: unsubscribe
#include <adapters/libuv.h>
#include <async.h>
#include <hiredis.h>

void callback(redisAsyncContext *c, void *r, void *privdata) {
  redisReply *reply = r;
  char *what = reply->element[0]->str;

  if (strcmp(what, "subscribe") == 0) {
    printf("subscribed\n");
    return;
  }

  if (strcmp(what, "message") == 0) {
    printf("message\n");
    redisAsyncCommand(c, NULL, NULL, "UNSUBSCRIBE");
    return;
  }

  if (strcmp(what, "unsubscribe") == 0) {
    printf("unsubscribed\n");
    redisAsyncFree(c);
  }
}

void pushCallback(redisAsyncContext *c, void *r) {
  redisReply *reply = r;
  char *what = reply->element[0]->str;
  printf("push: %s\n", what);
  redisAsyncFree(c);
}

int main(int argc, char **argv) {
  redisAsyncContext *c = redisAsyncConnect("127.0.0.1", 6379);

  if (c->err) {
    printf("Error: %s\n", c->errstr);
    return 1;
  }

  uv_loop_t *loop = uv_default_loop();
  redisLibuvAttach(c, loop);

  redisAsyncSetPushCallback(c, pushCallback);
  redisAsyncCommand(c, NULL, NULL, "HELLO 3");
  redisAsyncCommand(c, callback, NULL, "SUBSCRIBE foo");

  uv_run(loop, UV_RUN_DEFAULT);
  return 0;
}

It appears that this can be fixed with a one line patch:

--- a/async.c
+++ b/async.c
@@ -496,7 +496,8 @@ static int redisIsSubscribeReply(redisReply *reply) {
     len = reply->element[0]->len - off;
 
     return !strncasecmp(str, "subscribe", len) ||
-           !strncasecmp(str, "message", len);
+           !strncasecmp(str, "message", len) ||
+           !strncasecmp(str, "unsubscribe", len);
 
 }

Modified hiredis passes all unit tests and produces the output I expected.

@stolyaroleh
Copy link
Author

Probably related: #738
Would you be open to a PR with the change above?

@bjosv
Copy link
Contributor

bjosv commented Oct 13, 2021

We have the same problem and I will try-out your proposal a bit, or are there any new revelations regarding this issue?

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.

2 participants