On Wednesday 17 February 2010, Jeff Trawick wrote:
> > What about adding some code to apr-util's bucket debugging that
> > logs when destroyed brigades (i.e. where the pool cleanup has
> > been removed) are reused? I think this may be a common cause for
> > memory leaks.
>
> as in something like this?
>
> apr_brigade_destroy(foo->bb)
> ...
> if (!foo->bb) {
> foo->bb = apr_brigade_create();
> }
> ...
> APR_BRIGADE_INSERT_TAIL(foo->bb, b);
> /* no automatic cleanup of b */
Exactly. I have fixed a few of these in httpd trunk but I guess these
can appear in third party modules, too. And 2.2.x has these problems,
too.
One particular problem is that 2.2's core_output_filter may call
apr_brigade_destroy() on brigades which have been passed down the
filter chain and which may be later reused by the filter that has
created them. I have attached an (untested) patch for this issue. A
simpler variant would just change all three apr_brigade_destroy()
calls to apr_brigade_cleanup().
Index: server/core_filters.c
===================================================================
--- server/core_filters.c (Revision 910332)
+++ server/core_filters.c (Arbeitskopie)
@@ -655,6 +655,8 @@
/* Create a temporary brigade as a means
* of concatenating a bunch of buckets together
*/
+ temp_brig = apr_brigade_create(f->c->pool,
+ f->c->bucket_alloc);
if (last_merged_bucket) {
/* If we've concatenated together small
* buckets already in a previous pass,
@@ -667,16 +669,9 @@
* these buckets, so that the content
* in them doesn't have to be copied again.
*/
- apr_bucket_brigade *bb;
- bb = apr_brigade_split(b,
- APR_BUCKET_NEXT(last_merged_bucket));
- temp_brig = b;
- b = bb;
+ APR_BRIGADE_PREPEND(b, temp_brig);
+ brigade_move(temp_brig, b, APR_BUCKET_NEXT(last_merged_bucket));
}
- else {
- temp_brig = apr_brigade_create(f->c->pool,
- f->c->bucket_alloc);
- }
temp = APR_BRIGADE_FIRST(b);
while (temp != e) {
@@ -879,7 +874,7 @@
logio_add_bytes_out(c, bytes_sent);
}
- apr_brigade_destroy(b);
+ apr_brigade_cleanup(b);
/* drive cleanups for resources which were set aside
* this may occur before or after termination of the request which
@@ -910,7 +905,7 @@
"core_output_filter: writing data to the network");
if (more)
- apr_brigade_destroy(more);
+ apr_brigade_cleanup(more);
/* No need to check for SUCCESS, we did that above. */
if (!APR_STATUS_IS_EAGAIN(rv)) {
