You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rp...@apache.org on 2007/12/12 08:44:03 UTC
svn commit: r603502 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Author: rpluem
Date: Tue Dec 11 23:44:02 2007
New Revision: 603502
URL: http://svn.apache.org/viewvc?rev=603502&view=rev
Log:
* Fix another memory leak related to PR 44026. Now that we keep the connection
data structure alive in the reslist, the live time of c->pool is too long.
r->pool has the correct live time since rp dies before r.
Modified:
httpd/httpd/trunk/modules/proxy/proxy_util.c
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=603502&r1=603501&r2=603502&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Dec 11 23:44:02 2007
@@ -332,16 +332,16 @@
PROXY_DECLARE(request_rec *)ap_proxy_make_fake_req(conn_rec *c, request_rec *r)
{
- request_rec *rp = apr_pcalloc(c->pool, sizeof(*r));
+ request_rec *rp = apr_pcalloc(r->pool, sizeof(*r));
- rp->pool = c->pool;
+ rp->pool = r->pool;
rp->status = HTTP_OK;
- rp->headers_in = apr_table_make(c->pool, 50);
- rp->subprocess_env = apr_table_make(c->pool, 50);
- rp->headers_out = apr_table_make(c->pool, 12);
- rp->err_headers_out = apr_table_make(c->pool, 5);
- rp->notes = apr_table_make(c->pool, 5);
+ 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->server = r->server;
rp->proxyreq = r->proxyreq;
@@ -352,7 +352,7 @@
rp->proto_output_filters = c->output_filters;
rp->proto_input_filters = c->input_filters;
- rp->request_config = ap_create_request_config(c->pool);
+ rp->request_config = ap_create_request_config(r->pool);
proxy_run_create_req(r, rp);
return rp;
Re: svn commit: r603502 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Dec 12, 2007, at 8:06 AM, Plüm, Rüdiger, VF-Group wrote:
>
>
>> -----Ursprüngliche Nachricht-----
>> Von: Jim Jagielski [mailto:jim@jaguNET.com]
>> Gesendet: Mittwoch, 12. Dezember 2007 13:59
>> An: dev@httpd.apache.org
>> Betreff: Re: svn commit: r603502 -
>> /httpd/httpd/trunk/modules/proxy/proxy_util.c
>>
>>
>>
>> On Dec 12, 2007, at 6:16 AM, Plüm, Rüdiger, VF-Group wrote:
>>
>>>
>>> The connection memory pool was a different memory pool
>> before. It was
>>> the memory pool of the front end connection. Now it is the
>> memory pool
>>> of the backend connection pool connection. See also
>>>
>>> http://svn.apache.org/viewvc?view=rev&revision=603237
>>> http://svn.apache.org/viewvc?view=rev&revision=602542
>>>
>>> If we would use r->connection->pool instead of r->pool it would be
>>> exactly the
>>> same as before the two revisions above, but regarding the pool
>>> livetimes I
>>> think r->connection->pool lives too long and thus using r->pool
>>> wastes less
>>> memory.
>>>
>>
>> This is all based on not even looking at these changes, so I
>> may be blowing smoke. But certainly the backend connection
>> pool lasts longer than the initial request that "bootstrapped"
>> that connection, right? So if we creating the backend stuff
>> out of r->pool, then for sure that can't be right...
>
> The backend connection: Yes, I agree.
> The backend request (rp): No, it lasts shorter than r, and it gets
> recreated for every new request.
>
OK, wanted to make sure that we agreed that there are 2 scopes
we need to be aware of ;)
Re: svn commit: r603502 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Plüm,
Rüdiger,
VF-Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Jim Jagielski [mailto:jim@jaguNET.com]
> Gesendet: Mittwoch, 12. Dezember 2007 13:59
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r603502 -
> /httpd/httpd/trunk/modules/proxy/proxy_util.c
>
>
>
> On Dec 12, 2007, at 6:16 AM, Plüm, Rüdiger, VF-Group wrote:
>
> >
> > The connection memory pool was a different memory pool
> before. It was
> > the memory pool of the front end connection. Now it is the
> memory pool
> > of the backend connection pool connection. See also
> >
> > http://svn.apache.org/viewvc?view=rev&revision=603237
> > http://svn.apache.org/viewvc?view=rev&revision=602542
> >
> > If we would use r->connection->pool instead of r->pool it would be
> > exactly the
> > same as before the two revisions above, but regarding the pool
> > livetimes I
> > think r->connection->pool lives too long and thus using r->pool
> > wastes less
> > memory.
> >
>
> This is all based on not even looking at these changes, so I
> may be blowing smoke. But certainly the backend connection
> pool lasts longer than the initial request that "bootstrapped"
> that connection, right? So if we creating the backend stuff
> out of r->pool, then for sure that can't be right...
The backend connection: Yes, I agree.
The backend request (rp): No, it lasts shorter than r, and it gets
recreated for every new request.
Regards
Rüdiger
Re: svn commit: r603502 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Dec 12, 2007, at 6:16 AM, Plüm, Rüdiger, VF-Group wrote:
>
> The connection memory pool was a different memory pool before. It was
> the memory pool of the front end connection. Now it is the memory pool
> of the backend connection pool connection. See also
>
> http://svn.apache.org/viewvc?view=rev&revision=603237
> http://svn.apache.org/viewvc?view=rev&revision=602542
>
> If we would use r->connection->pool instead of r->pool it would be
> exactly the
> same as before the two revisions above, but regarding the pool
> livetimes I
> think r->connection->pool lives too long and thus using r->pool
> wastes less
> memory.
>
This is all based on not even looking at these changes, so I
may be blowing smoke. But certainly the backend connection
pool lasts longer than the initial request that "bootstrapped"
that connection, right? So if we creating the backend stuff
out of r->pool, then for sure that can't be right...
Re: svn commit: r603502 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Plüm,
Rüdiger,
VF-Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Nick Kew
> Gesendet: Mittwoch, 12. Dezember 2007 12:08
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r603502 -
> /httpd/httpd/trunk/modules/proxy/proxy_util.c
>
>
> On Wed, 12 Dec 2007 07:44:03 -0000
> rpluem@apache.org wrote:
>
> > Author: rpluem
> > Date: Tue Dec 11 23:44:02 2007
> > New Revision: 603502
>
> > - request_rec *rp = apr_pcalloc(c->pool, sizeof(*r));
> > + request_rec *rp = apr_pcalloc(r->pool, sizeof(*r));
>
> (more of the same).
>
> OK, that looks safe to me.
>
> Which begs the question: why was it using the connection pool before?
> I'm reluctant to believe there was no reason, until we've exhausted
> other possibilities.
The connection memory pool was a different memory pool before. It was
the memory pool of the front end connection. Now it is the memory pool
of the backend connection pool connection. See also
http://svn.apache.org/viewvc?view=rev&revision=603237
http://svn.apache.org/viewvc?view=rev&revision=602542
If we would use r->connection->pool instead of r->pool it would be exactly the
same as before the two revisions above, but regarding the pool livetimes I
think r->connection->pool lives too long and thus using r->pool wastes less
memory.
Regards
Rüdiger
Re: svn commit: r603502 -
/httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Nick Kew <ni...@webthing.com>.
On Wed, 12 Dec 2007 07:44:03 -0000
rpluem@apache.org wrote:
> Author: rpluem
> Date: Tue Dec 11 23:44:02 2007
> New Revision: 603502
> - request_rec *rp = apr_pcalloc(c->pool, sizeof(*r));
> + request_rec *rp = apr_pcalloc(r->pool, sizeof(*r));
(more of the same).
OK, that looks safe to me.
Which begs the question: why was it using the connection pool before?
I'm reluctant to believe there was no reason, until we've exhausted
other possibilities.
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/