mod_proxy_http: Allocate the fake backend request from a child pool

of the backend connection, instead of misusing the pool of the frontend
request. Fixes a thread safety issue where buckets set aside in the
backend connection leak into other threads, and then disappear when
the frontend request is cleaned up, in turn causing corrupted buckets
to make other threads spin.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1055250 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 8d25db4e6d09c111d834bac51000e0acf0bf5c27..8b23dce4ca6f213c7a1a10707c1294a2a9f3d97a 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,13 @@
 
 Changes with Apache 2.3.11
 
+  *) mod_proxy_http: Allocate the fake backend request from a child pool
+     of the backend connection, instead of misusing the pool of the frontend
+     request. Fixes a thread safety issue where buckets set aside in the
+     backend connection leak into other threads, and then disappear when
+     the frontend request is cleaned up, in turn causing corrupted buckets
+     to make other threads spin. [Graham Leggett]
+
   *) mod_ssl: Change the format of the SSL_{CLIENT,SERVER}_{I,S}_DN variables
      to be RFC 2253 compatible, convert non-ASCII characters to UTF8, and 
      escape other special characters with backslashes. The old format can

diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c
index 8b2d5f381501927257cd749f2daccfea9fc83417..29b2f1b517b62889ea7b2dfbf580bc08b1eb2247 100644 (file)
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -1397,7 +1397,6 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
     char buffer[HUGE_STRING_LEN];
     const char *buf;
     char keepchar;
-    request_rec *rp;
     apr_bucket *e;
     apr_bucket_brigade *bb, *tmp_bb;
     apr_bucket_brigade *pass_bb;
@@ -1448,21 +1447,21 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
      * filter chain
      */
 
-    rp = ap_proxy_make_fake_req(origin, r);
+    backend->r = ap_proxy_make_fake_req(origin, r);
     /* In case anyone needs to know, this is a fake request that is really a
      * response.
      */
-    rp->proxyreq = PROXYREQ_RESPONSE;
+    backend->r->proxyreq = PROXYREQ_RESPONSE;
     tmp_bb = apr_brigade_create(p, c->bucket_alloc);
     do {
         apr_status_t rc;
 
         apr_brigade_cleanup(bb);
 
-        rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), rp, 0, &len);
+        rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), backend->r, 0, &len);
         if (len == 0) {
             /* handle one potential stray CRLF */
-            rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), rp, 0, &len);
+            rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), backend->r, 0, &len);
         }
         if (len <= 0) {
             ap_log_rerror(APLOG_MARK, APLOG_ERR, rc, r,
@@ -1590,7 +1589,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
                          "Set-Cookie", NULL);
 
             /* shove the headers direct into r->headers_out */
-            ap_proxy_read_headers(r, rp, buffer, sizeof(buffer), origin,
+            ap_proxy_read_headers(r, backend->r, buffer, sizeof(buffer), origin,
                                   &pread_len);
 
             if (r->headers_out == NULL) {
@@ -1656,7 +1655,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
                 ap_set_content_type(r, apr_pstrdup(p, buf));
             }
             if (!ap_is_HTTP_INFO(proxy_status)) {
-                ap_proxy_pre_http_request(origin, rp);
+                ap_proxy_pre_http_request(origin, backend->r);
             }
 
             /* Clear hop-by-hop headers */
@@ -1798,7 +1797,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
             if (!r->header_only && /* not HEAD request */
                 (proxy_status != HTTP_NO_CONTENT) && /* not 204 */
                 (proxy_status != HTTP_NOT_MODIFIED)) { /* not 304 */
-                ap_discard_request_body(rp);
+                ap_discard_request_body(backend->r);
             }
             return proxy_status;
         }
@@ -1814,14 +1813,14 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
              * TE, so that they are preserved accordingly for
              * ap_http_filter to know where to end.
              */
-            rp->headers_in = apr_table_copy(r->pool, r->headers_out);
+            backend->r->headers_in = apr_table_copy(backend->r->pool, r->headers_out);
             /*
              * Restore Transfer-Encoding header from response if we saved
              * one before and there is none left. We need it for the
              * ap_http_filter. See above.
              */
-            if (te && !apr_table_get(rp->headers_in, "Transfer-Encoding")) {
-                apr_table_add(rp->headers_in, "Transfer-Encoding", te);
+            if (te && !apr_table_get(backend->r->headers_in, "Transfer-Encoding")) {
+                apr_table_add(backend->r->headers_in, "Transfer-Encoding", te);
             }
 
             apr_table_unset(r->headers_out,"Transfer-Encoding");
@@ -1853,7 +1852,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
                     apr_off_t readbytes;
                     apr_status_t rv;
 
-                    rv = ap_get_brigade(rp->input_filters, bb,
+                    rv = ap_get_brigade(backend->r->input_filters, bb,
                                         AP_MODE_READBYTES, mode,
                                         conf->io_buffer_size);
 

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 0a2b06df6c58602d17cbb843daa624349c173f01..1c4addf040f76b9d1cd697e02042269264df4e7b 100644 (file)
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -348,16 +348,20 @@ PROXY_DECLARE(const char *)
 
 PROXY_DECLARE(request_rec *)ap_proxy_make_fake_req(conn_rec *c, request_rec *r)
 {
-    request_rec *rp = apr_pcalloc(r->pool, sizeof(*r));
+    apr_pool_t *pool;
+
+    apr_pool_create(&pool, c->pool);
+
+    request_rec *rp = apr_pcalloc(pool, sizeof(*r));
 
-    rp->pool            = r->pool;
+    rp->pool            = pool;
     rp->status          = HTTP_OK;
 
-    rp->headers_in      = apr_table_make(r->pool, 50);
-    rp->subprocess_env  = apr_table_make(r->pool, 50);
-    rp->headers_out     = apr_table_make(r->pool, 12);
-    rp->err_headers_out = apr_table_make(r->pool, 5);
-    rp->notes           = apr_table_make(r->pool, 5);
+    rp->headers_in      = apr_table_make(pool, 50);
+    rp->subprocess_env  = apr_table_make(pool, 50);
+    rp->headers_out     = apr_table_make(pool, 12);
+    rp->err_headers_out = apr_table_make(pool, 5);
+    rp->notes           = apr_table_make(pool, 5);
 
     rp->server = r->server;
     rp->proxyreq = r->proxyreq;
@@ -368,7 +372,7 @@ PROXY_DECLARE(request_rec *)ap_proxy_make_fake_req(conn_rec *c, request_rec *r)
     rp->proto_output_filters  = c->output_filters;
     rp->proto_input_filters   = c->input_filters;
 
-    rp->request_config  = ap_create_request_config(r->pool);
+    rp->request_config  = ap_create_request_config(pool);
     proxy_run_create_req(r, rp);
 
     return rp;
@@ -1724,6 +1728,11 @@ static apr_status_t connection_cleanup(void *theconn)
         return APR_SUCCESS;
     }
 
+    if (conn->r) {
+        apr_pool_destroy(conn->r->pool);
+        conn->r = NULL;
+    }
+
 #if APR_HAS_THREADS
     /* Sanity check: Did we already return the pooled connection? */
     if (conn->inreslist) {