apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rob Saccoccio" <r...@fastcgi.com>
Subject [PATCH] poll.c pollacc.c ab.c
Date Sat, 03 Aug 2002 15:15:28 GMT
Below are the updated patches I mentioned last night.  They are the same as
previously submitted
(http://marc.theaimsgroup.com/?l=apr-dev&m=102721946317193&w=2,
http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=102721825716797&w=2) but
ported to HEAD.

Its not clear to me which poll API should be used when.  Apache uses the
deprecated API.  Some thoughts on the subject should probably be put in
poll.h and Apache updated.

Somewhere along the way in the recent wave of commits (rev 1.14?), a change
was committed to apr_poll() that presumes that the apr_pollfd_t struct is
packed.  I believe this was inadvertent, but its not clear because (the
deprecated) socket_add() and friends don't handle either packed or sparse
correctly.  These problems are corrected in the patch below (sparse
support).

--rob


NOTE:  The use of APR_FROM_OS_ERROR() should probably instead use
apr_get_netos_error(), but that doesn't look defined for platforms other
than WIN and NETWARE.


Index: poll.c
===================================================================
RCS file: /home/cvspublic/apr/poll/unix/poll.c,v
retrieving revision 1.24
diff -u -r1.24 poll.c
--- poll.c	3 Aug 2002 00:10:57 -0000	1.24
+++ poll.c	3 Aug 2002 14:45:36 -0000
@@ -221,7 +221,7 @@
 #endif
             fd = aprset[i].desc.s->socketdes;
         }
-        else {
+        else if (aprset[i].desc_type == APR_POLL_FILE) {
 #if !APR_FILES_AS_SOCKETS
             return APR_EBADF;
 #else
@@ -237,6 +237,13 @@

 #endif /* APR_FILES_AS_SOCKETS */
         }
+        else if (aprset[i].desc_type == APR_NO_DESC) {
+            continue;
+        }
+        else {
+            return APR_EBADF;
+        }
+
         if (aprset[i].reqevents & APR_POLLIN) {
             FD_SET(fd, &readset);
         }
@@ -265,27 +272,36 @@
     }
 #endif

