Commit 6a3e96ad authored by michael-grunder's avatar michael-grunder
Browse files

Maintain backward compatibiliy withour onConnect callback.

In f69fac76, our async onConnect
callback was improved to take a non-const redisAsyncContext allowing it
to be reentrant.

Unfortunately, this is a breaking change we can't make until hiredis
v2.0.0.

This commit creates a separate callback member and corresponding
function that allows us to use the new functionality, while maintaining
our existing API for legacy code.

Fixes #1086
parent e7afd998
...@@ -372,6 +372,8 @@ the disconnect callback is a good point to do so. ...@@ -372,6 +372,8 @@ the disconnect callback is a good point to do so.
Setting the connect or disconnect callbacks can only be done once per context. For subsequent calls the Setting the connect or disconnect callbacks can only be done once per context. For subsequent calls the
api will return `REDIS_ERR`. The function to set the callbacks have the following prototype: api will return `REDIS_ERR`. The function to set the callbacks have the following prototype:
```c ```c
/* Alternatively you can use redisAsyncSetConnectCallbackNC which will be passed a non-const
redisAsyncContext* on invocation (e.g. allowing writes to the privdata member). */
int redisAsyncSetConnectCallback(redisAsyncContext *ac, redisConnectCallback *fn); int redisAsyncSetConnectCallback(redisAsyncContext *ac, redisConnectCallback *fn);
int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn); int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn);
``` ```
......
...@@ -140,6 +140,7 @@ static redisAsyncContext *redisAsyncInitialize(redisContext *c) { ...@@ -140,6 +140,7 @@ static redisAsyncContext *redisAsyncInitialize(redisContext *c) {
ac->ev.scheduleTimer = NULL; ac->ev.scheduleTimer = NULL;
ac->onConnect = NULL; ac->onConnect = NULL;
ac->onConnectNC = NULL;
ac->onDisconnect = NULL; ac->onDisconnect = NULL;
ac->replies.head = NULL; ac->replies.head = NULL;
...@@ -226,17 +227,34 @@ redisAsyncContext *redisAsyncConnectUnix(const char *path) { ...@@ -226,17 +227,34 @@ redisAsyncContext *redisAsyncConnectUnix(const char *path) {
return redisAsyncConnectWithOptions(&options); return redisAsyncConnectWithOptions(&options);
} }
int redisAsyncSetConnectCallback(redisAsyncContext *ac, redisConnectCallback *fn) { static int
if (ac->onConnect == NULL) { redisAsyncSetConnectCallbackImpl(redisAsyncContext *ac, redisConnectCallback *fn,
ac->onConnect = fn; redisConnectCallbackNC *fn_nc)
{
/* If either are already set, this is an error */
if (ac->onConnect || ac->onConnectNC)
return REDIS_ERR;
/* The common way to detect an established connection is to wait for if (fn) {
* the first write event to be fired. This assumes the related event ac->onConnect = fn;
* library functions are already set. */ } else if (fn_nc) {
_EL_ADD_WRITE(ac); ac->onConnectNC = fn_nc;
return REDIS_OK;
} }
return REDIS_ERR;
/* The common way to detect an established connection is to wait for
* the first write event to be fired. This assumes the related event
* library functions are already set. */
_EL_ADD_WRITE(ac);
return REDIS_OK;
}
int redisAsyncSetConnectCallback(redisAsyncContext *ac, redisConnectCallback *fn) {
return redisAsyncSetConnectCallbackImpl(ac, fn, NULL);
}
int redisAsyncSetConnectCallbackNC(redisAsyncContext *ac, redisConnectCallbackNC *fn) {
return redisAsyncSetConnectCallbackImpl(ac, NULL, fn);
} }
int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn) { int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn) {
...@@ -305,14 +323,23 @@ static void __redisRunPushCallback(redisAsyncContext *ac, redisReply *reply) { ...@@ -305,14 +323,23 @@ static void __redisRunPushCallback(redisAsyncContext *ac, redisReply *reply) {
static void __redisRunConnectCallback(redisAsyncContext *ac, int status) static void __redisRunConnectCallback(redisAsyncContext *ac, int status)
{ {
if (ac->onConnect) { if (ac->onConnect == NULL && ac->onConnectNC == NULL)
if (!(ac->c.flags & REDIS_IN_CALLBACK)) { return;
ac->c.flags |= REDIS_IN_CALLBACK;
if (!(ac->c.flags & REDIS_IN_CALLBACK)) {
ac->c.flags |= REDIS_IN_CALLBACK;
if (ac->onConnect) {
ac->onConnect(ac, status); ac->onConnect(ac, status);
ac->c.flags &= ~REDIS_IN_CALLBACK;
} else { } else {
/* already in callback */ ac->onConnectNC(ac, status);
}
ac->c.flags &= ~REDIS_IN_CALLBACK;
} else {
/* already in callback */
if (ac->onConnect) {
ac->onConnect(ac, status); ac->onConnect(ac, status);
} else {
ac->onConnectNC(ac, status);
} }
} }
} }
......
...@@ -57,7 +57,8 @@ typedef struct redisCallbackList { ...@@ -57,7 +57,8 @@ typedef struct redisCallbackList {
/* Connection callback prototypes */ /* Connection callback prototypes */
typedef void (redisDisconnectCallback)(const struct redisAsyncContext*, int status); typedef void (redisDisconnectCallback)(const struct redisAsyncContext*, int status);
typedef void (redisConnectCallback)(struct redisAsyncContext*, int status); typedef void (redisConnectCallback)(const struct redisAsyncContext*, int status);
typedef void (redisConnectCallbackNC)(struct redisAsyncContext *, int status);
typedef void(redisTimerCallback)(void *timer, void *privdata); typedef void(redisTimerCallback)(void *timer, void *privdata);
/* Context for an async connection to Redis */ /* Context for an async connection to Redis */
...@@ -93,6 +94,7 @@ typedef struct redisAsyncContext { ...@@ -93,6 +94,7 @@ typedef struct redisAsyncContext {
/* Called when the first write event was received. */ /* Called when the first write event was received. */
redisConnectCallback *onConnect; redisConnectCallback *onConnect;
redisConnectCallbackNC *onConnectNC;
/* Regular command callbacks */ /* Regular command callbacks */
redisCallbackList replies; redisCallbackList replies;
...@@ -121,6 +123,7 @@ redisAsyncContext *redisAsyncConnectBindWithReuse(const char *ip, int port, ...@@ -121,6 +123,7 @@ redisAsyncContext *redisAsyncConnectBindWithReuse(const char *ip, int port,
const char *source_addr); const char *source_addr);
redisAsyncContext *redisAsyncConnectUnix(const char *path); redisAsyncContext *redisAsyncConnectUnix(const char *path);
int redisAsyncSetConnectCallback(redisAsyncContext *ac, redisConnectCallback *fn); int redisAsyncSetConnectCallback(redisAsyncContext *ac, redisConnectCallback *fn);
int redisAsyncSetConnectCallbackNC(redisAsyncContext *ac, redisConnectCallbackNC *fn);
int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn); int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn);
redisAsyncPushFn *redisAsyncSetPushCallback(redisAsyncContext *ac, redisAsyncPushFn *fn); redisAsyncPushFn *redisAsyncSetPushCallback(redisAsyncContext *ac, redisAsyncPushFn *fn);
......
...@@ -2008,7 +2008,7 @@ static redisAsyncContext *do_aconnect(struct config config, astest_no testno) ...@@ -2008,7 +2008,7 @@ static redisAsyncContext *do_aconnect(struct config config, astest_no testno)
{ {
redisOptions options = {0}; redisOptions options = {0};
memset(&astest, 0, sizeof(astest)); memset(&astest, 0, sizeof(astest));
astest.testno = testno; astest.testno = testno;
astest.connect_status = astest.disconnect_status = -2; astest.connect_status = astest.disconnect_status = -2;
...@@ -2039,7 +2039,7 @@ static redisAsyncContext *do_aconnect(struct config config, astest_no testno) ...@@ -2039,7 +2039,7 @@ static redisAsyncContext *do_aconnect(struct config config, astest_no testno)
c->data = &astest; c->data = &astest;
c->dataCleanup = asCleanup; c->dataCleanup = asCleanup;
redisPollAttach(c); redisPollAttach(c);
redisAsyncSetConnectCallback(c, connectCallback); redisAsyncSetConnectCallbackNC(c, connectCallback);
redisAsyncSetDisconnectCallback(c, disconnectCallback); redisAsyncSetDisconnectCallback(c, disconnectCallback);
return c; return c;
} }
...@@ -2058,7 +2058,7 @@ static void test_async_polling(struct config config) { ...@@ -2058,7 +2058,7 @@ static void test_async_polling(struct config config) {
int status; int status;
redisAsyncContext *c; redisAsyncContext *c;
struct config defaultconfig = config; struct config defaultconfig = config;
test("Async connect: "); test("Async connect: ");
c = do_aconnect(config, ASTEST_CONNECT); c = do_aconnect(config, ASTEST_CONNECT);
assert(c); assert(c);
...@@ -2095,7 +2095,7 @@ static void test_async_polling(struct config config) { ...@@ -2095,7 +2095,7 @@ static void test_async_polling(struct config config) {
test_cond(astest.connect_status == REDIS_ERR); test_cond(astest.connect_status == REDIS_ERR);
config = defaultconfig; config = defaultconfig;
} }
/* Test a ping/pong after connection */ /* Test a ping/pong after connection */
test("Async PING/PONG: "); test("Async PING/PONG: ");
c = do_aconnect(config, ASTEST_PINGPONG); c = do_aconnect(config, ASTEST_PINGPONG);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment