svn commit: r599199 - /httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c

[wr...@apache.org]

Author: wrowe
Date: Wed Nov 28 15:24:07 2007
New Revision: 599199

URL: http://svn.apache.org/viewvc?rev=599199&view=rev
Log:
Solve two flaws by refactoring the PORT connection code, ensure
we hit the throttle timer for explicit ports (as well as the
previous throttling of ephemeral ports) and emit more information
about the recycling of port timers.

Modified:
    httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c

Modified: httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c
URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c?rev=599199&r1=599198&r2=599199&view=diff
==============================================================================
--- httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c (original)
+++ httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c Wed Nov 28 15:24:07 2007
@@ -1378,7 +1378,7 @@
     apr_socket_t *s;
     apr_status_t rv;
     apr_port_t port;
-    int found_port;
+    int tries = 0;
 
     if (fc->csock) {
         apr_socket_close(fc->csock);
@@ -1418,55 +1418,49 @@
         return FTP_REPLY_CANNOT_OPEN_DATACONN;
     }
 
+    /* XXX: should pick an arbitrary port within the acceptable range,
+     * but dodge the throttle timer on the first reloop (init tries = -1?)
+     */
     port = fsc->pasv_min;
-    for (found_port = 0; !found_port; )
+    while (1)
     {
-        int tries;
-
-        sa->port = port; /* Magic here when port = 0 */
-
+        /* Magic here when port == 0, accepts an ephemeral port assignment */
+        sa->port = port;
         rv = apr_socket_bind(s, sa);
         if (rv == APR_SUCCESS) {
-            found_port = 1;
             break;
         }
 
+        if (rv != FTP_APR_EADDRINUSE || tries >= FTP_MAX_TRIES) {
+            ap_log_error(APLOG_MARK, APLOG_ERR, rv, c->base_server,
+                         "Couldn't bind to passive socket");
+            apr_socket_close(s);
+            return FTP_REPLY_CANNOT_OPEN_DATACONN;
+        }
+
         /* If we are using a range, increment the port, and continue.  If
          * we have reached the top of the range start over at the 
          * beginning 
          */
         if (port != 0) {
-            if (port == fsc->pasv_max) {
-                ap_log_error(APLOG_MARK, APLOG_ERR, 0, c->base_server,
-                             "Couldn't find port within range for passive "
-                             "connection.  Restarting at %d", fsc->pasv_min);
-                apr_socket_close(s);
-                return FTP_REPLY_CANNOT_OPEN_DATACONN;
+            if (port < fsc->pasv_max) {
+                ++port;
+                continue;
             }
-            port++;
-            continue;
+            port = fsc->pasv_min;
+            /* Fall through into throttle */
         } 
         
-        /* Otherwise, we are using a random port, loop attempting to bind.
-         * Under high load we may have many sockets in TIME_WAIT, causing
+        /* Under high load we may have many sockets in TIME_WAIT, causing
          * bind to fail with EADDRINUSE.  In these conditions we throttle
-         * the system by sleeping before we attempt to bind again
+         * the system by sleeping before we attempt to bind again within
+         * our acceptable port range
          */
-        for (tries = 0;; tries++) {
-            rv = apr_socket_bind(s, sa);
-            if (rv == APR_SUCCESS) {
-                found_port = 1;
-                break;
-            }
-            if (rv != FTP_APR_EADDRINUSE ||
-                tries >= FTP_MAX_TRIES) {
-                ap_log_error(APLOG_MARK, APLOG_ERR, rv, c->base_server,
-                             "Couldn't bind to passive socket");
-                apr_socket_close(s);
-                return FTP_REPLY_CANNOT_OPEN_DATACONN;
-            }
-            apr_sleep(tries * APR_USEC_PER_SEC);
-        }
+        ++tries;
+        ap_log_error(APLOG_MARK, APLOG_INFO, 0, c->base_server,
+                     "Couldn't find port within range for passive "
+                     "connection.  Restarting at %d (retry %d)", port, tries);
+        apr_sleep(tries * APR_USEC_PER_SEC);
     }
 
     rv = apr_socket_listen(s, 1);