I am actually pretty sure that allocating brigades out of the
bucket_allocator is a VERY big mistake. In fact, I am close to asking
that change to be backed out because it is too dangerous.
When we first designed buckets and bucket brigades, we made one VERY clear
distinction. Bucket_brigades are allocated out of a pool, because that
stops us from leaking memory. Buckets shouldn't be allocated out of a
pool, because nobody knows which pool to allocate them from. Basically,
we said that in Apache, it is safe to just drop a bucket briade because
the pool cleanups will free the memory. It is not safe to just drop a
bucket, it must either be left on the brigade to be cleaned up by the
brigade_cleanup function, or it must be destroyed when it is removed from
the brigade.
By allocating bucket_brigades out of the bucket_allocator, we have removed
the protection that the pool cleanups used to give us. I may be wrong
about how the bucket_allocator works, but I don't believe I am.
Basically, this change will make Apache leak memory on some requests, and
it goes against the original design of buckets and bucket brigades. In
fact, allocating the brigades out of a pool was a requirement, because
some people at the meeting were afraid of memory leaks with bucket
brigades.
Ryan
On Fri, 20 Dec 2002, Bill Stoddard wrote:
> If a pool is passed to apr_brigade_create, the brigade is allocated out of the
> pool. If the pool is NULL, the brigade is allocated out of the bucket
> allocator.
> We don't want a pool pointer hanging around in a brigade allocated out of the
> bucket allocator. That's just asking for trouble. This patch makes how the
> brigade is allocated, either out of the pool or out of the allocator,
> explicit.
>
> Index: apr_brigade.c
> ===================================================================
> RCS file: /home/cvs/apr-util/buckets/apr_brigade.c,v
> retrieving revision 1.55
> diff -u -r1.55 apr_brigade.c
> --- apr_brigade.c 17 Dec 2002 19:16:39 -0000 1.55
> +++ apr_brigade.c 20 Dec 2002 15:17:58 -0000
> @@ -95,7 +95,9 @@
> apr_pool_cleanup_kill(b->p, b, brigade_cleanup);
> }
> rv = apr_brigade_cleanup(b);
> - apr_bucket_free(b);
> + if (!b->p) {
> + apr_bucket_free(b);
> + }
> return rv;
> }
>
> @@ -104,7 +106,12 @@
> {
> apr_bucket_brigade *b;
>
> - b = apr_bucket_alloc(sizeof(*b), list);
> + if (p) {
> + b = apr_palloc(p, sizeof(*b));
> + }
> + else {
> + b = apr_bucket_alloc(sizeof(*b), list);
> + }
> b->p = p;
> b->bucket_alloc = list;
>
>
>
