On Sun, Oct 4, 2009 at 1:08 AM, <[email protected]> wrote: > Author: sf > Date: Sun Oct 4 08:08:50 2009 > New Revision: 821477 > > URL: http://svn.apache.org/viewvc?rev=821477&view=rev > Log: > Make sure to not destroy bucket brigades that have been created by earlier > filters. Otherwise the pool cleanups would be removed causing potential memory > leaks later on. >
I am not sure these changes make sense. The 'traditional' API view says that brigades passed down the output fitler chain should be destroyed, not cleared -- please see the thread started with this message: <http://mail-archives.apache.org/mod_mbox/httpd-dev/200504.mbox/%[email protected]%3e> <> > Modified: > httpd/httpd/trunk/CHANGES > httpd/httpd/trunk/modules/http/byterange_filter.c > httpd/httpd/trunk/modules/http/http_filters.c > httpd/httpd/trunk/server/core_filters.c > > Modified: httpd/httpd/trunk/CHANGES > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=821477&r1=821476&r2=821477&view=diff > ============================================================================== > --- httpd/httpd/trunk/CHANGES [utf-8] (original) > +++ httpd/httpd/trunk/CHANGES [utf-8] Sun Oct 4 08:08:50 2009 > @@ -10,6 +10,10 @@ > mod_proxy_ftp: NULL pointer dereference on error paths. > [Stefan Fritsch <sf fritsch.de>, Joe Orton] > > + *) core: Fix potential memory leaks by making sure to not destroy > + bucket brigades that have been created by earlier filters. > + [Stefan Fritsch] > + > *) core, mod_deflate, mod_sed: Reduce memory usage by reusing bucket > brigades in several places. [Stefan Fritsch] > > > Modified: httpd/httpd/trunk/modules/http/byterange_filter.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/byterange_filter.c?rev=821477&r1=821476&r2=821477&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/http/byterange_filter.c (original) > +++ httpd/httpd/trunk/modules/http/byterange_filter.c Sun Oct 4 08:08:50 2009 > @@ -307,7 +307,7 @@ > APR_BRIGADE_INSERT_TAIL(bsend, e); > > /* we're done with the original content - all of our data is in bsend. */ > - apr_brigade_destroy(bb); > + apr_brigade_cleanup(bb); > > /* send our multipart output */ > return ap_pass_brigade(f->next, bsend); > > Modified: httpd/httpd/trunk/modules/http/http_filters.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=821477&r1=821476&r2=821477&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/http/http_filters.c (original) > +++ httpd/httpd/trunk/modules/http/http_filters.c Sun Oct 4 08:08:50 2009 > @@ -1112,7 +1112,7 @@ > ctx = f->ctx = apr_pcalloc(r->pool, sizeof(header_filter_ctx)); > } > else if (ctx->headers_sent) { > - apr_brigade_destroy(b); > + apr_brigade_cleanup(b); > return OK; > } > } > @@ -1283,7 +1283,7 @@ > ap_pass_brigade(f->next, b2); > > if (r->header_only) { > - apr_brigade_destroy(b); > + apr_brigade_cleanup(b); > ctx->headers_sent = 1; > return OK; > } > > Modified: httpd/httpd/trunk/server/core_filters.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core_filters.c?rev=821477&r1=821476&r2=821477&view=diff > ============================================================================== > --- httpd/httpd/trunk/server/core_filters.c (original) > +++ httpd/httpd/trunk/server/core_filters.c Sun Oct 4 08:08:50 2009 > @@ -316,7 +316,7 @@ > static void setaside_remaining_output(ap_filter_t *f, > core_output_filter_ctx_t *ctx, > apr_bucket_brigade *bb, > - int make_a_copy, conn_rec *c); > + conn_rec *c); > > static apr_status_t send_brigade_nonblocking(apr_socket_t *s, > apr_bucket_brigade *bb, > @@ -392,19 +392,21 @@ > } > } > > + if (new_bb != NULL) { > + bb = new_bb; > + } > + > if ((ctx->buffered_bb != NULL) && > !APR_BRIGADE_EMPTY(ctx->buffered_bb)) { > - bb = ctx->buffered_bb; > - ctx->buffered_bb = NULL; > if (new_bb != NULL) { > - APR_BRIGADE_CONCAT(bb, new_bb); > + APR_BRIGADE_PREPEND(bb, ctx->buffered_bb); > + } > + else { > + bb = ctx->buffered_bb; > } > c->data_in_output_filters = 0; > } > - else if (new_bb != NULL) { > - bb = new_bb; > - } > - else { > + else if (new_bb == NULL) { > return APR_SUCCESS; > } > > @@ -444,7 +446,7 @@ > /* The client has aborted the connection */ > c->aborted = 1; > } > - setaside_remaining_output(f, ctx, bb, 0, c); > + setaside_remaining_output(f, ctx, bb, c); > return rv; > } > > @@ -508,14 +510,14 @@ > } > } > > - setaside_remaining_output(f, ctx, bb, 1, c); > + setaside_remaining_output(f, ctx, bb, c); > return APR_SUCCESS; > } > > static void setaside_remaining_output(ap_filter_t *f, > core_output_filter_ctx_t *ctx, > apr_bucket_brigade *bb, > - int make_a_copy, conn_rec *c) > + conn_rec *c) > { > if (bb == NULL) { > return; > @@ -523,20 +525,14 @@ > remove_empty_buckets(bb); > if (!APR_BRIGADE_EMPTY(bb)) { > c->data_in_output_filters = 1; > - if (make_a_copy) { > + if (bb != ctx->buffered_bb) { > /* XXX should this use a separate deferred write pool, like > * the original ap_core_output_filter? > */ > ap_save_brigade(f, &(ctx->buffered_bb), &bb, c->pool); > - apr_brigade_destroy(bb); > - } > - else { > - ctx->buffered_bb = bb; > + apr_brigade_cleanup(bb); > } > } > - else { > - apr_brigade_destroy(bb); > - } > } > > #ifndef APR_MAX_IOVEC_SIZE > > >
