On 07/20/2018 03:49 PM, Yann Ylavic wrote:
> On Fri, Jul 20, 2018 at 2:54 PM, Ruediger Pluem <rpl...@apache.org> wrote:
>>
>> On 07/20/2018 02:38 PM, Yann Ylavic wrote:
>>> On Fri, Jul 20, 2018 at 12:08 PM, Ruediger Pluem <rpl...@apache.org> wrote:
>>>>
>>>> On 07/19/2018 12:36 AM, yla...@apache.org 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?
You describe the inconsistency in case c->data_in_input_filters is 1,
I did for the case that c->data_in_input_filters is 0, but yes we could have
inconsistencies in both cases
and the question is if they are always fine. So far it looks like.
But probably the flags should just go in order to avoid these cases.
Regards
Rüdiger