-    (*nsds) = rv;
-    if ((*nsds) == 0) {
+    if (rv == 0) {
         return APR_TIMEUP;
     }
-    if ((*nsds) < 0) {
-        return errno;
+    if (rv < 0) {
+#ifdef WIN32
+        return APR_FROM_OS_ERROR(WSAGetLastError());
+#else
+        return APR_FROM_OS_ERROR(errno);
+#endif
     }

+    (*nsds) = rv;
+
     for (i = 0; i < num; i++) {
         apr_os_sock_t fd;

         if (aprset[i].desc_type == APR_POLL_SOCKET) {
             fd = aprset[i].desc.s->socketdes;
         }
-        else {
+        else if (aprset[i].desc_type == APR_POLL_FILE) {
 #if !APR_FILES_AS_SOCKETS
             return APR_EBADF;
 #else
             fd = aprset[i].desc.f->filedes;
 #endif
         }
+        else if (aprset[i].desc_type == APR_NO_DESC) {
+            continue;
+        }
+
         aprset[i].rtnevents = 0;
         if (FD_ISSET(fd, &readset)) {
             aprset[i].rtnevents |= APR_POLLIN;
Index: pollacc.c
===================================================================
RCS file: /home/cvspublic/apr/poll/unix/pollacc.c,v
retrieving revision 1.2
diff -u -r1.2 pollacc.c
--- pollacc.c	16 Jul 2002 05:25:44 -0000	1.2
+++ pollacc.c	3 Aug 2002 14:45:36 -0000
@@ -77,15 +77,14 @@
 static apr_pollfd_t *find_poll_sock(apr_pollfd_t *aprset, apr_socket_t
*sock)
 {
     apr_pollfd_t *curr = aprset;
-
-    while (curr->desc.s != sock) {
-        if (curr->desc_type == APR_POLL_LASTDESC) {
-            return NULL;
-        }
+
+    while (curr->desc_type != APR_POLL_LASTDESC) {
+        if (curr->desc.s == sock && curr->desc_type != APR_NO_DESC)
+            return curr;
         curr++;
     }

-    return curr;
+    return NULL;
 }

 APR_DECLARE(apr_status_t) apr_poll_socket_add(apr_pollfd_t *aprset,




NOTE:  I used -w to facilitate review, you'll no doubt want to fix the
indenting once applied (ab.c is full of tabs by the way).

Index: ab.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/support/ab.c,v
retrieving revision 1.114
diff -u -w -r1.114 ab.c
--- ab.c	30 Jul 2002 13:00:33 -0000	1.114
+++ ab.c	3 Aug 2002 14:56:48 -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;
@@ -705,14 +701,10 @@

 static void write_request(struct connection * c)
 {
-    do {
 	apr_time_t tnow = apr_time_now();
-	apr_size_t l = c->rwrite;
+    apr_size_t l;
 	apr_status_t e;

-	/*
-	 * First time round ?
-	 */
 	if (c->rwrite == 0) {
 #ifdef USE_SSL
             if (ssl != 1)
@@ -730,12 +722,13 @@
 	    return;
 	}

+    l = c->rwrite;
+
 #ifdef USE_SSL
         if (ssl == 1) {
             apr_size_t e_ssl;
             e_ssl = SSL_write(c->ssl,request + c->rwrote, l);
-            if (e_ssl != l)
-            {
+        if (e_ssl != l) {
                 printf("SSL write failed - closing connection\n");
                 close_connection (c);
                 return;
@@ -746,18 +739,20 @@
 #endif
 	e = apr_send(c->aprsock, request + c->rwrote, &l);

-	/*
-	 * Bail early on the most common case
-	 */
-	if (l == c->rwrite)
-	    break;
-
+    if (l == c->rwrite) {
+        totalposted += l;
+        c->state = STATE_READ;
+        c->endwrite = apr_time_now();
 #ifdef USE_SSL
         if (ssl != 1)
+#endif
+        readbits[c->socknum].reqevents = APR_POLLIN;
+        return;
+    }
+
+#ifdef USE_SSL
+    if (ssl != 1) {
 	if (e != APR_SUCCESS) {
-	    /*
-	     * Let's hope this traps EWOULDBLOCK too !
-	     */
 	    if (!APR_STATUS_IS_EAGAIN(e)) {
 		epipe++;
 		printf("Send request failed!\n");
@@ -765,18 +760,11 @@
 	    }
 	    return;
 	}
+    }
 #endif
 	c->rwrote += l;
 	c->rwrite -= l;
-    } while (1);
-
     totalposted += c->rwrite;
-    c->state = STATE_READ;
-    c->endwrite = apr_time_now();
-#ifdef USE_SSL
-    if (ssl != 1)
-#endif
-    apr_poll_socket_add(readbits, c->aprsock, APR_POLLIN);
 }

 /* --------------------------------------------------------- */
@@ -1208,7 +1196,7 @@
     }
 #endif

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

     c->read = 0;
@@ -1230,15 +1218,22 @@
         apr_err("socket nonblock", rv);
     }
     c->start = apr_time_now();
-    if ((rv = apr_connect(c->aprsock, destsa)) != APR_SUCCESS) {
+    if ((rv = apr_connect(c->aprsock, destsa)) == APR_SUCCESS) {
+        readbits[c->socknum].desc_type = APR_POLL_SOCKET;
+        readbits[c->socknum].desc.s = c->aprsock;
+        readbits[c->socknum].reqevents = APR_POLLOUT;
+    }
+    else {
 	if (APR_STATUS_IS_EINPROGRESS(rv)) {
 	    c->state = STATE_CONNECTING;
 	    c->rwrite = 0;
-	    apr_poll_socket_add(readbits, c->aprsock, APR_POLLOUT);
+            readbits[c->socknum].desc_type = APR_POLL_SOCKET;
+            readbits[c->socknum].desc.s = c->aprsock;
+            readbits[c->socknum].reqevents = APR_POLLOUT;
 	    return;
 	}
 	else {
-	    apr_poll_socket_remove(readbits, c->aprsock);
+            readbits[c->socknum].desc_type = APR_NO_DESC;
 	    apr_socket_close(c->aprsock);
 	    err_conn++;
 	    if (bad++ > 10) {
@@ -1303,7 +1298,7 @@
     }
     else {
 #endif
-    apr_poll_socket_remove(readbits, c->aprsock);
+    readbits[c->socknum].desc_type = APR_NO_DESC;
     apr_socket_close(c->aprsock);
 #ifdef USE_SSL
     }
@@ -1408,7 +1403,7 @@
 	    }
 	    else {
 		/* header is in invalid or too big - close connection */
-		apr_poll_socket_remove(readbits, c->aprsock);
+                readbits[c->socknum].desc_type = APR_NO_DESC;
 		apr_socket_close(c->aprsock);
 		err_response++;
 		if (bad++ > 10) {
@@ -1551,7 +1546,9 @@
     con = calloc(concurrency * sizeof(struct connection), 1);

     stats = calloc(requests * sizeof(struct data), 1);
-    apr_poll_setup(&readbits, concurrency, cntxt);
+
+    /* assume APR_NO_DESC == 0 */
+    readbits = calloc(concurrency * sizeof(apr_pollfd_t), 1);

     /* setup request */
     if (posting <= 0) {
@@ -1644,18 +1641,19 @@
 	    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)
+        if (status != APR_SUCCESS) {
+            if (status == APR_TIMEUP) {
+                err("Server timed out\n");
+            }
+            else {
 	    apr_err("apr_poll", status);
-
-	if (!n) {
-	    err("\nServer timed out\n\n");
+            }
 	}

 	for (i = 0; i < concurrency; i++) {
@@ -1670,7 +1668,8 @@
                 rv = APR_POLLIN;
             else
 #endif
-	    apr_poll_revents_get(&rv, con[i].aprsock, readbits);
+            rv = readbits[i].rtnevents;
+
 	    /*
 	     * Notes: APR_POLLHUP is set after FIN is received on some
 	     * systems, so treat that like APR_POLLIN so that we try to read
@@ -1693,20 +1692,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 +2072,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) {



Mime
View raw message