fielding 97/01/29 18:43:02
Modified: src CHANGES buff.h buff.c http_main.c http_protocol.c
Log:
Improved buffered output to the client by delaying the flush decision
until the BUFF code is actually about to read the next request.
This fixes a problem introduced in 1.2b5 with clients that send
an extra CRLF after a POST request.
Submitted by: Dean Gaudet
Reviewed by: Roy Fielding
Revision Changes Path
1.140 +6 -1 apache/src/CHANGES
Index: CHANGES
===================================================================
RCS file: /export/home/cvs/apache/src/CHANGES,v
retrieving revision 1.139
retrieving revision 1.140
diff -C3 -r1.139 -r1.140
*** CHANGES 1997/01/30 02:27:06 1.139
--- CHANGES 1997/01/30 02:42:56 1.140
***************
*** 1,6 ****
! Changes with Apache 1.2b6
*) Continue persistent connection after 304 response. [Dean Gaudet]
Changes with Apache 1.2b6
--- 1,11 ----
! Changes with Apache 1.2b7
*) Continue persistent connection after 304 response. [Dean Gaudet]
+
+ *) Improved buffered output to the client by delaying the flush decision
+ until the BUFF code is actually about to read the next request.
+ This fixes a problem introduced in 1.2b5 with clients that send
+ an extra CRLF after a POST request. [Dean Gaudet]
Changes with Apache 1.2b6
1.11 +2 -1 apache/src/buff.h
Index: buff.h
===================================================================
RCS file: /export/home/cvs/apache/src/buff.h,v
retrieving revision 1.10
retrieving revision 1.11
diff -C3 -r1.10 -r1.11
*** buff.h 1997/01/25 22:37:09 1.10
--- buff.h 1997/01/30 02:42:57 1.11
***************
*** 68,73 ****
--- 68,75 ----
#define B_ERROR (48)
/* Use chunked writing */
#define B_CHUNK (64)
+ /* bflush() if a read would block */
+ #define B_SAFEREAD (128)
typedef struct buff_struct BUFF;
***************
*** 120,126 ****
extern int bvputs(BUFF *fb, ...);
extern int bprintf(BUFF *fb,const char *fmt,...);
extern int vbprintf(BUFF *fb,const char *fmt,va_list vlist);
- extern int btestread(BUFF *fb);
/* Internal routines */
extern int bflsbuf(int c, BUFF *fb);
--- 122,127 ----
1.16 +57 -49 apache/src/buff.c
Index: buff.c
===================================================================
RCS file: /export/home/cvs/apache/src/buff.c,v
retrieving revision 1.15
retrieving revision 1.16
diff -C3 -r1.15 -r1.16
*** buff.c 1997/01/25 22:37:10 1.15
--- buff.c 1997/01/30 02:42:57 1.16
***************
*** 175,193 ****
}
/*
! * Set a flag on (1) or off (0). Currently, these flags work
! * as a function of the bcwrite() function, so we make sure to
! * flush before setting them one way or the other; otherwise
! * writes could end up with the wrong flag.
*/
int bsetflag(BUFF *fb, int flag, int value)
{
! bflush(fb);
if (value) fb->flags |= flag;
else fb->flags &= ~flag;
return value;
}
/*
* Read up to nbyte bytes into buf.
* If fewer than byte bytes are currently available, then return those.
--- 175,239 ----
}
/*
! * Set a flag on (1) or off (0).
*/
int bsetflag(BUFF *fb, int flag, int value)
{
! if( flag & B_CHUNK ) {
! /*
! * This shouldn't be true for much longer -djg
! * Currently, these flags work
! * as a function of the bcwrite() function, so we make sure to
! * flush before setting them one way or the other; otherwise
! * writes could end up with the wrong flag.
! */
! bflush(fb);
! }
if (value) fb->flags |= flag;
else fb->flags &= ~flag;
return value;
}
+
+ /*
+ * This is called instead of read() everywhere in here. It implements
+ * the B_SAFEREAD functionality -- which is to force a flush() if a read()
+ * would block. It also deals with the EINTR errno result from read().
+ * return code is like read() except EINTR is eliminated.
+ */
+ static int
+ saferead( BUFF *fb, void *buf, int nbyte )
+ {
+ int rv;
+
+ if( fb->flags & B_SAFEREAD ) {
+ fd_set fds;
+ struct timeval tv;
+
+ /* test for a block */
+ do {
+ FD_ZERO( &fds );
+ FD_SET( fb->fd_in, &fds );
+ tv.tv_sec = 0;
+ tv.tv_usec = 0;
+ #ifdef HPUX
+ rv = select( fb->fd_in + 1, (int *)&fds, NULL, NULL, &tv );
+ #else
+ rv = select( fb->fd_in + 1, &fds, NULL, NULL, &tv );
+ #endif
+ } while( rv < 0 && errno == EINTR );
+ /* treat any error as if it would block as well */
+ if( rv != 1 ) {
+ bflush(fb);
+ }
+ }
+ do {
+ rv = read( fb->fd_in, buf, nbyte );
+ } while ( rv == -1 && errno == EINTR );
+ return( rv );
+ }
+
+
/*
* Read up to nbyte bytes into buf.
* If fewer than byte bytes are currently available, then return those.
***************
*** 204,211 ****
if (!(fb->flags & B_RD))
{
/* Unbuffered reading */
! do i = read(fb->fd_in, buf, nbyte);
! while (i == -1 && errno == EINTR);
if (i == -1 && errno != EAGAIN) doerror(fb, B_RD);
return i;
}
--- 250,256 ----
if (!(fb->flags & B_RD))
{
/* Unbuffered reading */
! i = saferead( fb, buf, nbyte );
if (i == -1 && errno != EAGAIN) doerror(fb, B_RD);
return i;
}
***************
*** 233,240 ****
if (nbyte >= fb->bufsiz)
{
/* read directly into buffer */
! do i = read(fb->fd_in, buf, nbyte);
! while (i == -1 && errno == EINTR);
if (i == -1)
{
if (nrd == 0)
--- 278,284 ----
if (nbyte >= fb->bufsiz)
{
/* read directly into buffer */
! i = saferead( fb, buf, nbyte );
if (i == -1)
{
if (nrd == 0)
***************
*** 248,255 ****
{
/* read into hold buffer, then memcpy */
fb->inptr = fb->inbase;
! do i = read(fb->fd_in, fb->inptr, fb->bufsiz);
! while (i == -1 && errno == EINTR);
if (i == -1)
{
if (nrd == 0)
--- 292,298 ----
{
/* read into hold buffer, then memcpy */
fb->inptr = fb->inbase;
! i = saferead( fb, fb->inptr, fb->bufsiz );
if (i == -1)
{
if (nrd == 0)
***************
*** 310,317 ****
fb->inptr = fb->inbase;
fb->incnt = 0;
if (fb->flags & B_EOF) break;
! do i = read(fb->fd_in, fb->inptr, fb->bufsiz);
! while (i == -1 && errno == EINTR);
if (i == -1)
{
buff[ct] = '\0';
--- 353,359 ----
fb->inptr = fb->inbase;
fb->incnt = 0;
if (fb->flags & B_EOF) break;
! i = saferead( fb, fb->inptr, fb->bufsiz );
if (i == -1)
{
buff[ct] = '\0';
***************
*** 381,389 ****
if (fb->flags & B_EOF)
return 0;
! do {
! i = read(fb->fd_in, fb->inptr, fb->bufsiz);
! } while (i == -1 && errno == EINTR);
if (i == -1) {
if (errno != EAGAIN)
--- 423,429 ----
if (fb->flags & B_EOF)
return 0;
! i = saferead( fb, fb->inptr, fb->bufsiz );
if (i == -1) {
if (errno != EAGAIN)
***************
*** 402,438 ****
}
/*
- * Tests if there is data to be read. Returns 0 if there is data,
- * -1 otherwise.
- */
- int
- btestread(BUFF *fb)
- {
- fd_set fds;
- struct timeval tv;
- int rv;
-
- /* the simple case, we've already got data in the buffer */
- if( fb->incnt ) {
- return( 0 );
- }
-
- /* otherwise see if the descriptor would block if we try to read it */
- do {
- FD_ZERO( &fds );
- FD_SET( fb->fd_in, &fds );
- tv.tv_sec = 0;
- tv.tv_usec = 0;
- #ifdef HPUX
- rv = select( fb->fd_in + 1, (int *)&fds, NULL, NULL, &tv );
- #else
- rv = select( fb->fd_in + 1, &fds, NULL, NULL, &tv );
- #endif
- } while( rv < 0 && errno == EINTR );
- return( rv == 1 ? 0 : -1 );
- }
-
- /*
* Skip data until a linefeed character is read
* Returns 1 on success, 0 if no LF found, or -1 on error
*/
--- 442,447 ----
***************
*** 464,471 ****
fb->inptr = fb->inbase;
fb->incnt = 0;
if (fb->flags & B_EOF) return 0;
! do i = read(fb->fd_in, fb->inptr, fb->bufsiz);
! while (i == -1 && errno == EINTR);
if (i == 0) fb->flags |= B_EOF;
if (i == -1 && errno != EAGAIN) doerror(fb, B_RD);
if (i == 0 || i == -1) return i;
--- 473,479 ----
fb->inptr = fb->inbase;
fb->incnt = 0;
if (fb->flags & B_EOF) return 0;
! i = saferead( fb, fb->inptr, fb->bufsiz );
if (i == 0) fb->flags |= B_EOF;
if (i == -1 && errno != EAGAIN) doerror(fb, B_RD);
if (i == 0 || i == -1) return i;
1.115 +0 -5 apache/src/http_main.c
Index: http_main.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_main.c,v
retrieving revision 1.114
retrieving revision 1.115
diff -C3 -r1.114 -r1.115
*** http_main.c 1997/01/26 21:11:53 1.114
--- http_main.c 1997/01/30 02:42:58 1.115
***************
*** 1675,1684 ****
if (r) increment_counts(child_num,r,1);
#endif
while (r && current_conn->keepalive) {
- /* If there's no request waiting for us to read then flush the
- * socket. This is to avoid tickling a bug in older Netscape
- * clients. */
- if( btestread(conn_io) == -1 ) bflush(conn_io);
destroy_pool(r->pool);
(void)update_child_status (child_num, SERVER_BUSY_KEEPALIVE,
(request_rec*)NULL);
--- 1675,1680 ----
***************
*** 2153,2159 ****
if (r) process_request (r); /* else premature EOF (ignore) */
while (r && conn->keepalive) {
- if( btestread(cio) == -1 ) bflush(cio);
destroy_pool(r->pool);
r = read_request (conn);
if (r) process_request (r);
--- 2149,2154 ----
1.95 +16 -1 apache/src/http_protocol.c
Index: http_protocol.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_protocol.c,v
retrieving revision 1.94
retrieving revision 1.95
diff -C3 -r1.94 -r1.95
*** http_protocol.c 1997/01/30 02:27:07 1.94
--- http_protocol.c 1997/01/30 02:42:58 1.95
***************
*** 520,530 ****
/* Read past empty lines until we get a real request line,
* a read error, the connection closes (EOF), or we timeout.
*/
while ((len = getline(l, HUGE_STRING_LEN, conn->client, 0)) <= 0) {
! if ((len < 0) || bgetflag(conn->client, B_EOF))
return 0;
}
if (len == (HUGE_STRING_LEN - 1))
return 0; /* Should be a 414 error status instead */
--- 520,545 ----
/* Read past empty lines until we get a real request line,
* a read error, the connection closes (EOF), or we timeout.
+ *
+ * We skip empty lines because browsers have to tack a CRLF on to the
end
+ * of POSTs to support old CERN webservers. But note that we may not
+ * have flushed any previous response completely to the client yet.
+ * We delay the flush as long as possible so that we can improve
+ * performance for clients that are pipelining requests. If a request
+ * is pipelined then we won't block during the (implicit) read() below.
+ * If the requests aren't pipelined, then the client is still waiting
+ * for the final buffer flush from us, and we will block in the implicit
+ * read(). B_SAFEREAD ensures that the BUFF layer flushes if it will
+ * have to block during a read.
*/
+ bsetflag( conn->client, B_SAFEREAD, 1 );
while ((len = getline(l, HUGE_STRING_LEN, conn->client, 0)) <= 0) {
! if ((len < 0) || bgetflag(conn->client, B_EOF)) {
! bsetflag( conn->client, B_SAFEREAD, 0 );
return 0;
+ }
}
+ bsetflag( conn->client, B_SAFEREAD, 0 );
if (len == (HUGE_STRING_LEN - 1))
return 0; /* Should be a 414 error status instead */
