You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rob Saccoccio <ro...@fastcgi.com> on 2002/07/21 01:07:59 UTC
[patch] ab.c
I encountered some brokenness running ab on Win2K. It was spinning do to a
bug in apr_poll() (patches posted to apr) and start_connect().
The patch fixes this and a couple of other tweaks..
concurrency is now required to be smaller than the number of requests rather
than trying to deal with it (badly).
If there were no more requests to be made, start_connect() will now remove
the socket from the pollset. Without this the pollset will have an entry
with a socket with a closed handle. This change should probably be applied
to ssl_start_connect() as well.
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 20 Jul 2002 22:46:50 -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,10 @@
}
#endif
- if (!(started < requests))
- return;
+ if (started >= requests) {
+ apr_poll_socket_remove(readbits, c->aprsock);
+ return;
+ }
c->read = 0;
c->bread = 0;
@@ -1644,19 +1642,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++) {
/*
@@ -2087,6 +2086,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) {
[PATCH] ab.c (REVISED)
Posted by 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) {
RE: [patch] ab.c
Posted by Rob Saccoccio <ro...@fastcgi.com>.
This isn't quite (its still giving me some problems). I'm looking at it
again.
> -----Original Message-----
> From: Rob Saccoccio [mailto:robs@fastcgi.com]
> Sent: Saturday, July 20, 2002 7:08 PM
> To: dev@httpd.apache.org
> Subject: [patch] ab.c
>
>
> I encountered some brokenness running ab on Win2K. It was
> spinning do to a bug in apr_poll() (patches posted to apr) and
> start_connect().
>
> The patch fixes this and a couple of other tweaks..
>
> concurrency is now required to be smaller than the number of
> requests rather than trying to deal with it (badly).
>
> If there were no more requests to be made, start_connect() will
> now remove the socket from the pollset. Without this the pollset
> will have an entry with a socket with a closed handle. This
> change should probably be applied to ssl_start_connect() as well.
>
> 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 20 Jul 2002 22:46:50 -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,10 @@
> }
> #endif
>
> - if (!(started < requests))
> - return;
> + if (started >= requests) {
> + apr_poll_socket_remove(readbits, c->aprsock);
> + return;
> + }
>
> c->read = 0;
> c->bread = 0;
> @@ -1644,19 +1642,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++) {
> /*
> @@ -2087,6 +2086,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) {
>