You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Brian Pane <bp...@pacbell.net> on 2001/07/28 02:05:49 UTC

[PATCH] Re: Extraneous socket read?

Bill Stoddard wrote:

>Brian,
>Are you still considering working on an APR_INCOMPLETE_READ/APR_OPTIMISTIC_READ socket
>option?  I think something like this will be useful to the lingering close code
>(ap_lingering_close).  During lingering close, we do a non-blocking read first which
>returns EAGAIN then do a select.  In this case, it is clearly a win to just do the select
>(since the common case will be that we have no bytes to read on the inbound side of the
>connection) and avoid the initial read altogether.
>
>I think I prefer the existing operation (read then select) for the default but would like
>a way to tell APR to do a select then a read for special cases like this.
>
>Bill
>
Here's a patch that does the select before the read in three cases:
  * First read on a newly accepted connection
  * First read  during lingering close
  * Any read on socket where the last read returned fewer bytes than
    were requested.
The 3rd case is handled by default in the APR socket routines, while
the first two are controlled by the application (i.e., the httpd
overrides the default read-before-select behavior in these two
specific cases where it's useful to force select-first).  If anybody
later adds support for TCP_DEFER_ACCEPT, we can disable the select-first
on the initial read.

Half of the code in this patch is taken from the patch that Dean
posted; his heuristic for handling incomplete reads is more robust
than my original code.

--Brian

Index: srclib/apr/include/apr_network_io.h
===================================================================
RCS file: /home/cvspublic/apr/include/apr_network_io.h,v
retrieving revision 1.107
diff -u -r1.107 apr_network_io.h
--- srclib/apr/include/apr_network_io.h    2001/07/26 15:39:25    1.107
+++ srclib/apr/include/apr_network_io.h    2001/07/27 23:47:50
@@ -103,6 +103,17 @@
                                    * APR_TCP_NODELAY should be turned on
                                    * again when NOPUSH is turned off
                                    */
+#define APR_INCOMPLETE_READ 4096  /* Set on non-blocking sockets
+                   * (APR_SO_TIMEOUT != 0) on which the
+                   * previous read() did not fill a buffer
+                   * completely.  the next apr_recv() will
+                   * first call select()/poll() rather than
+                   * going straight into read().  (Can also
+                   * be set by an application to force a
+                   * select()/poll() call before the next
+                   * read, in cases where the app expects
+                   * that an immediate read would fail.)
+                   */
 
 #define APR_POLLIN    0x001
 #define APR_POLLPRI   0x002
Index: srclib/apr/network_io/unix/sockopt.c
===================================================================
RCS file: /home/cvspublic/apr/network_io/unix/sockopt.c,v
retrieving revision 1.47
diff -u -r1.47 sockopt.c
--- srclib/apr/network_io/unix/sockopt.c    2001/07/25 18:01:01    1.47
+++ srclib/apr/network_io/unix/sockopt.c    2001/07/27 23:47:50
@@ -203,6 +203,12 @@
                 }
             }
         }
+    /* must disable the incomplete read support if we change to a
+     * blocking socket.
+     */
+    if (on == 0) {
+        sock->netmask &= ~APR_INCOMPLETE_READ;
+    }
         sock->timeout = on;
         apr_set_option(&sock->netmask, APR_SO_TIMEOUT, on);
     }
@@ -265,6 +271,9 @@
 #else
         return APR_ENOTIMPL;
 #endif
+    }
+    if (opt & APR_INCOMPLETE_READ) {
+    apr_set_option(&sock->netmask, APR_INCOMPLETE_READ, on);
     }
 
     return APR_SUCCESS;
Index: srclib/apr/network_io/unix/sendrecv.c
===================================================================
RCS file: /home/cvspublic/apr/network_io/unix/sendrecv.c,v
retrieving revision 1.74
diff -u -r1.74 sendrecv.c
--- srclib/apr/network_io/unix/sendrecv.c    2001/07/26 17:11:04    1.74
+++ srclib/apr/network_io/unix/sendrecv.c    2001/07/27 23:47:50
@@ -125,14 +125,21 @@
 apr_status_t apr_recv(apr_socket_t *sock, char *buf, apr_size_t *len)
 {
     ssize_t rv;
-   
+    apr_status_t arv;
+
+    if (sock->netmask & APR_INCOMPLETE_READ) {
+    sock->netmask &= ~APR_INCOMPLETE_READ;
+        goto do_select;
+    }
+
     do {
         rv = read(sock->socketdes, buf, (*len));
     } while (rv == -1 && errno == EINTR);
 
     if (rv == -1 && (errno == EAGAIN || errno == EWOULDBLOCK) &&
       sock->timeout != 0) {
-        apr_status_t arv = apr_wait_for_io_or_timeout(sock, 1);
+do_select:
+    arv = apr_wait_for_io_or_timeout(sock, 1);
         if (arv != APR_SUCCESS) {
             *len = 0;
             return arv;
@@ -146,6 +153,9 @@
     if (rv == -1) {
         (*len) = 0;
         return errno;
+    }
+    if (sock->timeout && rv < *len) {
+    sock->netmask |= APR_INCOMPLETE_READ;
     }
     (*len) = rv;
     if (rv == 0) {
Index: server/protocol.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/protocol.c,v
retrieving revision 1.33
diff -u -r1.33 protocol.c
--- server/protocol.c    2001/07/24 22:55:29    1.33
+++ server/protocol.c    2001/07/27 23:47:51
@@ -588,6 +588,12 @@
                      (int)(keptalive
                      ? r->server->keep_alive_timeout * APR_USEC_PER_SEC
                      : r->server->timeout * APR_USEC_PER_SEC));
+
+    /* Do a select before the first read, rather than trying the
+     * read first (this is based on empirical observations that
+     * calling read first typically results in EAGAIN/EWOULDBLOCK)
+     */
+    apr_setsocketopt(conn->client_socket, APR_INCOMPLETE_READ, 1);
                     
     /* Get the request... */
     if (!read_request_line(r)) {
Index: server/connection.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/connection.c,v
retrieving revision 1.83
diff -u -r1.83 connection.c
--- server/connection.c    2001/07/12 03:20:48    1.83
+++ server/connection.c    2001/07/27 23:47:51
@@ -195,6 +195,7 @@
      */
     timeout = SECONDS_TO_LINGER * APR_USEC_PER_SEC;
     apr_setsocketopt(c->client_socket, APR_SO_TIMEOUT, timeout);
+    apr_setsocketopt(c->client_socket, APR_INCOMPLETE_READ, 1);
     for (;;) {
         nbytes = sizeof(dummybuf);
         rc = apr_recv(c->client_socket, dummybuf, &nbytes);