On 01/05/2011 01:23 AM, minf...@apache.org wrote: > Author: minfrin > Date: Wed Jan 5 00:23:43 2011 > New Revision: 1055250 > > URL: http://svn.apache.org/viewvc?rev=1055250&view=rev > Log: > 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. > > Modified: > httpd/httpd/trunk/CHANGES > httpd/httpd/trunk/modules/proxy/mod_proxy_http.c > httpd/httpd/trunk/modules/proxy/proxy_util.c >
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1055250&r1=1055249&r2=1055250&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original) > +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Wed Jan 5 00:23:43 2011 > @@ -1448,21 +1447,21 @@ apr_status_t ap_proxy_http_process_respo > * filter chain > */ > > - rp = ap_proxy_make_fake_req(origin, r); > + backend->r = ap_proxy_make_fake_req(origin, r); What about the comment in mod_proxy.h about the r element of proxy_conn_rec? /* Request record of the frontend request * which the backend currently answers. */ Doesn't this comment need to be adjusted now? > /* 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, > @@ -1814,14 +1813,14 @@ apr_status_t ap_proxy_http_process_respo > * 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); Doesn't that mean that we will get entries in backend->r->headers that have been allocated from r->pool instead of backend->r->pool? > /* > * 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); Doesn't that mean that we will get entries in backend->r->headers that have been allocated from r->pool instead of backend->r->pool? te is an element from r->headers_out. > } > > apr_table_unset(r->headers_out,"Transfer-Encoding"); > Regards Rüdiger
