paul at querna
Oct 4, 2009, 1:14 AM
Post #1 of 7
(188 views)
Permalink
|
|
Re: svn commit: r821477 - in /httpd/httpd/trunk: CHANGES modules/http/byterange_filter.c modules/http/http_filters.c server/core_filters.c
|
|
On Sun, Oct 4, 2009 at 1:08 AM, <sf[at]apache.org> 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/%3C9ccfb16df2220fc31c836bb7cd640554[at]ricilake.net%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
>
>
>
|
