ab: Fix maintenance of the pollset to resolve EINPROGRESS errors

with kqueue (BSD/OS X) and excessive CPU with event ports (Solaris).

The apr_pollset API requires that a socket be removed from
the pollset whenever there is a change to the requested events.
The socket must also be removed when it is closed.  This wasn't
completely implemented.

PR: 44584

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@749438 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index aa252afad70139e5b6b3328cd24c847c96e5dab6..366bca22c8baaf363343298ad1a49dce9aef25a3 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.3.2
 
+  *) ab: Fix maintenance of the pollset to resolve EINPROGRESS errors 
+     with kqueue (BSD/OS X) and excessive CPU with event ports (Solaris).
+     PR 44584.  [Jeff Trawick]
+
   *) mod_disk_cache: The module now turns off sendfile support if
      'EnableSendfile off' is defined globally. [Lars Eilebrecht]
 

diff --git a/support/ab.c b/support/ab.c
index 94e8772479b318690c2e171eb107432177a94a76..0f1aef5a9d455df5a6e316f8c2cebccd60d40e7e 100644 (file)
--- a/support/ab.c
+++ b/support/ab.c
@@ -208,13 +208,18 @@ typedef STACK_OF(X509) X509_STACK_TYPE;
 /* maximum number of requests on a time limited test */
 #define MAX_REQUESTS (INT_MAX > 50000 ? 50000 : INT_MAX)
 
-/* good old state hostname */
-#define STATE_UNCONNECTED 0
-#define STATE_CONNECTING  1     /* TCP connect initiated, but we don't
+/* connection state
+ * don't add enums or rearrange or otherwise change values without
+ * visiting set_conn_state()
+ */
+typedef enum {
+    STATE_UNCONNECTED = 0,
+    STATE_CONNECTING,           /* TCP connect initiated, but we don't
                                  * know if it worked yet
                                  */
-#define STATE_CONNECTED   2     /* we know TCP connect completed */
-#define STATE_READ        3
+    STATE_CONNECTED,            /* we know TCP connect completed */
+    STATE_READ
+} connect_state_e;
 
 #define CBUFFSIZE (2048)
 
@@ -239,6 +244,7 @@ struct connection {
                done;            /* Connection closed */
 
     int socknum;
+    apr_int16_t reqevents;      /* current poll events for this socket */
 #ifdef USE_SSL
     SSL *ssl;
 #endif
@@ -383,6 +389,56 @@ static void apr_err(char *s, apr_status_t rv)
     exit(rv);
 }
 
+static void set_polled_events(struct connection *c, apr_int16_t new_reqevents)
+{
+    apr_int16_t old_reqevents = c->reqevents;
+    apr_pollfd_t pfd;
+    apr_status_t rv;
+    char buf[120];
+
+    if (old_reqevents != new_reqevents) {
+        pfd.desc_type = APR_POLL_SOCKET;
+        pfd.desc.s = c->aprsock;
+        pfd.client_data = c;
+
+        if (old_reqevents != 0) {
+            pfd.reqevents = old_reqevents;
+            rv = apr_pollset_remove(readbits, &pfd);
+            if (rv != APR_SUCCESS) {
+                apr_err("apr_pollset_remove()", rv);
+            }
+        }
+
+        if (new_reqevents != 0) {
+            pfd.reqevents = new_reqevents;
+            rv = apr_pollset_add(readbits, &pfd);
+            if (rv != APR_SUCCESS) {
+                apr_err("apr_pollset_add()", rv);
+                exit(1);
+            }
+        }
+
+        c->reqevents = new_reqevents;
+    }
+}
+
+static void set_conn_state(struct connection *c, connect_state_e new_state)
+{
+    apr_int16_t events_by_state[] = {
+        0,           /* for STATE_UNCONNECTED */
+        APR_POLLOUT, /* for STATE_CONNECTING */
+        APR_POLLIN,  /* for STATE_CONNECTED; we don't poll in this state,
+                      * so prepare for polling in the following state --
+                      * STATE_READ
+                      */
+        APR_POLLIN   /* for STATE_READ */
+    };
+
+    c->state = new_state;
+
+    set_polled_events(c, events_by_state[new_state]);
+}
+
 /* --------------------------------------------------------- */
 /* write out request to a connection - assumes we can write
  * (small) request out in one go into our new socket buffer
@@ -556,7 +612,6 @@ static void ssl_proceed_handshake(struct connection *c)
 
     while (do_next) {
         int ret, ecode;
-        apr_pollfd_t new_pollfd;
 
         ret = SSL_do_handshake(c->ssl);
         ecode = SSL_get_error(c->ssl, ret);
@@ -588,11 +643,7 @@ static void ssl_proceed_handshake(struct connection *c)
             do_next = 0;
             break;
         case SSL_ERROR_WANT_READ:
-            new_pollfd.desc_type = APR_POLL_SOCKET;
-            new_pollfd.reqevents = APR_POLLIN;
-            new_pollfd.desc.s = c->aprsock;
-            new_pollfd.client_data = c;
-            apr_pollset_add(readbits, &new_pollfd);
+            set_polled_events(c, APR_POLLIN);
             do_next = 0;
             break;
         case SSL_ERROR_WANT_WRITE:
@@ -668,16 +719,8 @@ static void write_request(struct connection * c)
         c->rwrite -= l;
     } while (c->rwrite);
 
-    c->state = STATE_READ;
     c->endwrite = lasttime = apr_time_now();
-    {
-        apr_pollfd_t new_pollfd;
-        new_pollfd.desc_type = APR_POLL_SOCKET;
-        new_pollfd.reqevents = APR_POLLIN;
-        new_pollfd.desc.s = c->aprsock;
-        new_pollfd.client_data = c;
-        apr_pollset_add(readbits, &new_pollfd);
-    }
+    set_conn_state(c, STATE_READ);
 }
 
 /* --------------------------------------------------------- */
