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

redisDisconnectCallback is not called while disconnecting asynchronous connection to redis #1163

Open
zh1029 opened this issue Feb 2, 2023 · 0 comments

Comments

@zh1029
Copy link

zh1029 commented Feb 2, 2023

hiredis version: V1.1-0.

Uses redisAsyncConnectWithOptions() function with option has REDIS_OPT_PREFER_IPV4 as below code for example.

_struct event_base *base = event_base_new();
redisOptions options = {0};
REDIS_OPTIONS_SET_TCP(&options, "127.0.0.1", 6379);
struct timeval tv = {0};
tv.tv_sec = 1;
options.connect_timeout = &tv;
opt.options |= REDIS_OPT_PREFER_IPV4;

redisAsyncContext *c = redisAsyncConnectWithOptions(&options);
if (c->err) {
    /* Let *c leak for now... */
    printf("Error: %s\n", c->errstr);
    return 1;
}

redisLibeventAttach(c,base);
redisAsyncSetConnectCallback(c,connectCallback);
redisAsyncSetDisconnectCallback(c,disconnectCallback);
redisAsyncCommand(c, NULL, NULL, "SET key %b", argv[argc-1], strlen(argv[argc-1]));
redisAsyncCommand(c, getCallback, (char*)"end-1", "GET key");_

It lead to no calling disconnectCallback while the asynchronous connection disconnecting happened.
It is a bug in hiredis due to duplicated constants value between REDIS_OPT_PREFER_IPV4 and REDIS_OPT_NOAUTOFREE as below definition in hiredis.h

#define REDIS_OPT_PREFER_IPV4 0x04
#define REDIS_OPT_NOAUTOFREE 0x04

It cause logical error in redisConnectWithOptions() that mistaking set the context flag REDIS_NO_AUTO_FREE which is not as expected.

redisContext *redisConnectWithOptions(const redisOptions *options) {
redisContext *c = redisContextInit();
if (c == NULL) {
return NULL;
}
if (!(options->options & REDIS_OPT_NONBLOCK)) {
c->flags |= REDIS_BLOCK;
}
if (options->options & REDIS_OPT_REUSEADDR) {
c->flags |= REDIS_REUSEADDR;
}
if (options->options & REDIS_OPT_NOAUTOFREE) {
c->flags |= REDIS_NO_AUTO_FREE;
}

if (options->options & REDIS_OPT_NOAUTOFREEREPLIES) {
c->flags |= REDIS_NO_AUTO_FREE_REPLIES;
}
if (options->options & REDIS_OPT_PREFER_IPV4) {
c->flags |= REDIS_PREFER_IPV4;
}
if (options->options & REDIS_OPT_PREFER_IPV6) {
c->flags |= REDIS_PREFER_IPV6;
}

And eventually below __redisAsyncFree() is not invoked.

_/* Helper function to make the disconnect happen and clean up. */
void __redisAsyncDisconnect(redisAsyncContext ac) {
......
/
For non-clean disconnects, __redisAsyncFree() will execute pending
* callbacks with a NULL-reply. */
if (!(c->flags & REDIS_NO_AUTO_FREE)) {
_redisAsyncFree(ac);
}
......
}

Same duplicated constants value need to fix as well:
#define REDIS_OPT_PREFER_IPV6 0x08
#define REDIS_OPT_NO_PUSH_AUTOFREE 0x08

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

No branches or pull requests

1 participant