httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject [PATCH] ap_proxy_http_process_response double lifetime transform
Date Thu, 12 Dec 2013 17:45:01 GMT
Hi devs,

This was pointed out by Joe Orton's comment at
https://issues.apache.org/bugzilla/show_bug.cgi?id=50335#c40.

Here is a proposal (patch against ap_proxy_http_process_response) to
address the double lifetime transformation of the buckets from the backend
when its connection is released early (on EOS, before the last buckets are
forwarded to the client).

Regards,
Yann.

Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c    (revision 1550128)
+++ modules/proxy/mod_proxy_http.c    (working copy)
@@ -1729,11 +1729,9 @@ int ap_proxy_http_process_response(apr_pool_t * p,
                         break;
                     }

-                    /* Switch the allocator lifetime of the buckets */
-                    proxy_buckets_lifetime_transform(r, bb, pass_bb);
-
                     /* found the last brigade? */
-                    if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(pass_bb))) {
+                    if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) {
+                        apr_bucket_brigade *swap_bb;

                         /* signal that we must leave */
                         finish = TRUE;
@@ -1743,17 +1741,18 @@ int ap_proxy_http_process_response(apr_pool_t * p,
                          * 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)) {
+                        for (e = APR_BRIGADE_FIRST(bb);
+                                e != APR_BRIGADE_SENTINEL(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.
+                        /* swap bb and pass_bb to keep the logic below,
where
+                         * pass_bb is the one finally passed to the client.
                          */
-                        apr_brigade_cleanup(bb);
+                        swap_bb = bb;
+                        bb = pass_bb;
+                        pass_bb = swap_bb;

                         /* make sure we release the backend connection as
soon
                          * as we know we are done, so that the backend
isn't
@@ -1764,8 +1763,11 @@ int ap_proxy_http_process_response(apr_pool_t * p,
                                 backend, r->server);
                         /* Ensure that the backend is not reused */
                         *backend_ptr = NULL;
-
                     }
+                    else {
+                        /* Switch the allocator lifetime of the buckets */
+                        proxy_buckets_lifetime_transform(r, bb, pass_bb);
+                    }

                     /* try send what we read */
                     if (ap_pass_brigade(r->output_filters, pass_bb) !=
APR_SUCCESS
[EOS]

Mime
View raw message