@@ -1191,21 +1234,12 @@ static void start_connect(struct connection * c)
 #endif
     if ((rv = apr_socket_connect(c->aprsock, destsa)) != APR_SUCCESS) {
         if (APR_STATUS_IS_EINPROGRESS(rv)) {
-            apr_pollfd_t new_pollfd;
-            c->state = STATE_CONNECTING;
+            set_conn_state(c, STATE_CONNECTING);
             c->rwrite = 0;
-            new_pollfd.desc_type = APR_POLL_SOCKET;
-            new_pollfd.reqevents = APR_POLLOUT;
-            new_pollfd.desc.s = c->aprsock;
-            new_pollfd.client_data = c;
-            apr_pollset_add(readbits, &new_pollfd);
             return;
         }
         else {
-            apr_pollfd_t remove_pollfd;
-            remove_pollfd.desc_type = APR_POLL_SOCKET;
-            remove_pollfd.desc.s = c->aprsock;
-            apr_pollset_remove(readbits, &remove_pollfd);
+            set_conn_state(c, STATE_UNCONNECTED);
             apr_socket_close(c->aprsock);
             err_conn++;
             if (bad++ > 10) {
@@ -1213,14 +1247,14 @@ static void start_connect(struct connection * c)
                    "\nTest aborted after 10 failures\n\n");
                 apr_err("apr_socket_connect()", rv);
             }
-            c->state = STATE_UNCONNECTED;
+            
             start_connect(c);
             return;
         }
     }
 
     /* connected first time */
-    c->state = STATE_CONNECTED;
+    set_conn_state(c, STATE_CONNECTED); /* will this waste a pollset call? */
     started++;
 #ifdef USE_SSL
     if (c->ssl) {
@@ -1269,21 +1303,15 @@ static void close_connection(struct connection * c)
         }
     }
 
-    {
-        apr_pollfd_t remove_pollfd;
-        remove_pollfd.desc_type = APR_POLL_SOCKET;
-        remove_pollfd.desc.s = c->aprsock;
-        apr_pollset_remove(readbits, &remove_pollfd);
+    set_conn_state(c, STATE_UNCONNECTED);
 #ifdef USE_SSL
-        if (c->ssl) {
-            SSL_shutdown(c->ssl);
-            SSL_free(c->ssl);
-            c->ssl = NULL;
-        }
-#endif
-        apr_socket_close(c->aprsock);
+    if (c->ssl) {
+        SSL_shutdown(c->ssl);
+        SSL_free(c->ssl);
+        c->ssl = NULL;
     }
-    c->state = STATE_UNCONNECTED;
+#endif
+    apr_socket_close(c->aprsock);
 
     /* connect again */
     start_connect(c);
@@ -1401,10 +1429,7 @@ static void read_connection(struct connection * c)
             }
             else {
             /* header is in invalid or too big - close connection */
-                apr_pollfd_t remove_pollfd;
-                remove_pollfd.desc_type = APR_POLL_SOCKET;
-                remove_pollfd.desc.s = c->aprsock;
-                apr_pollset_remove(readbits, &remove_pollfd);
+                set_conn_state(c, STATE_UNCONNECTED);
                 apr_socket_close(c->aprsock);
                 err_response++;
                 if (bad++ > 10) {
@@ -1727,11 +1752,7 @@ static void test(void)
             }
             if (rv & APR_POLLOUT) {
                 if (c->state == STATE_CONNECTING) {
-                    apr_pollfd_t remove_pollfd;
                     rv = apr_socket_connect(c->aprsock, destsa);
-                    remove_pollfd.desc_type = APR_POLL_SOCKET;
-                    remove_pollfd.desc.s = c->aprsock;
-                    apr_pollset_remove(readbits, &remove_pollfd);
                     if (rv != APR_SUCCESS) {
                         apr_socket_close(c->aprsock);
                         err_conn++;
@@ -1740,12 +1761,12 @@ static void test(void)
                                     "\nTest aborted after 10 failures\n\n");
                             apr_err("apr_socket_connect()", rv);
                         }
-                        c->state = STATE_UNCONNECTED;
+                        set_conn_state(c, STATE_UNCONNECTED);
                         start_connect(c);
                         continue;
                     }
                     else {
-                        c->state = STATE_CONNECTED;
+                        set_conn_state(c, STATE_CONNECTED);
                         started++;
 #ifdef USE_SSL
                         if (c->ssl)
@@ -1759,22 +1780,6 @@ static void test(void)
                     write_request(c);
                 }
             }
-
-            /*
-             * When using a select based poll every time we check the bits
-             * are reset. In 1.3's ab we copied the FD_SET's each time
-             * through, but here we're going to check the state and if the
-             * connection is in STATE_READ or STATE_CONNECTING we'll add the
-             * socket back in as APR_POLLIN.
-             */
-                if (c->state == STATE_READ) {
-                    apr_pollfd_t new_pollfd;
-                    new_pollfd.desc_type = APR_POLL_SOCKET;
-                    new_pollfd.reqevents = APR_POLLIN;
-                    new_pollfd.desc.s = c->aprsock;
-                    new_pollfd.client_data = c;
-                    apr_pollset_add(readbits, &new_pollfd);
-                }
         }
     } while (lasttime < stoptime && done < requests);