On Sun, Apr 18, 2021 at 7:37 PM Christophe JAILLET
<christophe.jail...@wanadoo.fr> wrote:
>
> Le 03/02/2021 à 09:24, Ruediger Pluem a écrit :
> >
> > On 2/2/21 4:18 PM, Yann Ylavic wrote:
> >> On Tue, Feb 2, 2021 at 10:32 AM Ruediger Pluem <rpl...@apache.org> wrote:
> >>>
> >>>> New Revision: 1885605
> >>>>
> >>>> URL: http://svn.apache.org/viewvc?rev=1885605&view=rev
> >> []
> >>>> + /* Yield if the output filters stack is full? This is to avoid
> >>>> + * blocking and give the caller a chance to POLLOUT async.
> >>>> + */
> >>>> + if (flags & AP_PROXY_TRANSFER_YIELD_PENDING) {
> >>>> + int rc = OK;
> >>>> +
> >>>> + if (!ap_filter_should_yield(c_o->output_filters)) {
> >>>> + rc = ap_filter_output_pending(c_o);
> >>>
> >>> I am confused here: !ap_filter_should_yield(c_o->output_filters) means
> >>> there is no pending output.
> >>> Why should we try to send it then? Shouldn't it be the other way round?
> >>
> >> Yes, !ap_filter_should_yield() means that there is no pending output,
> >> but only for filters that play the
> >> ap_filter_{setaside,reinstate}_brigade() game.
> >>
> >> The goal here is to try to flush pending data of filters that don't
> >> ap_filter_setaside_brigade(), like e.g. ssl_io_filter_coalesce() which
> >
> > But isn't ap_filter_output_pending a noop that always returns DECLINED in
> > case
> > of !ap_filter_should_yield(c_o->output_filters)?
>
> Same backport for which a just sent a question about a potential missing
> part of r1877558.
Like Eric said, this is not relevant for 2.4.
>
> Unrelated, but is there still something to discuss here.
Probably :)
> The last comment from Rüdiger seems to have been left un-answered.
Ah yes, sorry Rüdiger, somehow I missed that one.
First for 2.4.x it's true, if !ap_filter_should_yield() then
ap_filter_output_pending() will always be DECLINED.
But ap_filter_should_yield() and ap_filter_output_pending() in 2.4.x
are just some helpers in proxy_util to keep trunk and 2.4.x code
aligned, not the real util_filter functions.
For trunk though, there is the ssl_io_filter_coalesce() case where
!ap_filter_should_yield() does not mean that
ap_filter_output_pending() has nothing to do. That's because
ssl_io_filter_coalesce() does not play the
ap_filter_{setaside,reinstate}_brigade() game for now, even though it
potentially retains data.
So in r1879416 I made a band aid such that ssl_io_filter_coalesce()
releases its data when it's called from ap_filter_output_pending(),
but the real fix would be that mod_ssl coalesces the data using
apr_brigade_write() and save/restore them with
ap_filter_{setaside,reinstate}_brigade(). But I didn't finish that
patch yet..
Note that in 2.4.x ssl_io_filter_coalesce() in not an issue for the
mod_proxy tunneling loop because ap_proxy_tunnel_create() initially
does:
/* No coalescing filters */
ap_remove_output_filter_byhandle(c_i->output_filters,
"SSL/TLS Coalescing Filter");
ap_remove_output_filter_byhandle(c_o->output_filters,
"SSL/TLS Coalescing Filter");
But in trunk we don't do that anymore, because ultimately
ap_proxy_tunnel_run() could be used for full async mod_proxy(_http),
not only tunneling, meaning that all the filters (and not only
connection ones) need to remain in place. I'm working on that (slowly)
too..
Hopefully I'm a bit more clear about this time..
Regards;
Yann.
