dgaudet 98/01/26 21:35:34
Modified: . STATUS
src CHANGES
src/main buff.c
Log:
really fix Rasmus' chunking problem this time
introduce LARGE_WRITE_THRESHOLD tunable for determining when large_write()
is a good idea
Reviewed by: Rasmus Lerdorf
Revision Changes Path
1.132 +1 -1 apachen/STATUS
Index: STATUS
===================================================================
RCS file: /export/home/cvs/apachen/STATUS,v
retrieving revision 1.131
retrieving revision 1.132
diff -u -r1.131 -r1.132
--- STATUS 1998/01/26 19:50:05 1.131
+++ STATUS 1998/01/27 05:35:22 1.132
@@ -70,7 +70,7 @@
* Dean's [PATCH] ap_snprintf should be more sane (fwd)
* Jim's/Ken's move of main/util_snprintf.c to ap/ap_snprintf.c
* [PATCH] Re: [BUGFIXES] Wrong GID for PID file and UMASK for logs
- * Dean's [PATCH] fix Rasmus' chunking error
+ * Dean's [PATCH] fix Rasmus' chunking error (take 2, really fix it)
* [PATCH] PR#1366: fix result of send_fd_length
* Ben Hyde's [PATCH] Finish suite of mutex ops for non-threaded platforms
* Ben Hyde's [PATCH] Serialize the update to pool.sub_* in destroy_pool
1.601 +15 -5 apachen/src/CHANGES
Index: CHANGES
===================================================================
RCS file: /export/home/cvs/apachen/src/CHANGES,v
retrieving revision 1.600
retrieving revision 1.601
diff -u -r1.600 -r1.601
--- CHANGES 1998/01/26 19:50:08 1.600
+++ CHANGES 1998/01/27 05:35:27 1.601
@@ -241,11 +241,21 @@
*) send_fd_length() did not calculate total_bytes_sent properly.
[Ben Reser <[EMAIL PROTECTED]>] PR#1366
- *) The large_write() changes tickled a bug in bputc(), this would
- show up as certain modules not working with Internet Explorer 4.0.
- Fix this bug, and also fix a performance bug related to bputc()
- causing a large_write() -- don't do large_write() unless there's
- at least two bytes to write. [Dean Gaudet]
+ *) The bputc() macro was not properly integrated with the chunking
+ code; in many cases modules using bputc() could cause completely
+ bogus chunked output. (Typically this will show up as problems
+ with Internet Explorer 4.0 reading a page, but other browsers
+ having no problem.) [Dean Gaudet]
+
+ *) Create LARGE_WRITE_THRESHOLD define which determines how many
+ bytes have to be supplied to bwrite() before it will consider
+ doing a writev() to assemble multiple buffers in one system
+ call. This is critical for modules such as mod_include,
+ mod_autoindex, mod_php3 which all use bputc()/bputs() of smaller
+ strings in some cases. The result would be extra effort
+ setting up writev(), and in many cases extra effort building
+ chunks. The default is 31, it can be override at compile
+ time. [Dean Gaudet]
*) Move the gid switching code into the child so that log files
and pid files are opened with the root gid.
1.60 +39 -12 apachen/src/main/buff.c
Index: buff.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/buff.c,v
retrieving revision 1.59
retrieving revision 1.60
diff -u -r1.59 -r1.60
--- buff.c 1998/01/26 16:46:07 1.59
+++ buff.c 1998/01/27 05:35:32 1.60
@@ -71,7 +71,21 @@
#include <bstring.h> /* for IRIX, FD_SET calls bzero() */
#endif
+#ifndef DEFAULT_BUFSIZE
#define DEFAULT_BUFSIZE (4096)
+#endif
+
+/* bwrite()s of greater than this size can result in a large_write() call,
+ * which can result in a writev(). It's a little more work to set up the
+ * writev() rather than copy bytes into the buffer, so we don't do it for
small
+ * writes. This is especially important when chunking (which is a very
likely
+ * source of small writes if it's a module using bputc/bputs)... because we
+ * have the expense of actually building two chunks for each writev().
+ */
+#ifndef LARGE_WRITE_THRESHOLD
+#define LARGE_WRITE_THRESHOLD 31
+#endif
+
/*
* Buffered I/O routines.
@@ -347,7 +361,13 @@
}
/*
- * start chunked encoding
+ * Start chunked encoding.
+ *
+ * Note that in order for bputc() to be an efficient macro we have to
guarantee
+ * that start_chunk() has always been called on the buffer before we leave
any
+ * routine in this file. Said another way, if a routine here uses
end_chunk()
+ * and writes something on the wire, then it has to call start_chunk() or set
+ * an error condition before returning.
*/
static void start_chunk(BUFF *fb)
{
@@ -865,16 +885,9 @@
API_EXPORT(int) bflsbuf(int c, BUFF *fb)
{
char ss[1];
- int rc;
ss[0] = c;
- rc = bwrite(fb, ss, 1);
- /* We do start_chunk() here so that the bputc macro can be smaller
- * and faster
- */
- if (rc == 1 && (fb->flags & B_CHUNK))
- start_chunk(fb);
- return rc;
+ return bwrite(fb, ss, 1);
}
/*
@@ -1069,7 +1082,6 @@
int nvec;
char chunksize[16];
- nvec = 0;
/* it's easiest to end the current chunk */
if (fb->flags & B_CHUNK) {
end_chunk(fb);
@@ -1103,7 +1115,13 @@
}
fb->outcnt = 0;
- return writev_it_all(fb, vec, nvec) ? -1 : nbyte;
+ if (writev_it_all(fb, vec, nvec)) {
+ return -1;
+ }
+ else if (fb->flags & B_CHUNK) {
+ start_chunk(fb);
+ }
+ return nbyte;
}
#endif
@@ -1154,7 +1172,8 @@
* us to use writev() too frequently. In those cases we really should just
* start a new buffer.
*/
- if (fb->outcnt > 0 && nbyte > 1 && nbyte + fb->outcnt >= fb->bufsiz) {
+ if (fb->outcnt > 0 && nbyte > LARGE_WRITE_THRESHOLD
+ && nbyte + fb->outcnt >= fb->bufsiz) {
return large_write(fb, buf, nbyte);
}
#endif
@@ -1215,6 +1234,10 @@
* original buffer until there is less than bufsiz left. Note that we
* use bcwrite() to do this for us, it will do the chunking so that
* we don't have to dink around building a chunk in our own buffer.
+ *
+ * Note also that bcwrite never does a partial write if we're chunking,
+ * so we're guaranteed to either end in an error state, or make it
+ * out of this loop and call start_chunk() below.
*/
while (nbyte >= fb->bufsiz) {
i = bcwrite(fb, buf, nbyte);
@@ -1279,6 +1302,10 @@
*/
if (fb->flags & B_EOUT)
return -1;
+ }
+
+ if (fb->flags & B_CHUNK) {
+ start_chunk(fb);
}
return 0;
