You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Stefan Fritsch <sf...@sfritsch.de> on 2010/09/26 18:13:26 UTC
memory leak in mod_slotmem_shm
Hi,
mod_slotmem_shm creates a new global pool on every graceful restart
but never destroys it.
From quickly looking at the code it is not clear to me what the
correct behaviour would be. Should the pool be created from pconf
instead? Or should it be destroyed in cleanup_slotmem()?
Cheers,
Stefan
Re: memory leak in mod_slotmem_shm
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Sep 27, 2010, at 1:19 PM, Stefan Fritsch wrote:
> On Monday 27 September 2010, Jim Jagielski wrote:
>>> BTW, it is not that obvious that the shm is supposed to be
>>> cleaned up and re-created on graceful restarts. This should be
>>> documented in the code.
>>>
>>>
>>
>> That's an interesting point. I always assumed that it should be,
>> since one use for it would be as a scoreboard-like replacement
>> (and the pain for the scoreboard is that it's a set size)...
>> But I can also see reasons for it to NOT be cleaned/recreated...
>> How best to handle that??
>
> If it would be used for e.g. storing mod_proxy_balancer's worker
> configuration/state, one would want the data to survive a graceful
> restart, wouldn't one? But from a consumer's perspective, it doesn't
> matter if this achieved by not destroying the shm segment or by saving
> the data to disk and reloading it after recreating the shm segment.
> So it seems to me that the current functionality is enough.
>
But it *does* persist the data... it just clears up
the sh mem.
Re: memory leak in mod_slotmem_shm
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 28 September 2010, Jim Jagielski wrote:
> Anyone have issues if I see if there is some way to see
> if the clean-and-clear-at-graceful can be runtime variable.
> If the size and number of slots don't change, it would be
> useful to persist the data between graceful restarts at
> the shmem layer...
I think Rainer's concern is valid. In case of a graceful restart, the
child processes live longer than the config pool. There is a similar
problem with mod_ldap's shm cache:
https://issues.apache.org/bugzilla/show_bug.cgi?id=48958#c7
If you can make it work that the shm segment ist not destroyed during
graceful restarts, that would be a win.
Re: memory leak in mod_slotmem_shm
Posted by Jim Jagielski <ji...@jaguNET.com>.
Anyone have issues if I see if there is some way to see
if the clean-and-clear-at-graceful can be runtime variable.
If the size and number of slots don't change, it would be
useful to persist the data between graceful restarts at
the shmem layer...
Re: memory leak in mod_slotmem_shm
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Mon, Sep 27, 2010 at 08:03:51PM +0200, Rainer Jung wrote:
> On 27.09.2010 19:19, Stefan Fritsch wrote:
> >On Monday 27 September 2010, Jim Jagielski wrote:
> >>>BTW, it is not that obvious that the shm is supposed to be
> >>>cleaned up and re-created on graceful restarts. This should be
> >>>documented in the code.
> >>>
> >>>
> >>
> >>That's an interesting point. I always assumed that it should be,
> >>since one use for it would be as a scoreboard-like replacement
> >>(and the pain for the scoreboard is that it's a set size)...
> >>But I can also see reasons for it to NOT be cleaned/recreated...
> >>How best to handle that??
> >
> >If it would be used for e.g. storing mod_proxy_balancer's worker
> >configuration/state, one would want the data to survive a graceful
> >restart, wouldn't one? But from a consumer's perspective, it doesn't
> >matter if this achieved by not destroying the shm segment or by saving
> >the data to disk and reloading it after recreating the shm segment.
> >So it seems to me that the current functionality is enough.
>
> So if a graceful restart is initiated, will old children working on
> requests still be able to use the old shm segment, i.e. will it be
> destroyed after the last old child dies or earlier?
>
The cleanup is when the config pool is cleaned up, at which point
there are no more consumers of that pool so we're safe.
--
===========================================================================
Jim Jagielski [|] jim@jaguNET.com [|] http://www.jaguNET.com/
"Great is the guilt of an unnecessary war" ~ John Adams
Re: memory leak in mod_slotmem_shm
Posted by Rainer Jung <ra...@kippdata.de>.
On 27.09.2010 19:19, Stefan Fritsch wrote:
> On Monday 27 September 2010, Jim Jagielski wrote:
>>> BTW, it is not that obvious that the shm is supposed to be
>>> cleaned up and re-created on graceful restarts. This should be
>>> documented in the code.
>>>
>>>
>>
>> That's an interesting point. I always assumed that it should be,
>> since one use for it would be as a scoreboard-like replacement
>> (and the pain for the scoreboard is that it's a set size)...
>> But I can also see reasons for it to NOT be cleaned/recreated...
>> How best to handle that??
>
> If it would be used for e.g. storing mod_proxy_balancer's worker
> configuration/state, one would want the data to survive a graceful
> restart, wouldn't one? But from a consumer's perspective, it doesn't
> matter if this achieved by not destroying the shm segment or by saving
> the data to disk and reloading it after recreating the shm segment.
> So it seems to me that the current functionality is enough.
So if a graceful restart is initiated, will old children working on
requests still be able to use the old shm segment, i.e. will it be
destroyed after the last old child dies or earlier?
Regards,
Rainer
Re: memory leak in mod_slotmem_shm
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Monday 27 September 2010, Jim Jagielski wrote:
> > BTW, it is not that obvious that the shm is supposed to be
> > cleaned up and re-created on graceful restarts. This should be
> > documented in the code.
> >
> >
>
> That's an interesting point. I always assumed that it should be,
> since one use for it would be as a scoreboard-like replacement
> (and the pain for the scoreboard is that it's a set size)...
> But I can also see reasons for it to NOT be cleaned/recreated...
> How best to handle that??
If it would be used for e.g. storing mod_proxy_balancer's worker
configuration/state, one would want the data to survive a graceful
restart, wouldn't one? But from a consumer's perspective, it doesn't
matter if this achieved by not destroying the shm segment or by saving
the data to disk and reloading it after recreating the shm segment.
So it seems to me that the current functionality is enough.
Re: memory leak in mod_slotmem_shm
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Sep 27, 2010, at 10:28 AM, Stefan Fritsch wrote:
> On Monday 27 September 2010, Jim Jagielski wrote:
>> On Sep 27, 2010, at 9:19 AM, Jim Jagielski wrote:
>>> On Sep 27, 2010, at 8:17 AM, Jim Jagielski wrote:
>>>> Let me look...
>>>>
>>>> On Sep 26, 2010, at 12:13 PM, Stefan Fritsch wrote:
>>>>> Hi,
>>>>>
>>>>> mod_slotmem_shm creates a new global pool on every graceful
>>>>> restart but never destroys it.
>>>>>
>>>>> From quickly looking at the code it is not clear to me what the
>>>>> correct behaviour would be. Should the pool be created from
>>>>> pconf instead? Or should it be destroyed in cleanup_slotmem()?
>>>
>>> creating from pconf is cleaner...
>>
>> Hold on a tic... I'm not seeing the leak. During
>> cleanup_slotmem we do destroy next->gpool which
>> is set to gpool.
>
> Hmm. There seem to be two issues here:
>
> If no other module uses shm slotmem (i.e. slotmem_create() is never
> called), globallistmem stays NULL and the cleanup does nothing. In
> this case gpool is leaked every time.
Yeppers. That's true.
>
> The pool is created on every call to pre_config, but the cleanup is
> registered only on the second and subsequent calls to post_config. In
> this case only the first gpool is leaked.
>
Hmmmm...
> BTW, it is not that obvious that the shm is supposed to be cleaned up
> and re-created on graceful restarts. This should be documented in the
> code.
>
That's an interesting point. I always assumed that it *should* be,
since one use for it would be as a scoreboard-like replacement
(and the pain for the scoreboard is that it's a set size)...
But I can also see reasons for it to NOT be cleaned/recreated...
How best to handle that??
Re: memory leak in mod_slotmem_shm
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Monday 27 September 2010, Jim Jagielski wrote:
> On Sep 27, 2010, at 9:19 AM, Jim Jagielski wrote:
> > On Sep 27, 2010, at 8:17 AM, Jim Jagielski wrote:
> >> Let me look...
> >>
> >> On Sep 26, 2010, at 12:13 PM, Stefan Fritsch wrote:
> >>> Hi,
> >>>
> >>> mod_slotmem_shm creates a new global pool on every graceful
> >>> restart but never destroys it.
> >>>
> >>> From quickly looking at the code it is not clear to me what the
> >>> correct behaviour would be. Should the pool be created from
> >>> pconf instead? Or should it be destroyed in cleanup_slotmem()?
> >
> > creating from pconf is cleaner...
>
> Hold on a tic... I'm not seeing the leak. During
> cleanup_slotmem we do destroy next->gpool which
> is set to gpool.
Hmm. There seem to be two issues here:
If no other module uses shm slotmem (i.e. slotmem_create() is never
called), globallistmem stays NULL and the cleanup does nothing. In
this case gpool is leaked every time.
The pool is created on every call to pre_config, but the cleanup is
registered only on the second and subsequent calls to post_config. In
this case only the first gpool is leaked.
BTW, it is not that obvious that the shm is supposed to be cleaned up
and re-created on graceful restarts. This should be documented in the
code.
Re: memory leak in mod_slotmem_shm
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Sep 27, 2010, at 9:19 AM, Jim Jagielski wrote:
>
> On Sep 27, 2010, at 8:17 AM, Jim Jagielski wrote:
>
>> Let me look...
>> On Sep 26, 2010, at 12:13 PM, Stefan Fritsch wrote:
>>
>>> Hi,
>>>
>>> mod_slotmem_shm creates a new global pool on every graceful restart
>>> but never destroys it.
>>>
>>> From quickly looking at the code it is not clear to me what the
>>> correct behaviour would be. Should the pool be created from pconf
>>> instead? Or should it be destroyed in cleanup_slotmem()?
>>>
>
> creating from pconf is cleaner...
>
Hold on a tic... I'm not seeing the leak. During
cleanup_slotmem we do destroy next->gpool which
is set to gpool.
Re: memory leak in mod_slotmem_shm
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Sep 27, 2010, at 8:17 AM, Jim Jagielski wrote:
> Let me look...
> On Sep 26, 2010, at 12:13 PM, Stefan Fritsch wrote:
>
>> Hi,
>>
>> mod_slotmem_shm creates a new global pool on every graceful restart
>> but never destroys it.
>>
>> From quickly looking at the code it is not clear to me what the
>> correct behaviour would be. Should the pool be created from pconf
>> instead? Or should it be destroyed in cleanup_slotmem()?
>>
creating from pconf is cleaner...
Re: memory leak in mod_slotmem_shm
Posted by Jim Jagielski <ji...@jaguNET.com>.
Let me look...
On Sep 26, 2010, at 12:13 PM, Stefan Fritsch wrote:
> Hi,
>
> mod_slotmem_shm creates a new global pool on every graceful restart
> but never destroys it.
>
> From quickly looking at the code it is not clear to me what the
> correct behaviour would be. Should the pool be created from pconf
> instead? Or should it be destroyed in cleanup_slotmem()?
>
> Cheers,
> Stefan
>