httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: svn commit: r1035504 - in /httpd/httpd/trunk: include/ap_mmn.h modules/proxy/mod_proxy.h modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c
Date Tue, 16 Nov 2010 12:05:23 GMT
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
--


Mime
View raw message