[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: immense Apache memory leak, plus patch

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: Wed, 09 Sep 2009 09:54:39 -0400

C. Michael Pilato wrote:
> Mark Phippard wrote:
>> On Thu, Sep 3, 2009 at 10:14 AM, Mark Phippard<markphip_at_gmail.com> wrote:
>>> On Thu, Sep 3, 2009 at 9:45 AM, C. Michael Pilato<cmpilato_at_collab.net> wrote:
>>>> Blaise Tarr wrote:
>>>>> Greg Stein wrote:
>>>>>> I don't see how this would work, however, because there are *still* a
>>>>>> lot of uses of the 'output' variable. Just changing the one in
>>>>>> ap_pass_brigade() is going to be insufficient. Seems that all of them
>>>>>> should really be changed.
>>>>> That patch is just the bare minimum to prevent the crashes we've
>>>>> experienced, and it confirms the source of the problem, but it's
>>>>> definitely not the proper fix. Sorry, I should have stated that earlier.
>>>>> It works because the pointers in the 'output' ap_filter_t struct still
>>>>> happen to be valid, or so it seems.
>>>>>
>>>>> Changing all the references to 'output' would more correct, but that
>>>>> would essentially render the 'output' function parameter useless, and
>>>>> that doesn't feel right.
>>>>>
>>>>> One possibility is to create a dummy (sentinel) filter at the beginning
>>>>> of the filter chain that would simply pass the brigade along. It would
>>>>> always be present, and references to the filter chain using it would
>>>>> always be valid. Implementing it would be more appropriate in Apache
>>>>> itself, but there should be no harm in having mod_dav_svn do it.
>>>> Please excuse my ignorance, but what would such a filter look like?
>>>> Anything like the following?
>>>>
>>>> apr_status_t
>>>> dav_svn__dummy_filter(ap_filter_t *f,
>>>> apr_bucket_brigade *bb)
>>>> {
>>>> return ap_pass_brigade(f->r->output_filters, bb);
>>>> }
>>> Mike,
>>>
>>> Did you see this?
>>>
>>> http://httpd.apache.org/docs/trunk/developer/output-filters.html
>>>
>>> How would we guarantee this would be put at the beginning of the
>>> filter chain? Is that possible?
>>>
>>> Is it possible to detect that mod_deflate has removed itself from the
>>> chain and not call it?
>> Anyone have any comments on these questions?
>>
>> Has anyone looked into whether mod_deflate could be patched? Maybe it
>> could just pass along the brigade rather than removing itself from the
>> filter chain?
>
> gstein said previously: "From a design standpoint, it would seem that a
> filter should never attempt to disengage itself from the filter chain.
> Instead, it should just pass the brigade along."
>
> That certainly seems like the ideal scenario, but wouldn't that mean
> doublechecking *all* the available filters to ensure that they don't remove
> themselves from the chain? I mean, we saw this with mod_deflate for a given
> scenario -- could it crop up elsewhere, too for ultimately the same reason?
>

Here's a bright guy who cheated the system yeeeeeeears ago by changing
Apache's "remove the filter" code to instead "replace the filter with a
dummy filter":
http://mail-archives.apache.org/mod_mbox/httpd-dev/200501.mbox/<41E30B42.4060202@stason.org>

Sadly, I see no response to his post either validating or condemning it.

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2392887

Received on 2009-09-09 15:55:08 CEST

This is an archived mail posted to the Subversion Dev mailing list.