That's what I worried...
I applied Justin's patch, so far so good. So this is the fix we'll use.
BTW, by APR_BUCKET_INSERT_TAIL, I think you mean APR_BRIGADE_INSERT_TAIL
Thanks,
-- Jin
Brian Pane wrote:
>
> Justin Erenkrantz wrote:
>
> >On Fri, Dec 14, 2001 at 09:32:11AM -0800, Ian Holsman wrote:
> >
> >>hi guys.
> >>we've been investigating a couple of core dumps in some testing and we
> >>think we found one in mod-include.
> >>
> >>here is the email thread.
> >>does anyone have any reason NOT to reset the state when reseting
> >>the bytes parsed ?
> >>
> >
> >Just to make sure I understand what you guys are thinking of,
> >this is your proposed patch, right? (more follows)
> >
> >Index: modules/filters/mod_include.c
> >===================================================================
> >RCS file: /home/cvs/httpd-2.0/modules/filters/mod_include.c,v
> >retrieving revision 1.166
> >diff -u -r1.166 mod_include.c
> >--- modules/filters/mod_include.c 2001/12/08 03:14:50 1.166
> >+++ modules/filters/mod_include.c 2001/12/14 18:12:31
> >@@ -3066,6 +3066,7 @@
> > }
> > else {
> > ctx->bytes_parsed = 0;
> >+ ctx->state = PRE_HEAD;
> > }
> >
> > if ((parent = ap_get_module_config(r->request_config, &include_module))) {
> >
> >If so, then I'm kind of leery about this.
> >
> >The reason is that filters can be called many times over the
> >lifetime of the data. So, we could leave the parse engine in a
> >ctx->state = POST_HEAD between calls because we are still waiting
> >for a bucket-brigade containing the rest of the tag. We'd obliterate
> >that value with this. =) So, I don't think resetting the state is
> >going to do you any good here.
> >
> >Based on your description, what seems to be happening is that the
> >client either disconnects or timeouts and the finalize_request is
> >passing down an EOS but mod_include is stuck in some internal
> >state. mod_include just isn't handling it well.
> >
> >So, I'm thinking the appropriate check is to stop at EOS or
> >SENTINEL. There is no good reason to continue processing if we
> >see an EOS. But, we have a potentially saved brigade to handle.
> >So, what do we do? Perhaps this patch might be better:
> >
>
> I haven't had time to read through through Justin's patch in detail,
> but conceptually I think it's the right approach: check for the EOS
> condition as a special case in the end-of-brigade code that's crashing,
> rather than resetting the state every time we enter the filter.
>
> --Brian
