apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <brian.p...@cnet.com>
Subject Re: [PATCH] Update to Brian's patch to allocate brigades out of the bucket allocator
Date Fri, 20 Dec 2002 19:29:49 GMT
On Fri, 2002-12-20 at 11:24, rbb@apache.org wrote:

> Ummmm, I don't believe that it is possible for a brigade to outlive the
> transaction that created it, especially not when you look at how brigades
> are used in the web server.

Sure it can.  An output brigade created in a subrequest, for
example, may not be sent until after the subrequest has gone
out of scope.  Currently, we have to set aside the buckets
into a new brigade with a longer lifetime.  That's error-prone
(based on historical patterns) and adds overhead, but today it
works.  It won't work if the handler and the output filter are
running in different threads in the future, though.

>   Basically, a handler creates a brigade and
> passes it down the filter stack.  Each filter in the line is then allowed
> to either concatenate that brigade to it's own brigade or keep sending it
> down the stack.  At the bottom of the stack, the brigade must have been
> generated out of the connection pool, which by definition lives as long as
> the transaction that created it.
> 
> Basically, if you received a brigade from a higher filter, you can only
> assume that it will survive a single trip down the filter stack.  That is
> why all filters must create their own brigade to save data between calls.
> 
> > > By allocating bucket_brigades out of the bucket_allocator, we have removed
> > > the protection that the pool cleanups used to give us.
> >
> > It's important to keep in mind just what sort of protection
> > the pool-based brigade allocation offered.  It allowed code
> > that did this:
> >
> >     bb = apr_brigade_create(...);
> >     apr_brigade_destroy(bb);
> >     APR_BRIGADE_INSERT_HEAD(bb, bucket);
> >
> > to work accidentally.  I'm hesitant to call that "protection."
> 
> Give it up, that is a side-effect of pools, and it isn't the protection
> that we discussed when designing buckets and brigades.  The protection
> that we are talking about is against memory leaks, which this change
> almost definately opened up in the web server.  There is at least one
> place in the code where we drop a bucket brigade on the floor without
> cleaning it up, because we rely on the pool_cleanup to do it for us.  This
> change removes that ability.

The change doesn't remove that ability at all.  Both my patch
and Bill's variant allow for a pool cleanup to destroy a brigade.

Brian



Mime
View raw message