From 09ef521d2c88955d04d6c91f7b5a1671a1955130 Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Mon, 28 Jun 2010 17:05:29 -0700 Subject: [PATCH] Fix for interface issues (Bug 571909 <571909@bugs.launchpad.net>) --- ChangeLog | 1 + libmemcached/connect.c | 206 +++++++++++++++++++++++++---------------- tests/mem_functions.c | 16 ++-- 3 files changed, 135 insertions(+), 88 deletions(-) diff --git a/ChangeLog b/ChangeLog index 13716ca0..608d492b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,7 @@ * Added limemcached_ping() to libmemcached_util * Bugfix for some cases where connect would have issues with timeout. * Wrong value for errno given as error on an IO failure inside of poll. + * Bug fix for issue where multiple interfaces with bad DNS were not being caught. 0.40 Thu Apr 22 19:01:25 PDT 2010 * Placed retry logic in for busted resolvers diff --git a/libmemcached/connect.c b/libmemcached/connect.c index a43de956..3002727d 100644 --- a/libmemcached/connect.c +++ b/libmemcached/connect.c @@ -1,9 +1,96 @@ +/* LibMemcached + * Copyright (C) 2006-2010 Brian Aker + * All rights reserved. + * + * Use and distribution licensed under the BSD license. See + * the COPYING file in the parent directory for full text. + * + * Summary: Server IO, Not public! + * + */ + #include "common.h" #include #include #include #include +static memcached_return_t connect_poll(memcached_server_st *ptr) +{ + struct pollfd fds[1]; + fds[0].fd = ptr->fd; + fds[0].events = POLLOUT; + + int timeout= ptr->root->connect_timeout; + if (ptr->root->flags.no_block == true) + timeout= -1; + + int error; + size_t loop_max= 5; + + while (--loop_max) // Should only loop on cases of ERESTART or EINTR + { + error= poll(fds, 1, timeout); + + switch (error) + { + case 1: + { + int err; + socklen_t len= sizeof (err); + (void)getsockopt(ptr->fd, SOL_SOCKET, SO_ERROR, &err, &len); + + // We check the value to see what happened wth the socket. + if (err == 0) + { + return MEMCACHED_SUCCESS; + } + else + { + ptr->cached_errno= errno; + + return MEMCACHED_ERRNO; + } + } + case 0: + return MEMCACHED_TIMEOUT; + default: // A real error occurred and we need to completely bail + WATCHPOINT_ERRNO(errno); + switch (errno) + { +#ifdef TARGET_OS_LINUX + case ERESTART: +#endif + case EINTR: + continue; + default: + if (fds[0].revents & POLLERR) + { + int err; + socklen_t len= sizeof (err); + (void)getsockopt(ptr->fd, SOL_SOCKET, SO_ERROR, &err, &len); + ptr->cached_errno= (err == 0) ? errno : err; + } + else + { + ptr->cached_errno= errno; + } + + (void)close(ptr->fd); + ptr->fd= -1; + + return MEMCACHED_ERRNO; + } + } + WATCHPOINT_ASSERT(0); // Programming error + } + + // This should only be possible from ERESTART or EINTR; + ptr->cached_errno= errno; + + return MEMCACHED_ERRNO; +} + static memcached_return_t set_hostinfo(memcached_server_st *server) { struct addrinfo *ai; @@ -265,6 +352,8 @@ test_connect: static memcached_return_t network_connect(memcached_server_st *ptr) { + bool timeout_error_occured= false; + if (ptr->fd == -1) { struct addrinfo *use; @@ -305,93 +394,39 @@ static memcached_return_t network_connect(memcached_server_st *ptr) (void)set_socket_options(ptr); /* connect to server */ - if ((connect(ptr->fd, use->ai_addr, use->ai_addrlen) == -1)) + if ((connect(ptr->fd, use->ai_addr, use->ai_addrlen) > -1)) { - ptr->cached_errno= errno; - if (errno == EINPROGRESS || /* nonblocking mode - first return, */ - errno == EALREADY) /* nonblocking mode - subsequent returns */ - { - struct pollfd fds[1]; - fds[0].fd = ptr->fd; - fds[0].events = POLLOUT; - - int timeout= ptr->root->connect_timeout; - if (ptr->root->flags.no_block == true) - timeout= -1; - - size_t loop_max= 5; - while (--loop_max) - { - int error= poll(fds, 1, timeout); - - switch (error) - { - case 1: - loop_max= 1; - break; - case 0: - if (loop_max==1) - return MEMCACHED_TIMEOUT; - continue; - // A real error occurred and we need to completely bail - default: - WATCHPOINT_ERRNO(errno); - switch (errno) - { -#ifdef TARGET_OS_LINUX - case ERESTART: -#endif - case EINTR: - continue; - default: - if (fds[0].revents & POLLERR) - { - int err; - socklen_t len= sizeof (err); - (void)getsockopt(ptr->fd, SOL_SOCKET, SO_ERROR, &err, &len); - ptr->cached_errno= (err == 0) ? errno : err; - } - - (void)close(ptr->fd); - ptr->fd= -1; - - break; - } - } - } - } - else if (errno == EISCONN) /* we are connected :-) */ - { - break; - } - else if (errno != EINTR) - { - (void)close(ptr->fd); - ptr->fd= -1; - break; - } + break; // Success } -#ifdef LIBMEMCACHED_WITH_SASL_SUPPORT - if (ptr->fd != -1 && ptr->root->sasl && ptr->root->sasl->callbacks) + /* An error occurred */ + ptr->cached_errno= errno; + if (errno == EINPROGRESS || /* nonblocking mode - first return, */ + errno == EALREADY) /* nonblocking mode - subsequent returns */ { - memcached_return rc= memcached_sasl_authenticate_connection(ptr); - if (rc != MEMCACHED_SUCCESS) - { - (void)close(ptr->fd); - ptr->fd= -1; + memcached_return_t rc; + rc= connect_poll(ptr); - return rc; - } - } -#endif + if (rc == MEMCACHED_TIMEOUT) + timeout_error_occured= true; - - if (ptr->fd != -1) + if (rc == MEMCACHED_SUCCESS) + break; + } + else if (errno == EISCONN) /* we are connected :-) */ + { + break; + } + else if (errno == EINTR) // Special case, we retry ai_addr { - return MEMCACHED_SUCCESS; + (void)close(ptr->fd); + ptr->fd= -1; + continue; } - use = use->ai_next; + + (void)close(ptr->fd); + ptr->fd= -1; + use= use->ai_next; } } @@ -408,7 +443,7 @@ static memcached_return_t network_connect(memcached_server_st *ptr) ptr->next_retry= next_time.tv_sec + ptr->root->retry_timeout; } - if (ptr->cached_errno == 0) + if (timeout_error_occured) return MEMCACHED_TIMEOUT; return MEMCACHED_ERRNO; /* The last error should be from connect() */ @@ -481,6 +516,17 @@ memcached_return_t memcached_connect(memcached_server_write_instance_st ptr) case MEMCACHED_CONNECTION_UDP: case MEMCACHED_CONNECTION_TCP: rc= network_connect(ptr); +#ifdef LIBMEMCACHED_WITH_SASL_SUPPORT + if (ptr->fd != -1 && ptr->root->sasl && ptr->root->sasl->callbacks) + { + rc= memcached_sasl_authenticate_connection(ptr); + if (rc != MEMCACHED_SUCCESS) + { + (void)close(ptr->fd); + ptr->fd= -1; + } + } +#endif break; case MEMCACHED_CONNECTION_UNIX_SOCKET: rc= unix_socket_connect(ptr); diff --git a/tests/mem_functions.c b/tests/mem_functions.c index e12ad3f7..d2b59112 100644 --- a/tests/mem_functions.c +++ b/tests/mem_functions.c @@ -5018,7 +5018,7 @@ static test_return_t memcached_get_hashkit_test (memcached_st *memc) We are testing the error condition when we connect to a server via memcached_get() but find that the server is not available. */ -static test_return_t memcached_get_MEMCACHED_SOME_ERRORS(memcached_st *memc) +static test_return_t memcached_get_MEMCACHED_ERRNO(memcached_st *memc) { (void)memc; memcached_st *tl_memc_h; @@ -5032,7 +5032,7 @@ static test_return_t memcached_get_MEMCACHED_SOME_ERRORS(memcached_st *memc) // Create a handle. tl_memc_h= memcached_create(NULL); - servers= memcached_servers_parse("localhost:9898"); // This server should not exist + servers= memcached_servers_parse("localhost:9898,localhost:9899"); // This server should not exist memcached_server_push(tl_memc_h, servers); memcached_server_list_free(servers); @@ -5046,7 +5046,7 @@ static test_return_t memcached_get_MEMCACHED_SOME_ERRORS(memcached_st *memc) } test_true(len == 0); - test_true(rc == MEMCACHED_SOME_ERRORS); + test_true(rc == MEMCACHED_ERRNO); return TEST_SUCCESS; } @@ -5083,7 +5083,7 @@ static test_return_t memcached_get_MEMCACHED_NOTFOUND(memcached_st *memc) We are testing the error condition when we connect to a server via memcached_get_by_key() but find that the server is not available. */ -static test_return_t memcached_get_by_key_MEMCACHED_SOME_ERRORS(memcached_st *memc) +static test_return_t memcached_get_by_key_MEMCACHED_ERRNO(memcached_st *memc) { (void)memc; memcached_st *tl_memc_h; @@ -5097,7 +5097,7 @@ static test_return_t memcached_get_by_key_MEMCACHED_SOME_ERRORS(memcached_st *me // Create a handle. tl_memc_h= memcached_create(NULL); - servers= memcached_servers_parse("localhost:9898"); // This server should not exist + servers= memcached_servers_parse("localhost:9898,localhost:9899"); // This server should not exist memcached_server_push(tl_memc_h, servers); memcached_server_list_free(servers); @@ -5111,7 +5111,7 @@ static test_return_t memcached_get_by_key_MEMCACHED_SOME_ERRORS(memcached_st *me } test_true(len == 0); - test_true(rc == MEMCACHED_SOME_ERRORS); + test_true(rc == MEMCACHED_ERRNO); return TEST_SUCCESS; } @@ -6332,9 +6332,9 @@ test_st hash_tests[] ={ }; test_st error_conditions[] ={ - {"memcached_get_MEMCACHED_SOME_ERRORS", 0, (test_callback_fn)memcached_get_MEMCACHED_SOME_ERRORS }, + {"memcached_get_MEMCACHED_ERRNO", 0, (test_callback_fn)memcached_get_MEMCACHED_ERRNO }, {"memcached_get_MEMCACHED_NOTFOUND", 0, (test_callback_fn)memcached_get_MEMCACHED_NOTFOUND }, - {"memcached_get_by_key_MEMCACHED_SOME_ERRORS", 0, (test_callback_fn)memcached_get_by_key_MEMCACHED_SOME_ERRORS }, + {"memcached_get_by_key_MEMCACHED_ERRNO", 0, (test_callback_fn)memcached_get_by_key_MEMCACHED_ERRNO }, {"memcached_get_by_key_MEMCACHED_NOTFOUND", 0, (test_callback_fn)memcached_get_by_key_MEMCACHED_NOTFOUND }, {0, 0, (test_callback_fn)0} }; -- 2.30.2