[PATCH] ab.c (REVISED)

[Rob Saccoccio <ro...@fastcgi.com>]

All right, let me try again...

I encountered some brokenness running ab on Win2K.

It was spinning do to a bug in apr_poll() (patches posted to apr) and
because ab.c:test() was adding a socket to a pollset that was already in
there.

apr_poll_socket_add() was actually returning APR_NOMEM because the pollset
was full, but the return values aren't checked in ab.c.  Once the pollset
had available slots the socket did get added a second/third/etc time.

Should apr_poll_socket_add() be checking to see if the socket's file
descriptor is already in the pollset (instead of adding it again)?

concurrency is now required to be smaller than the number of requests rather
than trying to deal with it (badly).

A timeout return from apr_poll() is now properly handled.

Note there are piles of tabs in ab.c that should be removed.

--rob


diff -u -r1.113 ab.c
--- ab.c	16 Jul 2002 09:30:16 -0000	1.113
+++ ab.c	21 Jul 2002 02:18:44 -0000
@@ -592,11 +592,7 @@
     int i, count, hdone = 0;
     char ssl_hostname[80];

-    /* XXX - Verify if it's okay - TBD */
-    if (requests < concurrency)
-        requests = concurrency;
-
-    if (!(started < requests))
+    if (started >= requests)
         return;

     c->read = 0;
@@ -1208,8 +1204,8 @@
     }
 #endif

-    if (!(started < requests))
-	return;
+    if (started >= requests)
+        return;

     c->read = 0;
     c->bread = 0;
@@ -1542,7 +1538,7 @@
 	if (isproxy)
 	    printf("[through %s:%d] ", proxyhost, proxyport);
 	printf("(be patient)%s",
-	       (heartbeatres ? "\n" : "..."));
+	       (heartbeatres ? "\n" : "... "));
 	fflush(stdout);
     }

@@ -1644,19 +1640,20 @@
 	    break;		/* no need to do another round */
 	}

-	n = concurrency;
 #ifdef USE_SSL
         if (ssl == 1)
             status = APR_SUCCESS;
         else
 #endif
 	status = apr_poll(readbits, concurrency, &n, aprtimeout);
-	if (status != APR_SUCCESS)
-	    apr_err("apr_poll", status);
-
-	if (!n) {
-	    err("\nServer timed out\n\n");
-	}
+        if (status != APR_SUCCESS) {
+            if (status == APR_TIMEUP) {
+                err("Server timed out\n");
+            }
+            else {
+                apr_err("apr_poll", status);
+            }
+        }

 	for (i = 0; i < concurrency; i++) {
 	    /*
@@ -1693,20 +1690,6 @@
 	    }
 	    if (rv & APR_POLLOUT)
 		write_request(&con[i]);
-
-	    /*
-	     * 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.
-	     */
-#ifdef USE_SSL
-            if (ssl != 1)
-#endif
-	    if (con[i].state == STATE_READ || con[i].state == STATE_CONNECTING)
-		apr_poll_socket_add(readbits, con[i].aprsock, APR_POLLIN);
-
 	}
     }

@@ -2087,6 +2070,11 @@
 	    copyright();
 	    return 0;
 	}
+    }
+
+    if (requests < concurrency) {
+        fprintf(stderr, "%s: requests is less than concurrency\n",
argv[0]);
+        usage(argv[0]);
     }

     if (opt->ind != argc - 1) {