Re: worker MPM test failures (was Re: Still Failing: apache/httpd#190 (trunk - 894b6a1))

[Yann Ylavic <yl...@gmail.com>]

On Thu, Jan 23, 2020 at 8:23 AM Pluem, Ruediger, Vodafone Group
<ru...@vodafone.com> wrote:
> >
> > https://github.com/apache/httpd/pull/88
>
> Anyone had a look?

If I understand the patch correctly, ap_request_core_filter() would
let everything through on EOR, regardless of potential morphing
buckets set aside?
So if ap_request_core_filter() yields because of network contention,
setting aside all the remaining buckets (say including a morphing
one), and then ap_process_request_after_handler() sends the EOR (the
handler has finished sending everything on its side), it seems that
ap_request_core_filter() won't play its role anymore?
This plus the fact that ap_request_core_filter() filters "stack up"
for each request, so the oldest request's filter may setaside data for
newer ones, with its own (wrong) request pool.

How about we use a single ap_request_core_filter() for all the
requests, at AP_FTYPE_CONNECTION level, and have it "detect" requests
based on SOR/EOR ranges, where the SOR (Start Of Request) bucket is to
be inserted by the AP_FTYPE_PROTOCOL module (e.g.
ap_http_header_filter for http) before sending any data pertaining to
its request?

It tried that in the attached patch, it passes the test framework and
Joe's reproducer.
Thoughts?

Regards,
Yann.

Re: worker MPM test failures (was Re: Still Failing: apache/httpd#190 (trunk - 894b6a1))

[Ruediger Pluem <rp...@apache.org>]

On 3/22/20 6:43 PM, Yann Ylavic wrote:
> +        APR_BRIGADE_INSERT_TAIL(ctx->tmp_bb, bucket);
> +        if (do_pass
> +                || APR_BRIGADE_EMPTY(ctx->bb)
> +                || (bucket->length == (apr_size_t)-1)

How can we have a bucket length of -1 here?
What about flush buckets? Shouldn't they cause a passing here as well?

> +                || (tmp_bb_len += bucket->length) >=
> +                        conf->flush_max_threshold) {
> +            rv = ap_pass_brigade(f->next, ctx->tmp_bb);
> +            apr_brigade_cleanup(ctx->tmp_bb);
> +            if (status == APR_SUCCESS) {
> +                status = rv;
>              }

Regards

Rüdiger