On 10 Nov 2010, at 9:02 AM, Ruediger Pluem wrote:
The fix in r1030855 is wrong: ap_proxy_buckets_lifetime_transform is
not copying the data but only creates transient buckets from the data
in the buckets in bb. If you then destroy bb before passing pass_bb,
the data where the buckets in pass_bb point gets freed and later gets
overwritten.
Good catch. I reviewed the code again and then remembered the idea why
we use transient buckets: If everything goes well and the data is
sent out
by ap_pass_brigade no copying of the contents of the buckets is
needed.
Only if things are buffered somewhere down the chain, the according
filter
needs to set the buckets aside (which causes copying).
So I guess with the approach to release the backend connection
immediately
we will loose this advantage. That is regrettable.
I guess an easy solution would be to use heap buckets instead of
transient
buckets.
Have we not created a pool lifetime problem for ourselves here?
In theory, any attempt to read from the backend connection should
create buckets allocated from the r->connection->bucket_alloc
allocator, which should be removed from the backend connection when
the backend connection is returned to the pool.
Instead, it seems we create our own bucket_alloc from the backend
connection's pool, and then we only work because we're holding onto
the backend until we've stopped writing buckets - far longer than we
should be, and all along we've been working, but only by accident.
Regards,
Graham
--
