You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2017/01/19 21:00:19 UTC
Re: svn commit: r1779525 - /httpd/httpd/trunk/modules/http2/h2_mplx.c
On 01/19/2017 09:38 PM, icing@apache.org wrote:
> Author: icing
> Date: Thu Jan 19 20:38:50 2017
> New Revision: 1779525
>
> URL: http://svn.apache.org/viewvc?rev=1779525&view=rev
> Log:
> On the trunk:
>
> mod_http2: decoupling lifetime of mplx pool from h2_session which messed up the cleanup ordering.
>
>
> Modified:
> httpd/httpd/trunk/modules/http2/h2_mplx.c
>
> Modified: httpd/httpd/trunk/modules/http2/h2_mplx.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_mplx.c?rev=1779525&r1=1779524&r2=1779525&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http2/h2_mplx.c (original)
> +++ httpd/httpd/trunk/modules/http2/h2_mplx.c Thu Jan 19 20:38:50 2017
> @@ -280,7 +280,7 @@ h2_mplx *h2_mplx_create(conn_rec *c, apr
> m->id = c->id;
> APR_RING_ELEM_INIT(m, link);
> m->c = c;
> - apr_pool_create_ex(&m->pool, parent, NULL, allocator);
> + apr_pool_create_ex(&m->pool, NULL, NULL, allocator);
Without further investigations: Global pools always make me worry. Are you sure we don't introduce
a memory leak here?
Regards
R�diger
Re: svn commit: r1779525 - /httpd/httpd/trunk/modules/http2/h2_mplx.c
Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 20.01.2017 um 21:59 schrieb Jim Jagielski <ji...@jaguNET.com>:
>
>
>> On Jan 19, 2017, at 4:00 PM, Ruediger Pluem <rp...@apache.org> wrote:
>>
>>
>>
>> On 01/19/2017 09:38 PM, icing@apache.org wrote:
>>> Author: icing
>>> Date: Thu Jan 19 20:38:50 2017
>>> New Revision: 1779525
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1779525&view=rev
>>> Log:
>>> On the trunk:
>>>
>>> mod_http2: decoupling lifetime of mplx pool from h2_session which messed up the cleanup ordering.
>>>
>>>
>>> Modified:
>>> httpd/httpd/trunk/modules/http2/h2_mplx.c
>>>
>>> Modified: httpd/httpd/trunk/modules/http2/h2_mplx.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_mplx.c?rev=1779525&r1=1779524&r2=1779525&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/modules/http2/h2_mplx.c (original)
>>> +++ httpd/httpd/trunk/modules/http2/h2_mplx.c Thu Jan 19 20:38:50 2017
>>> @@ -280,7 +280,7 @@ h2_mplx *h2_mplx_create(conn_rec *c, apr
>>> m->id = c->id;
>>> APR_RING_ELEM_INIT(m, link);
>>> m->c = c;
>>> - apr_pool_create_ex(&m->pool, parent, NULL, allocator);
>>> + apr_pool_create_ex(&m->pool, NULL, NULL, allocator);
>>
>> Without further investigations: Global pools always make me worry. Are you sure we don't introduce
>> a memory leak here?
>>
>
> Especially untagged ones!
Has already been reverted.
Stefan Eissing
<green/>bytes GmbH
Hafenstrasse 16
48155 Münster
www.greenbytes.de
Re: svn commit: r1779525 - /httpd/httpd/trunk/modules/http2/h2_mplx.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
> On Jan 19, 2017, at 4:00 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
>
>
> On 01/19/2017 09:38 PM, icing@apache.org wrote:
>> Author: icing
>> Date: Thu Jan 19 20:38:50 2017
>> New Revision: 1779525
>>
>> URL: http://svn.apache.org/viewvc?rev=1779525&view=rev
>> Log:
>> On the trunk:
>>
>> mod_http2: decoupling lifetime of mplx pool from h2_session which messed up the cleanup ordering.
>>
>>
>> Modified:
>> httpd/httpd/trunk/modules/http2/h2_mplx.c
>>
>> Modified: httpd/httpd/trunk/modules/http2/h2_mplx.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_mplx.c?rev=1779525&r1=1779524&r2=1779525&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/http2/h2_mplx.c (original)
>> +++ httpd/httpd/trunk/modules/http2/h2_mplx.c Thu Jan 19 20:38:50 2017
>> @@ -280,7 +280,7 @@ h2_mplx *h2_mplx_create(conn_rec *c, apr
>> m->id = c->id;
>> APR_RING_ELEM_INIT(m, link);
>> m->c = c;
>> - apr_pool_create_ex(&m->pool, parent, NULL, allocator);
>> + apr_pool_create_ex(&m->pool, NULL, NULL, allocator);
>
> Without further investigations: Global pools always make me worry. Are you sure we don't introduce
> a memory leak here?
>
Especially untagged ones!
Re: svn commit: r1779525 - /httpd/httpd/trunk/modules/http2/h2_mplx.c
Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 19.01.2017 um 22:00 schrieb Ruediger Pluem <rp...@apache.org>:
> On 01/19/2017 09:38 PM, icing@apache.org wrote:
>> Author: icing
>> Date: Thu Jan 19 20:38:50 2017
>> New Revision: 1779525
>>
>> URL: http://svn.apache.org/viewvc?rev=1779525&view=rev
>> Log:
>> On the trunk:
>>
>> mod_http2: decoupling lifetime of mplx pool from h2_session which messed up the cleanup ordering.
>>
>>
>> Modified:
>> httpd/httpd/trunk/modules/http2/h2_mplx.c
>>
>> Modified: httpd/httpd/trunk/modules/http2/h2_mplx.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_mplx.c?rev=1779525&r1=1779524&r2=1779525&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/http2/h2_mplx.c (original)
>> +++ httpd/httpd/trunk/modules/http2/h2_mplx.c Thu Jan 19 20:38:50 2017
>> @@ -280,7 +280,7 @@ h2_mplx *h2_mplx_create(conn_rec *c, apr
>> m->id = c->id;
>> APR_RING_ELEM_INIT(m, link);
>> m->c = c;
>> - apr_pool_create_ex(&m->pool, parent, NULL, allocator);
>> + apr_pool_create_ex(&m->pool, NULL, NULL, allocator);
>
> Without further investigations: Global pools always make me worry. Are you sure we don't introduce
> a memory leak here?
It's good that someone's looking at this. This pool is taken down inside a pre_cleanup registry of the parent. So, there is no mem leak, but it did not change anything either. Just double-checked that pre_cleanups run before child pools are destroyed. So, I will revert this. But again, thanks for checking.
> Regards
>
> Rüdiger
Stefan Eissing
<green/>bytes GmbH
Hafenstrasse 16
48155 Münster
www.greenbytes.de