[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: svn commit: r1836239 - in /httpd/httpd/trunk: include/httpd.h modules/http/http_request.c server/mpm/event/event.c server/mpm/motorz/motorz.c server/mpm/simple/simple_io.c server/util_filter.c

On Fri, Jul 20, 2018 at 2:54 PM, Ruediger Pluem <rpluem@xxxxxxxxxx> wrote:
> On 07/20/2018 02:38 PM, Yann Ylavic wrote:
>> On Fri, Jul 20, 2018 at 12:08 PM, Ruediger Pluem <rpluem@xxxxxxxxxx> wrote:
>>> On 07/19/2018 12:36 AM, ylavic@xxxxxxxxxx wrote:
>>>> -    if (!c->data_in_input_filters || ap_run_input_pending(c) != OK) {
>>>> +    if (ap_run_input_pending(c) != OK) {
>>> We have a different code flow here now. If c->data_in_input_filters is 0, then
>>> ap_filter_input_pending does iterate over the ring whereas it did not do before,
>>> because it was not called.
>> Right, though ap_filter_input_pending() iteration is quite cheap, and
>> could be even cheaper if pending input and output filters were handled
>> separetely (next step...).
>> I prefer to keep the logic in ap_filter_*_pending() if you don't mind,
>> and avoid global c->data_in_*_filter checks all over the place
>> (possibly they'll disappear from conn_rec some day).
>> For now c->data_in_*_filter are used internally (core) to positively
>> force pending data, never negatively (and that's wise I think).
> So if c->data_in_input_filters is 0 the iteration is not expected to return OK,
> as otherwise we would have an inconsistency between c->data_in_input_filters and the ring, correct?

Well, it's hard/unwise to keep a per-connection flag aligned with a
per-filter feature, that's why those flags should disappear I think.
But yes, when ap_run_input_pending() is called from
ap_process_request() it will never return OK if
c->data_in_input_filters is 0.

The issue, I think, is rather that if c->data_in_*_filters is 1 it
will not change until the next call to ap_process_[async_]request(),
even though the filter chain may be emptied until then, so
ap_filter_*_pending() may return OK while it shouldn't.

Hmm, let me look at this more closely, it seems that these flags
should really disappear now.

> On the other hand this inconsistency is possible if ap_run_input_pending is called from
> other locations in the code than the above where we did not have this !c->data_in_input_filters check, correct?

This says the same thing as I said above right?