httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r711886 - /httpd/httpd/branches/2.2.x/STATUS
Date Sat, 08 Nov 2008 11:14:23 GMT


On 11/07/2008 05:33 PM, Joe Orton wrote:
> On Fri, Nov 07, 2008 at 01:29:15PM +0100, "Plüm, Rüdiger, VF-Group" wrote:
>>> Would it be possible to substitute the backend ("fake") conn_rec's 
>>> ->bucket_alloc pointer with the "real" r->connection->bucket_alloc,

>>> for the duration of the request/response to the backend?  Wouldn't 
>>> that ensure that all the buckets in question exactly have the 
>>> correct lifetime?
>> I though about this as well, but I suppose this is a risky road to take.
>> Keep in mind that mod_ssl and the core network filters below might use this
>> bucket allocator and that we keep this backend connection alive while the front
>> end connection might have died. So we might still have buckets in this
>> backend connection pipe created by the previous request that are dead on the
>> new request. This may cause segfaults when we start using the backend
>> connection again.
> 
> Fair enough.  Transmuting the buckets into TRANSIENTs seems like a 
> reasonable solution, in any case, it's sort of saying "lifetime issues, 
> buyer beware" in bucket-speak which exactly matches the problem in hand.
> 
> In ap_proxy_buckets_lifetime_transform() is there any reason to restrict 
> to:
> 
> +        if (APR_BUCKET_IS_HEAP(e) || APR_BUCKET_IS_TRANSIENT(e)
> +            || APR_BUCKET_IS_IMMORTAL(e) || APR_BUCKET_IS_POOL(e)) {
> +            apr_bucket_read(e, &data, &bytes, APR_BLOCK_READ);
> 
> instead of simply:
> 
>          if (!APR_BUCKET_IS_METADATA(e)) {
>             // ...likewise transmute to a TRANSIENT
> 
> ?  You already know that the brigade size is constrained, right, so 
> there is no worry about memory consumption from morphing buckets, and 
> the technique should should work equally well for any data bucket?

This is a valid and good point I have not though about before :-).
Especially that the length of the brigade is limited. And it seems better
in this case to possibly do a sub optimal conversion of a data bucket
(like an MMAP bucket or especially a file bucket) than to not convert it at
all. So I adjusted this part of the code. Thanks for reviewing.

Regards

Rüdiger


Mime
View raw message