httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <b...@wstoddard.com>
Subject Re: cvs commit: httpd-2.0/server core.c
Date Tue, 01 May 2001 18:00:47 GMT
This patch is seriously broken.  Request a very large file (100MB or greater) and watch what
happens
to memory usage.

The problem is this loop. We basically read the entire content of the file into memory before
sending it out on the network. Haven't given much though on the best way to fix this.

>   +                APR_BRIGADE_FOREACH(bucket, b) {
>   +                    const char *str;
>   +                    apr_size_t n;
>   +
>   +                    rv = apr_bucket_read(bucket, &str, &n, APR_BLOCK_READ);
>   +                    apr_brigade_write(ctx->b, NULL, NULL, str, n);
>   +                }

Bill

----- Original Message -----
From: <rbb@apache.org>
To: <httpd-2.0-cvs@apache.org>
Sent: Sunday, April 29, 2001 1:05 PM
Subject: cvs commit: httpd-2.0/server core.c


> rbb         01/04/29 10:05:50
>
>   Modified:    .        CHANGES
>                server   core.c
>   Log:
>   Create Files, and thus MMAPs, out of the request pool, not the
>   connection pool.  This solves a small resource leak that had us
>   not closing files until a connection was closed.  In order to do
>   this, at the end of the core_output_filter, we loop through the
>   brigade and convert any data we have into a single HEAP bucket
>   that we know will survive clearing the request_rec.
>
>   Submitted by: Ryan Bloom, Justin Erenkrantz <jerenkrantz@ebuilt.com>,
>                   Cliff Woolley
>
>   Revision  Changes    Path
>   1.190     +9 -0      httpd-2.0/CHANGES
>
>   Index: CHANGES
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/CHANGES,v
>   retrieving revision 1.189
>   retrieving revision 1.190
>   diff -u -d -b -w -u -r1.189 -r1.190
>   --- CHANGES 2001/04/29 06:45:34 1.189
>   +++ CHANGES 2001/04/29 17:05:49 1.190
>   @@ -1,5 +1,14 @@
>    Changes with Apache 2.0.18-dev
>
>   +  *) Create Files, and thus MMAPs, out of the request pool, not the
>   +     connection pool.  This solves a small resource leak that had us
>   +     not closing files until a connection was closed.  In order to do
>   +     this, at the end of the core_output_filter, we loop through the
>   +     brigade and convert any data we have into a single HEAP bucket
>   +     that we know will survive clearing the request_rec.
>   +     [Ryan Bloom, Justin Erenkrantz <jerenkrantz@ebuilt.com>,
>   +      Cliff Woolley]
>   +
>      *) Completely revamp configure so that it preserves the standard make
>         variables CPPFLAGS, CFLAGS, CXXFLAGS, LDFLAGS and LIBS by moving
>         the configure additions to EXTRA_* variables.  Also, allow the user
>
>
>
>   1.10      +21 -4     httpd-2.0/server/core.c
>
>   Index: core.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/core.c,v
>   retrieving revision 1.9
>   retrieving revision 1.10
>   diff -u -d -b -w -u -r1.9 -r1.10
>   --- core.c 2001/04/22 22:19:32 1.9
>   +++ core.c 2001/04/29 17:05:49 1.10
>   @@ -2965,7 +2965,7 @@
>            return HTTP_METHOD_NOT_ALLOWED;
>        }
>
>   -    if ((status = apr_file_open(&fd, r->filename, APR_READ | APR_BINARY, 0,
r->connection->pool)) != APR_SUCCESS) {
>   +    if ((status = apr_file_open(&fd, r->filename, APR_READ | APR_BINARY, 0,
r->pool)) !=
APR_SUCCESS) {
>            ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
>         "file permissions deny server access: %s", r->filename);
>            return HTTP_FORBIDDEN;
>   @@ -3134,15 +3134,32 @@
>            if ((!fd && !more &&
>                 (nbytes < AP_MIN_BYTES_TO_WRITE) && !APR_BUCKET_IS_FLUSH(e))
>                || (APR_BUCKET_IS_EOS(e) && c->keepalive)) {
>   -
>                /* NEVER save an EOS in here.  If we are saving a brigade with
>                 * an EOS bucket, then we are doing keepalive connections, and
>                 * we want to process to second request fully.
>                 */
>                if (APR_BUCKET_IS_EOS(e)) {
>   -                apr_bucket_delete(e);
>   +                apr_bucket *bucket = NULL;
>   +                /* If we are in here, then this request is a keepalive.  We
>   +                 * need to be certain that any data in a bucket is valid
>   +                 * after the request_pool is cleared.
>   +                 */
>   +                if (ctx->b == NULL) {
>   +                    ctx->b = apr_brigade_create(f->c->pool);
>                }
>   +
>   +                APR_BRIGADE_FOREACH(bucket, b) {
>   +                    const char *str;
>   +                    apr_size_t n;
>   +
>   +                    rv = apr_bucket_read(bucket, &str, &n, APR_BLOCK_READ);
>   +                    apr_brigade_write(ctx->b, NULL, NULL, str, n);
>   +                }
>   +                apr_brigade_destroy(b);
>   +            }
>   +            else {
>                ap_save_brigade(f, &ctx->b, &b);
>   +            }
>                return APR_SUCCESS;
>            }
>
>
>
>


Mime
View raw message