add a command_timeout to redisContextOptions (#839)

Add an additional timeout so the user has a convenient way of controlling distinct connect and command timeouts

7 files changed, 80 insertions, 42 deletions

diff --git a/async.c b/async.c
index 1073d8d..020cb09 100644
--- a/async.c
+++ b/async.c
@@ -868,19 +868,19 @@ redisAsyncPushFn *redisAsyncSetPushCallback(redisAsyncContext *ac, redisAsyncPus
 }
 
 int redisAsyncSetTimeout(redisAsyncContext *ac, struct timeval tv) {
-    if (!ac->c.timeout) {
-        ac->c.timeout = hi_calloc(1, sizeof(tv));
-        if (ac->c.timeout == NULL) {
+    if (!ac->c.command_timeout) {
+        ac->c.command_timeout = hi_calloc(1, sizeof(tv));
+        if (ac->c.command_timeout == NULL) {
             __redisSetError(&ac->c, REDIS_ERR_OOM, "Out of memory");
             __redisAsyncCopyError(ac);
             return REDIS_ERR;
         }
     }
 
-    if (tv.tv_sec != ac->c.timeout->tv_sec ||
-        tv.tv_usec != ac->c.timeout->tv_usec)
+    if (tv.tv_sec != ac->c.command_timeout->tv_sec ||
+        tv.tv_usec != ac->c.command_timeout->tv_usec)
     {
-        *ac->c.timeout = tv;
+        *ac->c.command_timeout = tv;
     }
 
     return REDIS_OK;
diff --git a/async_private.h b/async_private.h
index d0133ae..e4a22ab 100644
--- a/async_private.h
+++ b/async_private.h
@@ -54,15 +54,14 @@
     } while(0);
 
 static inline void refreshTimeout(redisAsyncContext *ctx) {
-    if (ctx->c.timeout && ctx->ev.scheduleTimer &&
-        (ctx->c.timeout->tv_sec || ctx->c.timeout->tv_usec)) {
-        ctx->ev.scheduleTimer(ctx->ev.data, *ctx->c.timeout);
-    // } else {
-    //     printf("Not scheduling timer.. (tmo=%p)\n", ctx->c.timeout);
-    //     if (ctx->c.timeout){
-    //         printf("tv_sec: %u. tv_usec: %u\n", ctx->c.timeout->tv_sec,
-    //                ctx->c.timeout->tv_usec);
-    //     }
+    if (!(ctx->c.flags & REDIS_CONNECTED)) {
+        if (ctx->c.connect_timeout && ctx->ev.scheduleTimer &&
+            (ctx->c.connect_timeout->tv_sec || ctx->c.connect_timeout->tv_usec)) {
+            ctx->ev.scheduleTimer(ctx->ev.data, *ctx->c.connect_timeout);
+        }
+    } else if (ctx->c.command_timeout && ctx->ev.scheduleTimer &&
+               (ctx->c.command_timeout->tv_sec || ctx->c.command_timeout->tv_usec)) {
+            ctx->ev.scheduleTimer(ctx->ev.data, *ctx->c.command_timeout);
     }
 }
 
diff --git a/examples/example-libevent.c b/examples/example-libevent.c
index f3fa2a6..49bddd0 100644
--- a/examples/example-libevent.c
+++ b/examples/example-libevent.c
@@ -47,7 +47,7 @@ int main (int argc, char **argv) {
     REDIS_OPTIONS_SET_TCP(&options, "127.0.0.1", 6379);
     struct timeval tv = {0};
     tv.tv_sec = 1;
-    options.timeout = &tv;
+    options.connect_timeout = &tv;
 
 
     redisAsyncContext *c = redisAsyncConnectWithOptions(&options);
diff --git a/examples/example-ssl.c b/examples/example-ssl.c
index 5eb2bbb..c754177 100644
--- a/examples/example-ssl.c
+++ b/examples/example-ssl.c
@@ -33,7 +33,7 @@ int main(int argc, char **argv) {
     struct timeval tv = { 1, 500000 }; // 1.5 seconds
     redisOptions options = {0};
     REDIS_OPTIONS_SET_TCP(&options, hostname, port);
-    options.timeout = &tv;
+    options.connect_timeout = &tv;
     c = redisConnectWithOptions(&options);
 
     if (c == NULL || c->err) {
diff --git a/hiredis.c b/hiredis.c
index e9761ec..8d72d35 100644
--- a/hiredis.c
+++ b/hiredis.c
@@ -44,6 +44,9 @@
 #include "async.h"
 #include "win32.h"
 
+extern int redisContextUpdateConnectTimeout(redisContext *c, const struct timeval *timeout);
+extern int redisContextUpdateCommandTimeout(redisContext *c, const struct timeval *timeout);
+
 static redisContextFuncs redisContextDefaultFuncs = {
     .free_privdata = NULL,
     .async_read = redisAsyncRead,
@@ -723,7 +726,8 @@ void redisFree(redisContext *c) {
     hi_free(c->tcp.host);
     hi_free(c->tcp.source_addr);
     hi_free(c->unix_sock.path);
-    hi_free(c->timeout);
+    hi_free(c->connect_timeout);
+    hi_free(c->command_timeout);
     hi_free(c->saddr);
     if (c->funcs->free_privdata) {
         c->funcs->free_privdata(c->privdata);
@@ -761,18 +765,24 @@ int redisReconnect(redisContext *c) {
         return REDIS_ERR;
     }
 
+    int ret = REDIS_ERR;
     if (c->connection_type == REDIS_CONN_TCP) {
-        return redisContextConnectBindTcp(c, c->tcp.host, c->tcp.port,
-                c->timeout, c->tcp.source_addr);
+        ret = redisContextConnectBindTcp(c, c->tcp.host, c->tcp.port,
+               c->connect_timeout, c->tcp.source_addr);
     } else if (c->connection_type == REDIS_CONN_UNIX) {
-        return redisContextConnectUnix(c, c->unix_sock.path, c->timeout);
+        ret = redisContextConnectUnix(c, c->unix_sock.path, c->connect_timeout);
     } else {
         /* Something bad happened here and shouldn't have. There isn't
            enough information in the context to reconnect. */
         __redisSetError(c,REDIS_ERR_OTHER,"Not enough information to reconnect");
+        ret = REDIS_ERR;
     }
 
-    return REDIS_ERR;
+    if (c->command_timeout != NULL && (c->flags & REDIS_BLOCK) && c->fd != REDIS_INVALID_FD) {
+        redisContextSetTimeout(c, *c->command_timeout);
+    }
+
+    return ret;
 }
 
 redisContext *redisConnectWithOptions(const redisOptions *options) {
@@ -790,13 +800,19 @@ redisContext *redisConnectWithOptions(const redisOptions *options) {
         c->flags |= REDIS_NO_AUTO_FREE;
     }
 
+    if (redisContextUpdateConnectTimeout(c, options->connect_timeout) != REDIS_OK ||
+        redisContextUpdateCommandTimeout(c, options->command_timeout) != REDIS_OK) {
+        __redisSetError(c, REDIS_ERR_OOM, "Out of memory");
+        return c;
+    }
+
     if (options->type == REDIS_CONN_TCP) {
         redisContextConnectBindTcp(c, options->endpoint.tcp.ip,
-                                   options->endpoint.tcp.port, options->timeout,
+                                   options->endpoint.tcp.port, options->connect_timeout,
                                    options->endpoint.tcp.source_addr);
     } else if (options->type == REDIS_CONN_UNIX) {
         redisContextConnectUnix(c, options->endpoint.unix_socket,
-                                options->timeout);
+                                options->connect_timeout);
     } else if (options->type == REDIS_CONN_USERFD) {
         c->fd = options->endpoint.fd;
         c->flags |= REDIS_CONNECTED;
@@ -805,6 +821,10 @@ redisContext *redisConnectWithOptions(const redisOptions *options) {
         return NULL;
     }
 
+    if (options->command_timeout != NULL && (c->flags & REDIS_BLOCK) && c->fd != REDIS_INVALID_FD) {
+        redisContextSetTimeout(c, *options->command_timeout);
+    }
+
     return c;
 }
 
@@ -820,7 +840,7 @@ redisContext *redisConnect(const char *ip, int port) {
 redisContext *redisConnectWithTimeout(const char *ip, int port, const struct timeval tv) {
     redisOptions options = {0};
     REDIS_OPTIONS_SET_TCP(&options, ip, port);
-    options.timeout = &tv;
+    options.connect_timeout = &tv;
     return redisConnectWithOptions(&options);
 }
 
@@ -858,7 +878,7 @@ redisContext *redisConnectUnix(const char *path) {
 redisContext *redisConnectUnixWithTimeout(const char *path, const struct timeval tv) {
     redisOptions options = {0};
     REDIS_OPTIONS_SET_UNIX(&options, path);
-    options.timeout = &tv;
+    options.connect_timeout = &tv;
     return redisConnectWithOptions(&options);
 }
 
diff --git a/hiredis.h b/hiredis.h
index 20e9bfa..fe2dae4 100644
--- a/hiredis.h
+++ b/hiredis.h
@@ -176,8 +176,10 @@ typedef struct {
     int type;
     /* bit field of REDIS_OPT_xxx */
     int options;
-    /* timeout value. if NULL, no timeout is used */
-    const struct timeval *timeout;
+    /* timeout value for connect operation. if NULL, no timeout is used */
+    const struct timeval *connect_timeout;
+    /* timeout value for commands. if NULL, no timeout is used. (can be set later on with redisSetTimeout/redisAsyncSetTimeout) */
+    const struct timeval *command_timeout;
     union {
         /** use this field for tcp/ip connections */
         struct {
@@ -230,7 +232,8 @@ typedef struct redisContext {
     redisReader *reader; /* Protocol reader */
 
     enum redisConnectionType connection_type;
-    struct timeval *timeout;
+    struct timeval *connect_timeout;
+    struct timeval *command_timeout;
 
     struct {
         char *host;
diff --git a/net.c b/net.c
index 882e2c4..c6b0e5d 100644
--- a/net.c
+++ b/net.c
@@ -217,7 +217,7 @@ int redisSetTcpNoDelay(redisContext *c) {
 
 static int redisContextTimeoutMsec(redisContext *c, long *result)
 {
-    const struct timeval *timeout = c->timeout;
+    const struct timeval *timeout = c->connect_timeout;
     long msec = -1;
 
     /* Only use timeout when not NULL. */
@@ -328,19 +328,35 @@ int redisContextSetTimeout(redisContext *c, const struct timeval tv) {
     return REDIS_OK;
 }
 
-static int _redisContextUpdateTimeout(redisContext *c, const struct timeval *timeout) {
+int redisContextUpdateConnectTimeout(redisContext *c, const struct timeval *timeout) {
     /* Same timeval struct, short circuit */
-    if (c->timeout == timeout)
+    if (c->connect_timeout == timeout)
         return REDIS_OK;
 
     /* Allocate context timeval if we need to */
-    if (c->timeout == NULL) {
-        c->timeout = hi_malloc(sizeof(*c->timeout));
-        if (c->timeout == NULL)
+    if (c->connect_timeout == NULL) {
+        c->connect_timeout = hi_malloc(sizeof(*c->connect_timeout));
+        if (c->connect_timeout == NULL)
             return REDIS_ERR;
     }
 
-    memcpy(c->timeout, timeout, sizeof(*c->timeout));
+    memcpy(c->connect_timeout, timeout, sizeof(*c->connect_timeout));
+    return REDIS_OK;
+}
+
+int redisContextUpdateCommandTimeout(redisContext *c, const struct timeval *timeout) {
+    /* Same timeval struct, short circuit */
+    if (c->command_timeout == timeout)
+        return REDIS_OK;
+
+    /* Allocate context timeval if we need to */
+    if (c->command_timeout == NULL) {
+        c->command_timeout = hi_malloc(sizeof(*c->command_timeout));
+        if (c->command_timeout == NULL)
+            return REDIS_ERR;
+    }
+
+    memcpy(c->command_timeout, timeout, sizeof(*c->command_timeout));
     return REDIS_OK;
 }
 
@@ -376,11 +392,11 @@ static int _redisContextConnectTcp(redisContext *c, const char *addr, int port,
     }
 
     if (timeout) {
-        if (_redisContextUpdateTimeout(c, timeout) == REDIS_ERR)
+        if (redisContextUpdateConnectTimeout(c, timeout) == REDIS_ERR)
             goto oom;
     } else {
-        hi_free(c->timeout);
-        c->timeout = NULL;
+        hi_free(c->connect_timeout);
+        c->connect_timeout = NULL;
     }
 
     if (redisContextTimeoutMsec(c, &timeout_msec) != REDIS_OK) {
@@ -549,11 +565,11 @@ int redisContextConnectUnix(redisContext *c, const char *path, const struct time
     }
 
     if (timeout) {
-        if (_redisContextUpdateTimeout(c, timeout) == REDIS_ERR)
+        if (redisContextUpdateConnectTimeout(c, timeout) == REDIS_ERR)
             goto oom;
     } else {
-        hi_free(c->timeout);
-        c->timeout = NULL;
+        hi_free(c->connect_timeout);
+        c->connect_timeout = NULL;
     }
 
     if (redisContextTimeoutMsec(c,&timeout_msec) != REDIS_OK)