Re: immense Apache memory leak, plus patch

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?

-- 
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=2392552