This patch makes the open_socket() function non-recursive, providing the benefits of iterative functions in terms of performance, also replacing the close_socket (same level) reference with the lower level apr_socket_close(), since this would reserve close_socket() for a case where the open_socket() successfully occurred. This socket closure now also occurs in the case where a socket is to be reopened in the case of TIME_WAIT exhaustion, which was the recursive condition. Please examine and apply this change, which was made to the current CVS tree, without reference to my former submission.
-Norman Tuttle, OpenDemand Systems Developer, [EMAIL PROTECTED] PS: The conditions of lack of success for apr_socket_create() (found within apr_socket_open() function) should include a case where memory wasn't available to allocate for the creation of the APR socket structure, but I did not see evidence of this in either the Win32 or Unix versions of the APR library. Both functions call a void function alloc_socket() which does the allocation and then immediately sets members of the allocated socket, so they are obviously assuming the allocation occurred. Unfortunately, real software cannot be so sure. I was wondering if (1) the APR provided alternate functions which check for this failure case, and (2) how many APR functions are doing memory checking. I am dittoing this comment to the APR mailing list.
--- flood_net.c 2003-10-16 16:14:16.000000000 -0400 +++ \flood-1.1\flood_net.c 2003-09-09 05:49:50.000000000 -0400 @@ -58,8 +58,6 @@ #include "flood_profile.h" #include "flood_net.h" -#define FOUR_MIN 4*60*APR_USEC_PER_SEC - /* Open the TCP connection to the server */ flood_socket_t* open_socket(apr_pool_t *pool, request_t *r, apr_status_t *status) @@ -67,37 +65,54 @@ apr_status_t rv = 0; apr_sockaddr_t *destsa; flood_socket_t* fs; - + fs = apr_palloc(pool, sizeof(flood_socket_t)); - if ((rv = apr_sockaddr_info_get(&destsa, r->parsed_uri->hostname, APR_INET, - r->parsed_uri->port, 0, pool)) - == APR_SUCCESS) do { /* goal of do-loop is - to get past EAGAIN error so we need a 0 (APR_SUCCESS) status to pass */ - if ((rv = apr_socket_create(&fs->socket, APR_INET, SOCK_STREAM, - APR_PROTO_TCP, pool)) != APR_SUCCESS) break; - if ((rv = apr_socket_connect(fs->socket, destsa)) != APR_SUCCESS) { - apr_socket_close(fs->socket); /* If we have created socket and cannot - connect it (including on reopen), we should now destroy the socket */ - if (APR_STATUS_IS_EAGAIN(rv)) + if ((rv = apr_sockaddr_info_get(&destsa, r->parsed_uri->hostname, APR_INET, + r->parsed_uri->port, 0, pool)) + != APR_SUCCESS) { + if (status) { + *status = rv; + } + return NULL; + } + + if ((rv = apr_socket_create(&fs->socket, APR_INET, SOCK_STREAM, + APR_PROTO_TCP, pool)) != APR_SUCCESS) { + if (status) { + *status = rv; + } + return NULL; + } + + if ((rv = apr_socket_connect(fs->socket, destsa)) != APR_SUCCESS) { + if (APR_STATUS_IS_EINPROGRESS(rv)) { + /* FIXME: Handle better */ + close_socket(fs); + if (status) { + *status = rv; + } + return NULL; + } + else if (APR_STATUS_IS_EAGAIN(rv)) { /* We have run out of ports available due to TIME_WAIT exhaustion. - * Sleep for four minutes, and try again. + * Sleep for four minutes, and try again. * Note: Solaris returns EADDRNOTAVAIL, Linux returns EAGAIN. * XXX: Then APR'IZE THIS ALREADY */ - apr_sleep(FOUR_MIN); + apr_sleep(4 * 60 * APR_USEC_PER_SEC); + return open_socket(pool, r, status); } - else break; /* Includes APR_STATUS_IS_EINPROGRESS with other errors, - * will exit while loop & func with an error status */ - } - } while (rv); - if (rv) - { - if (status) { - *status = rv; + else + { + /* FIXME: Handle */ + close_socket(fs); + if (status) { + *status = rv; + } + return NULL; } - return (flood_socket_t *)(fs->socket = NULL); } apr_socket_opt_set(fs->socket, APR_SO_TIMEOUT, LOCAL_SOCKET_TIMEOUT); @@ -105,7 +120,7 @@ fs->read_pollset.desc.s = fs->socket; fs->read_pollset.reqevents = APR_POLLIN; fs->read_pollset.p = pool; - + return fs; } @@ -154,11 +169,11 @@ pout.desc.s = s->socket; pout.reqevents = APR_POLLIN | APR_POLLPRI | APR_POLLERR | APR_POLLHUP | APR_POLLNVAL; pout.p = pool; - + e = apr_poll(&pout, 1, &socketsRead, 1000); if (socketsRead && pout.rtnevents) { return APR_EGENERAL; } - + return APR_SUCCESS; }