httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rici Lake <r...@ricilake.net>
Subject Re: RFC: Who owns a passed brigade?
Date Mon, 25 Apr 2005 16:07:07 GMT

On 25-Apr-05, at 8:29 AM, Joe Orton wrote:

> I think it's fine to leave the *contents* of the brigade as undefined
> after a call to ap_pass_brigade.  (that's essentially the case now 
> since
> it's not explicitly defined to be "empty": I don't know of any
> particular bugs which have been caused by this?)

Undefined contents make me nervous. It is very easy to define the
results: just do a cleanup in ap_pass_brigade. (This will also
protect against some resource leaks in failed error handling, for
example.)

I don't know of any particular bugs which have been caused by this
either, but it would be very easy to create one. Take a filter which
expects the brigade to be emptied and feed it into a filter which
does not empty the brigade. Then watch the output quadratically
expand.

I don't believe there are any such bugs in the distribution,
because the only output filter in the distribution that I can
find which does not empty its brigade is mod_case_filter.c
(see below). Furthermore, most of the interesting filters
in the distribution actually create a new brigade for every
invocation, which I think we can all agree is a bad idea.

Some filters, such as mod_deflate.c, do expect the brigade
to be cleaned. In this case, it is unlikely that a bug will
be noticed, because few third-party filters will get inserted
after mod_deflate.c.

If filters were modified to always use the same bucket brigade
for communication with downstream filters, you might start to
see this error with third-party filters which don't clean
their input brigade, unless every filter religiously calls
ap_brigade_cleanup after ap_pass_brigade. But if you were
going to do that, you might as well put the call to
ap_brigade_cleanup in ap_pass_brigade. The cost of an
unnecessary call is minimal, and it simplifies the api.

Note: I honestly believe mod_case_filter.c should be removed
from the distribution, unless someone is willing to rewrite
it as a proper model of how to write an output filter. It is not
useful as a module, and it's only use in the distribution is to
serve as an example for module authors, which it definitely is not.


Mime
View raw message