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;

 }

Reply via email to