apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <...@apache.org>
Subject Re: [PATCH] Update to Brian's patch to allocate brigades out of the bucket allocator
Date Fri, 20 Dec 2002 16:57:50 GMT


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;
>
>
>



Mime
View raw message