From 3bb41abee5dc727a757cb6ca3f4b0b86f500c605 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 23 Jan 2012 15:24:46 +0000 Subject: [PATCH] Merge r1234848 from trunk: * server/core_filters.c (ap_core_output_filter): Don't read the entire output of a morphing bucket into RAM. Submitted by: jorton, sf git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1234854 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 3 +++ server/core_filters.c | 62 ++++++++++++++++++++++++++++--------------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/CHANGES b/CHANGES index 4ceb1e4e20..aa469c67f3 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,9 @@ Changes with Apache 2.4.1 when no custom ErrorDocument is specified for status code 400. [Eric Covener] + *) core: Fix memory consumption in core output filter with streaming + bucket types like CGI or PIPE. [Joe Orton, Stefan Fritsch] + *) configure: Disable modules at configure time if a prerequisite module is not enabled. PR 52487. [Stefan Fritsch] diff --git a/server/core_filters.c b/server/core_filters.c index 2c24b13a18..bd1b41e062 100644 --- a/server/core_filters.c +++ b/server/core_filters.c @@ -368,7 +368,7 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb) apr_bucket_brigade *bb = NULL; apr_bucket *bucket, *next, *flush_upto = NULL; apr_size_t bytes_in_brigade, non_file_bytes_in_brigade; - int eor_buckets_in_brigade; + int eor_buckets_in_brigade, morphing_bucket_in_brigade; apr_status_t rv; /* Fail quickly if the connection has already been aborted. */ @@ -414,7 +414,7 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb) } /* Scan through the brigade and decide whether to attempt a write, - * based on the following rules: + * and how much to write, based on the following rules: * * 1) The new_bb is null: Do a nonblocking write of as much as * possible: do a nonblocking write of as much data as possible, @@ -423,10 +423,13 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb) * completion and has just determined that this connection * is writable.) * - * 2) The brigade contains a flush bucket: Do a blocking write + * 2) Determine if and up to which bucket we need to do a blocking + * write: + * + * a) The brigade contains a flush bucket: Do a blocking write * of everything up that point. * - * 3) The request is in CONN_STATE_HANDLER state, and the brigade + * b) The request is in CONN_STATE_HANDLER state, and the brigade * contains at least THRESHOLD_MAX_BUFFER bytes in non-file * buckets: Do blocking writes until the amount of data in the * buffer is less than THRESHOLD_MAX_BUFFER. (The point of this @@ -434,14 +437,25 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb) * streaming out lots of data faster than the data can be * sent to the client.) * - * 4) The request is in CONN_STATE_HANDLER state, and the brigade + * c) The request is in CONN_STATE_HANDLER state, and the brigade * contains at least MAX_REQUESTS_IN_PIPELINE EOR buckets: * Do blocking writes until less than MAX_REQUESTS_IN_PIPELINE EOR * buckets are left. (The point of this rule is to prevent too many * FDs being kept open by pipelined requests, possibly allowing a * DoS). * - * 5) The brigade contains at least THRESHOLD_MIN_WRITE + * d) The brigade contains a morphing bucket: If there was no other + * reason to do a blocking write yet, try reading the bucket. If its + * contents fit into memory before THRESHOLD_MAX_BUFFER is reached, + * everything is fine. Otherwise we need to do a blocking write the + * up to and including the morphing bucket, because ap_save_brigade() + * would read the whole bucket into memory later on. + * + * 3) Actually do the blocking write up to the last bucket determined + * by rules 2a-d. The point of doing only one flush is to make as + * few calls to writev() as possible. + * + * 4) If the brigade contains at least THRESHOLD_MIN_WRITE * bytes: Do a nonblocking write of as much data as possible, * then save the rest in ctx->buffered_bb. */ @@ -463,40 +477,43 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb) bytes_in_brigade = 0; non_file_bytes_in_brigade = 0; eor_buckets_in_brigade = 0; + morphing_bucket_in_brigade = 0; + for (bucket = APR_BRIGADE_FIRST(bb); bucket != APR_BRIGADE_SENTINEL(bb); bucket = next) { next = APR_BUCKET_NEXT(bucket); if (!APR_BUCKET_IS_METADATA(bucket)) { if (bucket->length == (apr_size_t)-1) { - const char *data; - apr_size_t length; - /* XXX support nonblocking read here? */ - rv = apr_bucket_read(bucket, &data, &length, APR_BLOCK_READ); - if (rv != APR_SUCCESS) { - return rv; - } - /* reading may have split the bucket, so recompute next: */ - next = APR_BUCKET_NEXT(bucket); + /* + * A setaside of morphing buckets would read everything into + * memory. Instead, we will flush everything up to and + * including this bucket. + */ + morphing_bucket_in_brigade = 1; } - bytes_in_brigade += bucket->length; - if (!APR_BUCKET_IS_FILE(bucket)) { - non_file_bytes_in_brigade += bucket->length; + else { + bytes_in_brigade += bucket->length; + if (!APR_BUCKET_IS_FILE(bucket)) + non_file_bytes_in_brigade += bucket->length; } } else if (AP_BUCKET_IS_EOR(bucket)) { eor_buckets_in_brigade++; } - if (APR_BUCKET_IS_FLUSH(bucket) || - (non_file_bytes_in_brigade >= THRESHOLD_MAX_BUFFER) || - (eor_buckets_in_brigade > MAX_REQUESTS_IN_PIPELINE) ) - { + if (APR_BUCKET_IS_FLUSH(bucket) + || non_file_bytes_in_brigade >= THRESHOLD_MAX_BUFFER + || morphing_bucket_in_brigade + || eor_buckets_in_brigade > MAX_REQUESTS_IN_PIPELINE) { + /* this segment of the brigade MUST be sent before returning. */ + if (APLOGctrace6(c)) { char *reason = APR_BUCKET_IS_FLUSH(bucket) ? "FLUSH bucket" : (non_file_bytes_in_brigade >= THRESHOLD_MAX_BUFFER) ? "THRESHOLD_MAX_BUFFER" : + morphing_bucket_in_brigade ? "morphing bucket" : "MAX_REQUESTS_IN_PIPELINE"; ap_log_cerror(APLOG_MARK, APLOG_TRACE6, 0, c, "core_output_filter: flushing because of %s", @@ -510,6 +527,7 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb) bytes_in_brigade = 0; non_file_bytes_in_brigade = 0; eor_buckets_in_brigade = 0; + morphing_bucket_in_brigade = 0; } } -- 2.40.0