On 16 Nov 2010, at 12:17 PM, Plüm, Rüdiger, VF-Group wrote:
Sorry for having been in grumpy mode this morning, but I saw this code
which is what I pointed out before to be not working :-).
As you've pointed out, the code is definitely wrong, I suspect you
choked on your coffee and for that I apologise :)
That looks like a plan. *Before* we sent the final brigade (which
had its
buckets transformed to the transient buckets of c->bucket_alloc)
containing the EOS bucket do a setaside on each bucket in this brigade
(maybe via ap_save_brigade).
That is what the new patch did, the mistake I made was to think that
the forced setaside could replace the wrapping in transient buckets,
when in reality it needed to be done in addition to the wrapping in
transient buckets.
This is the part of the original patch that does it:
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (revision 1035578)
+++ modules/proxy/mod_proxy_http.c (working copy)
@@ -1902,7 +1902,6 @@
/* Switch the allocator lifetime of the buckets */
ap_proxy_buckets_lifetime_transform(r, bb,
pass_bb);
- apr_brigade_cleanup(bb);
/* found the last brigade? */
if
(APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(pass_bb))) {
@@ -1910,6 +1909,23 @@
/* signal that we must leave */
finish = TRUE;
+ /* the brigade may contain transient buckets
that contain
+ * data that lives only as long as the
backend connection.
+ * Force a setaside so these transient
buckets become heap
+ * buckets that live as long as the request.
+ */
+ for (e = APR_BRIGADE_FIRST(pass_bb); e
+ != APR_BRIGADE_SENTINEL(pass_bb); e
+ = APR_BUCKET_NEXT(e)) {
+ apr_bucket_setaside(e, r->pool);
+ }
+
+ /* finally it is safe to clean up the brigade
from the
+ * connection pool, as we have forced a
setaside on all
+ * buckets.
+ */
+ apr_brigade_cleanup(bb);
+
/* make sure we release the backend
connection as soon
* as we know we are done, so that the
backend isn't
* left waiting for a slow client to
eventually
@@ -1930,6 +1946,7 @@
/* make sure we always clean up after ourselves */
apr_brigade_cleanup(pass_bb);
+ apr_brigade_cleanup(bb);
} while (!finish);
}
Regards,
Graham
--
