Mailing List Archive

Re: svn commit: r1891148 - in /httpd/httpd/trunk: include/ap_mmn.h include/util_filter.h modules/proxy/proxy_util.c server/util_filter.c
On Thu, Jul 1, 2021 at 12:38 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>
> On Wed, Jun 30, 2021 at 9:42 AM Joe Orton <jorton@redhat.com> wrote:
> >
> > If so I wonder if it wouldn't be better to overload FLUSH for this, e.g.
> > by having a FLUSH bucket with a non-NULL ->data pointer or something?
> > The core knows it is special but everywhere else treats as FLUSH.
>
> That's a great idea, let me try that.

Here is a new patch, the semantics of WC buckets are defined as:
/**
* @brief Write Completion (WC) bucket
*
* A WC bucket is a FLUSH bucket with special ->data == &ap_bucket_wc_data,
* still both AP_BUCKET_IS_WC() and APR_BUCKET_IS_FLUSH() hold for them so
* they have the same semantics for most filters, namely:
* Everything produced before shall be passed to the next filter, including
* the WC/FLUSH bucket itself.
* The distinction between WC and FLUSH buckets is only for filters that care
* about write completion (calling ap_filter_reinstate_brigade() with non-NULL
* flush_upto), those can setaside WC buckets and the preceding data provided
* they have first determined that the next filter(s) have pending data
* already, usually by calling ap_filter_should_yield(f->next).
*/

The only filters that care about write completion for now are
ap_core_output_filter() and ssl_io_filter_output(), which try to fill
in the pipe as much as possible, using
ap_filter_reinstate_brigade(&flush_upto) to determine whether they
should flush (blocking) or setaside their remaining data.

So ap_filter_reinstate_brigade() is made to not treat WC as FLUSH
buckets and keep the above filters working as before (and correctly
w.r.t WC bucket semantics). I first thought adding a new
ap_filter_rec->proto_flags like AP_FILTER_PROTO_WC_READY to do this
only for filter registered with this flag, and then register
ap_core_output_filter() and ssl_io_filter_output() accordingly, but
since they are the only users of ap_filter_reinstate_brigade() with
flush_upto != NULL I kept it simple for now..

WDYT?

Cheers;
Yann.
Re: svn commit: r1891148 - in /httpd/httpd/trunk: include/ap_mmn.h include/util_filter.h modules/proxy/proxy_util.c server/util_filter.c [ In reply to ]
On Thu, Aug 19, 2021 at 03:28:44PM +0200, Yann Ylavic wrote:
> The only filters that care about write completion for now are
> ap_core_output_filter() and ssl_io_filter_output(), which try to fill
> in the pipe as much as possible, using
> ap_filter_reinstate_brigade(&flush_upto) to determine whether they
> should flush (blocking) or setaside their remaining data.
>
> So ap_filter_reinstate_brigade() is made to not treat WC as FLUSH
> buckets and keep the above filters working as before (and correctly
> w.r.t WC bucket semantics). I first thought adding a new
> ap_filter_rec->proto_flags like AP_FILTER_PROTO_WC_READY to do this
> only for filter registered with this flag, and then register
> ap_core_output_filter() and ssl_io_filter_output() accordingly, but
> since they are the only users of ap_filter_reinstate_brigade() with
> flush_upto != NULL I kept it simple for now..
>
> WDYT?

+1

Very nice, I like it, thanks Yann.

Regards, Joe