httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dean Gaudet <dgau...@arctic.org>
Subject [PATCH] buff.c bug fix
Date Sat, 25 Jan 1997 07:28:07 GMT

On Fri, 24 Jan 1997, Roy T. Fielding wrote:

>   * report from some folks at w3c.org about pipelined requests
>        Info: http://www.w3.org/pub/WWW/Protocols/HTTP/Performance/Apache.html
>        Status: deferred til after 1.2b5, maybe beyond 1.2

I've been looking into this one.

So far I can see a definite bug in bcwrite().  It's probably due to
the assumption that if r=write(f,b,n) then r is one of -1, or n unless
f is non-blocking.  That isn't the case, all values -1 <= r <= n are
possible for any f.  I notice later on in bwrite() that it also indicates
that assumption... but the bwrite() code looks ok on my first reading.

In particular, looking through the linux kernel code (the w3 report used
linux 2.0.18 for the server) I notice that it's possible if the tcp stack
can't get memory during a write() and has to block to do it but received
an unblocked signal then it returns a partial write().  There are also
other cases that will return partial write()s. 

Included is a patch that fixes bcwrite().  (A better performance patch
would use writev() to write the three pieces.)  I've lightly tested it...
although I'm not really sure how to test chunked encoding.

Also included in this patch is a performance fix for a case where bwrite()
would return while having a full buffer.

Plus a fix to remove an unused variable in bflush().

Dean

Index: buff.c
===================================================================
RCS file: /export/home/cvs/apache/src/buff.c,v
retrieving revision 1.14
diff -c -3 -r1.14 buff.c
*** buff.c	1997/01/20 04:28:07	1.14
--- buff.c	1997/01/25 07:21:39
***************
*** 469,474 ****
--- 469,502 ----
      else return buf[0];
  }
  
+ 
+ /*
+  * When doing chunked encodings we really have to write everything in the
+  * chunk before proceeding onto anything else.  This routine either writes
+  * nbytes and returns 0 or returns -1 indicating a failure.
+  *
+  * This is *seriously broken* if used on a non-blocking fd.  It will poll.
+  */
+ static int
+ write_it_all( int fd, const void *buf, int nbyte ) {
+ 
+     int i;
+ 
+     while( nbyte > 0 ) {
+ 	i = write( fd, buf, nbyte );
+ 	if( i == -1 ) {
+ 	    if( errno != EAGAIN && errno != EINTR ) {
+ 		return( -1 );
+ 	    }
+ 	} else {
+ 	    nbyte -= i;
+ 	    buf = i + (const char *)buf;
+ 	}
+     }
+     return( 0 );
+ }
+ 
+ 
  /*
   * A hook to write() that deals with chunking. This is really a protocol-
   * level issue, but we deal with it here because it's simpler; this is
***************
*** 476,493 ****
   * 2.0, using something like sfio stacked disciplines or BSD's funopen().
   */
  int bcwrite(BUFF *fb, const void *buf, int nbyte) {
-     int r;
  
      if (fb->flags & B_CHUNK) {
  	char chunksize[16];	/* Big enough for practically anything */
  
  	ap_snprintf(chunksize, sizeof(chunksize), "%x\015\012", nbyte);
! 	write(fb->fd, chunksize, strlen(chunksize));
      }
!     r = write(fb->fd, buf, nbyte);
!     if ((r > 0) && (fb->flags & B_CHUNK))
! 	write(fb->fd, "\015\012", 2);
!     return r;
  }
  
  /*
--- 504,526 ----
   * 2.0, using something like sfio stacked disciplines or BSD's funopen().
   */
  int bcwrite(BUFF *fb, const void *buf, int nbyte) {
  
      if (fb->flags & B_CHUNK) {
  	char chunksize[16];	/* Big enough for practically anything */
  
  	ap_snprintf(chunksize, sizeof(chunksize), "%x\015\012", nbyte);
! 	if( write_it_all(fb->fd, chunksize, strlen(chunksize)) == -1 ) {
! 	    return( -1 );
! 	}
! 	if( write_it_all(fb->fd, buf, nbyte) == -1 ) {
! 	    return( -1 );
! 	}
! 	if( write_it_all(fb->fd, "\015\012", 2) == -1 ) {
! 	    return( -1 );
! 	}
! 	return( nbyte );
      }
!     return( write(fb->fd, buf, nbyte) );
  }
  
  /*
***************
*** 573,579 ****
  /* we have emptied the file buffer. Now try to write the data from the
   * original buffer until there is less than bufsiz left
   */
!     while (nbyte > fb->bufsiz)
      {
  	do i = bcwrite(fb, buf, nbyte);
  	while (i == -1 && errno == EINTR);
--- 606,612 ----
  /* we have emptied the file buffer. Now try to write the data from the
   * original buffer until there is less than bufsiz left
   */
!     while (nbyte >= fb->bufsiz)
      {
  	do i = bcwrite(fb, buf, nbyte);
  	while (i == -1 && errno == EINTR);
***************
*** 611,617 ****
  int
  bflush(BUFF *fb)
  {
!     int i, j;
  
      if (!(fb->flags & B_WR) || (fb->flags & B_EOUT)) return 0;
  
--- 644,650 ----
  int
  bflush(BUFF *fb)
  {
!     int i;
  
      if (!(fb->flags & B_WR) || (fb->flags & B_EOUT)) return 0;
  
***************
*** 620,626 ****
      while (fb->outcnt > 0)
      {
  /* the buffer must be full */
- 	j = fb->outcnt;
  	do i = bcwrite(fb, fb->outbase, fb->outcnt);
  	while (i == -1 && errno == EINTR);
  	if (i > 0) fb->bytes_sent += i;
--- 653,658 ----



Mime
View raw message