Unverified Commit 67036ef7 authored by Mark Nunberg's avatar Mark Nunberg Committed by GitHub
Browse files

Merge pull request #578 from mnunberg/connfix

Proper error reporting for connect failures
parents 747d78be 3cb4fb23
...@@ -506,22 +506,22 @@ void redisProcessCallbacks(redisAsyncContext *ac) { ...@@ -506,22 +506,22 @@ void redisProcessCallbacks(redisAsyncContext *ac) {
* write event fires. When connecting was not successful, the connect callback * write event fires. When connecting was not successful, the connect callback
* is called with a REDIS_ERR status and the context is free'd. */ * is called with a REDIS_ERR status and the context is free'd. */
static int __redisAsyncHandleConnect(redisAsyncContext *ac) { static int __redisAsyncHandleConnect(redisAsyncContext *ac) {
int completed = 0;
redisContext *c = &(ac->c); redisContext *c = &(ac->c);
if (redisCheckConnectDone(c, &completed) == REDIS_ERR) {
if (redisCheckSocketError(c) == REDIS_ERR) { /* Error! */
/* Try again later when connect(2) is still in progress. */ redisCheckSocketError(c);
if (errno == EINPROGRESS) if (ac->onConnect) ac->onConnect(ac, REDIS_ERR);
return REDIS_OK;
if (ac->onConnect) ac->onConnect(ac,REDIS_ERR);
__redisAsyncDisconnect(ac); __redisAsyncDisconnect(ac);
return REDIS_ERR; return REDIS_ERR;
} else if (completed == 1) {
/* connected! */
if (ac->onConnect) ac->onConnect(ac, REDIS_OK);
c->flags |= REDIS_CONNECTED;
return REDIS_OK;
} else {
return REDIS_OK;
} }
/* Mark context as connected. */
c->flags |= REDIS_CONNECTED;
if (ac->onConnect) ac->onConnect(ac,REDIS_OK);
return REDIS_OK;
} }
/* This function should be called when the socket is readable. /* This function should be called when the socket is readable.
......
...@@ -93,6 +93,10 @@ typedef struct redisAsyncContext { ...@@ -93,6 +93,10 @@ typedef struct redisAsyncContext {
/* Regular command callbacks */ /* Regular command callbacks */
redisCallbackList replies; redisCallbackList replies;
/* Address used for connect() */
struct sockaddr *saddr;
size_t addrlen;
/* Subscription callbacks */ /* Subscription callbacks */
struct { struct {
redisCallbackList invalid; redisCallbackList invalid;
......
...@@ -606,12 +606,14 @@ void redisFree(redisContext *c) { ...@@ -606,12 +606,14 @@ void redisFree(redisContext *c) {
return; return;
if (c->fd > 0) if (c->fd > 0)
close(c->fd); close(c->fd);
sdsfree(c->obuf); sdsfree(c->obuf);
redisReaderFree(c->reader); redisReaderFree(c->reader);
free(c->tcp.host); free(c->tcp.host);
free(c->tcp.source_addr); free(c->tcp.source_addr);
free(c->unix_sock.path); free(c->unix_sock.path);
free(c->timeout); free(c->timeout);
free(c->saddr);
free(c); free(c);
} }
......
...@@ -134,6 +134,9 @@ typedef struct redisContext { ...@@ -134,6 +134,9 @@ typedef struct redisContext {
char *path; char *path;
} unix_sock; } unix_sock;
/* For non-blocking connect */
struct sockadr *saddr;
size_t addrlen;
} redisContext; } redisContext;
redisContext *redisConnect(const char *ip, int port); redisContext *redisConnect(const char *ip, int port);
......
...@@ -221,8 +221,10 @@ static int redisContextWaitReady(redisContext *c, long msec) { ...@@ -221,8 +221,10 @@ static int redisContextWaitReady(redisContext *c, long msec) {
return REDIS_ERR; return REDIS_ERR;
} }
if (redisCheckSocketError(c) != REDIS_OK) if (redisCheckConnectDone(c, &res) != REDIS_OK || res == 0) {
redisCheckSocketError(c);
return REDIS_ERR; return REDIS_ERR;
}
return REDIS_OK; return REDIS_OK;
} }
...@@ -232,8 +234,28 @@ static int redisContextWaitReady(redisContext *c, long msec) { ...@@ -232,8 +234,28 @@ static int redisContextWaitReady(redisContext *c, long msec) {
return REDIS_ERR; return REDIS_ERR;
} }
int redisCheckConnectDone(redisContext *c, int *completed) {
int rc = connect(c->fd, (const struct sockaddr *)c->saddr, c->addrlen);
if (rc == 0) {
*completed = 1;
return REDIS_OK;
}
switch (errno) {
case EISCONN:
*completed = 1;
return REDIS_OK;
case EALREADY:
case EINPROGRESS:
case EWOULDBLOCK:
*completed = 0;
return REDIS_OK;
default:
return REDIS_ERR;
}
}
int redisCheckSocketError(redisContext *c) { int redisCheckSocketError(redisContext *c) {
int err = 0; int err = 0, errno_saved = errno;
socklen_t errlen = sizeof(err); socklen_t errlen = sizeof(err);
if (getsockopt(c->fd, SOL_SOCKET, SO_ERROR, &err, &errlen) == -1) { if (getsockopt(c->fd, SOL_SOCKET, SO_ERROR, &err, &errlen) == -1) {
...@@ -241,6 +263,10 @@ int redisCheckSocketError(redisContext *c) { ...@@ -241,6 +263,10 @@ int redisCheckSocketError(redisContext *c) {
return REDIS_ERR; return REDIS_ERR;
} }
if (err == 0) {
err = errno_saved;
}
if (err) { if (err) {
errno = err; errno = err;
__redisSetErrorFromErrno(c,REDIS_ERR_IO,NULL); __redisSetErrorFromErrno(c,REDIS_ERR_IO,NULL);
...@@ -373,12 +399,27 @@ addrretry: ...@@ -373,12 +399,27 @@ addrretry:
goto error; goto error;
} }
} }
/* For repeat connection */
if (c->saddr) {
free(c->saddr);
}
c->saddr = malloc(p->ai_addrlen);
memcpy(c->saddr, p->ai_addr, p->ai_addrlen);
c->addrlen = p->ai_addrlen;
if (connect(s,p->ai_addr,p->ai_addrlen) == -1) { if (connect(s,p->ai_addr,p->ai_addrlen) == -1) {
if (errno == EHOSTUNREACH) { if (errno == EHOSTUNREACH) {
redisContextCloseFd(c); redisContextCloseFd(c);
continue; continue;
} else if (errno == EINPROGRESS && !blocking) { } else if (errno == EINPROGRESS) {
/* This is ok. */ if (blocking) {
goto wait_for_ready;
}
/* This is ok.
* Note that even when it's in blocking mode, we unset blocking
* for `connect()`
*/
} else if (errno == EADDRNOTAVAIL && reuseaddr) { } else if (errno == EADDRNOTAVAIL && reuseaddr) {
if (++reuses >= REDIS_CONNECT_RETRIES) { if (++reuses >= REDIS_CONNECT_RETRIES) {
goto error; goto error;
...@@ -387,6 +428,7 @@ addrretry: ...@@ -387,6 +428,7 @@ addrretry:
goto addrretry; goto addrretry;
} }
} else { } else {
wait_for_ready:
if (redisContextWaitReady(c,timeout_msec) != REDIS_OK) if (redisContextWaitReady(c,timeout_msec) != REDIS_OK)
goto error; goto error;
} }
......
...@@ -45,5 +45,6 @@ int redisContextConnectBindTcp(redisContext *c, const char *addr, int port, ...@@ -45,5 +45,6 @@ int redisContextConnectBindTcp(redisContext *c, const char *addr, int port,
const char *source_addr); const char *source_addr);
int redisContextConnectUnix(redisContext *c, const char *path, const struct timeval *timeout); int redisContextConnectUnix(redisContext *c, const char *path, const struct timeval *timeout);
int redisKeepAlive(redisContext *c, int interval); int redisKeepAlive(redisContext *c, int interval);
int redisCheckConnectDone(redisContext *c, int *completed);
#endif #endif
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include <string.h> #include <string.h>
#include <strings.h> #include <strings.h>
#include <sys/time.h> #include <sys/time.h>
#include <netdb.h>
#include <assert.h> #include <assert.h>
#include <unistd.h> #include <unistd.h>
#include <signal.h> #include <signal.h>
...@@ -91,7 +92,7 @@ static int disconnect(redisContext *c, int keep_fd) { ...@@ -91,7 +92,7 @@ static int disconnect(redisContext *c, int keep_fd) {
return -1; return -1;
} }
static redisContext *connect(struct config config) { static redisContext *do_connect(struct config config) {
redisContext *c = NULL; redisContext *c = NULL;
if (config.type == CONN_TCP) { if (config.type == CONN_TCP) {
...@@ -248,7 +249,7 @@ static void test_append_formatted_commands(struct config config) { ...@@ -248,7 +249,7 @@ static void test_append_formatted_commands(struct config config) {
char *cmd; char *cmd;
int len; int len;
c = connect(config); c = do_connect(config);
test("Append format command: "); test("Append format command: ");
...@@ -434,18 +435,32 @@ static void test_free_null(void) { ...@@ -434,18 +435,32 @@ static void test_free_null(void) {
static void test_blocking_connection_errors(void) { static void test_blocking_connection_errors(void) {
redisContext *c; redisContext *c;
struct addrinfo hints = {.ai_family = AF_INET};
test("Returns error when host cannot be resolved: "); struct addrinfo *ai_tmp = NULL;
c = redisConnect((char*)"idontexist.test", 6379); const char *bad_domain = "idontexist.com";
test_cond(c->err == REDIS_ERR_OTHER &&
(strcmp(c->errstr,"Name or service not known") == 0 || int rv = getaddrinfo(bad_domain, "6379", &hints, &ai_tmp);
strcmp(c->errstr,"Can't resolve: idontexist.test") == 0 || if (rv != 0) {
strcmp(c->errstr,"nodename nor servname provided, or not known") == 0 || // Address does *not* exist
strcmp(c->errstr,"No address associated with hostname") == 0 || test("Returns error when host cannot be resolved: ");
strcmp(c->errstr,"Temporary failure in name resolution") == 0 || // First see if this domain name *actually* resolves to NXDOMAIN
strcmp(c->errstr,"hostname nor servname provided, or not known") == 0 || c = redisConnect("dontexist.com", 6379);
strcmp(c->errstr,"no address associated with name") == 0)); test_cond(
redisFree(c); c->err == REDIS_ERR_OTHER &&
(strcmp(c->errstr, "Name or service not known") == 0 ||
strcmp(c->errstr, "Can't resolve: sadkfjaskfjsa.com") == 0 ||
strcmp(c->errstr,
"nodename nor servname provided, or not known") == 0 ||
strcmp(c->errstr, "No address associated with hostname") == 0 ||
strcmp(c->errstr, "Temporary failure in name resolution") == 0 ||
strcmp(c->errstr,
"hostname nor servname provided, or not known") == 0 ||
strcmp(c->errstr, "no address associated with name") == 0));
redisFree(c);
} else {
printf("Skipping NXDOMAIN test. Found evil ISP!\n");
freeaddrinfo(ai_tmp);
}
test("Returns error when the port is not open: "); test("Returns error when the port is not open: ");
c = redisConnect((char*)"localhost", 1); c = redisConnect((char*)"localhost", 1);
...@@ -463,7 +478,7 @@ static void test_blocking_connection(struct config config) { ...@@ -463,7 +478,7 @@ static void test_blocking_connection(struct config config) {
redisContext *c; redisContext *c;
redisReply *reply; redisReply *reply;
c = connect(config); c = do_connect(config);
test("Is able to deliver commands: "); test("Is able to deliver commands: ");
reply = redisCommand(c,"PING"); reply = redisCommand(c,"PING");
...@@ -544,7 +559,7 @@ static void test_blocking_connection_timeouts(struct config config) { ...@@ -544,7 +559,7 @@ static void test_blocking_connection_timeouts(struct config config) {
const char *cmd = "DEBUG SLEEP 3\r\n"; const char *cmd = "DEBUG SLEEP 3\r\n";
struct timeval tv; struct timeval tv;
c = connect(config); c = do_connect(config);
test("Successfully completes a command when the timeout is not exceeded: "); test("Successfully completes a command when the timeout is not exceeded: ");
reply = redisCommand(c,"SET foo fast"); reply = redisCommand(c,"SET foo fast");
freeReplyObject(reply); freeReplyObject(reply);
...@@ -556,7 +571,7 @@ static void test_blocking_connection_timeouts(struct config config) { ...@@ -556,7 +571,7 @@ static void test_blocking_connection_timeouts(struct config config) {
freeReplyObject(reply); freeReplyObject(reply);
disconnect(c, 0); disconnect(c, 0);
c = connect(config); c = do_connect(config);
test("Does not return a reply when the command times out: "); test("Does not return a reply when the command times out: ");
s = write(c->fd, cmd, strlen(cmd)); s = write(c->fd, cmd, strlen(cmd));
tv.tv_sec = 0; tv.tv_sec = 0;
...@@ -590,7 +605,7 @@ static void test_blocking_io_errors(struct config config) { ...@@ -590,7 +605,7 @@ static void test_blocking_io_errors(struct config config) {
int major, minor; int major, minor;
/* Connect to target given by config. */ /* Connect to target given by config. */
c = connect(config); c = do_connect(config);
{ {
/* Find out Redis version to determine the path for the next test */ /* Find out Redis version to determine the path for the next test */
const char *field = "redis_version:"; const char *field = "redis_version:";
...@@ -625,7 +640,7 @@ static void test_blocking_io_errors(struct config config) { ...@@ -625,7 +640,7 @@ static void test_blocking_io_errors(struct config config) {
strcmp(c->errstr,"Server closed the connection") == 0); strcmp(c->errstr,"Server closed the connection") == 0);
redisFree(c); redisFree(c);
c = connect(config); c = do_connect(config);
test("Returns I/O error on socket timeout: "); test("Returns I/O error on socket timeout: ");
struct timeval tv = { 0, 1000 }; struct timeval tv = { 0, 1000 };
assert(redisSetTimeout(c,tv) == REDIS_OK); assert(redisSetTimeout(c,tv) == REDIS_OK);
...@@ -659,7 +674,7 @@ static void test_invalid_timeout_errors(struct config config) { ...@@ -659,7 +674,7 @@ static void test_invalid_timeout_errors(struct config config) {
} }
static void test_throughput(struct config config) { static void test_throughput(struct config config) {
redisContext *c = connect(config); redisContext *c = do_connect(config);
redisReply **replies; redisReply **replies;
int i, num; int i, num;
long long t1, t2; long long t1, t2;
......
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