You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Yann Ylavic <yl...@gmail.com> on 2021/09/03 10:27:42 UTC
Re: svn commit: r1892740 - in /httpd/httpd/trunk: changes-entries/ap_proxy_tunnel_run.txt
modules/proxy/proxy_util.c
On Tue, Aug 31, 2021 at 9:29 AM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 8/30/21 8:04 PM, ylavic@apache.org wrote:
> > +
> > + /* Flush any pending input data now, we don't know when
> > + * the next POLLIN will trigger and retaining data might
> > + * deadlock the underlying protocol. We don't check for
> > + * pending data first with ap_filter_input_pending() since
> > + * the read from proxy_tunnel_forward() is nonblocking
> > + * anyway and returning OK if there's no data.
> > + */
> > + rc = proxy_tunnel_forward(tunnel, in);
> > + if (rc != OK) {
> > + return rc;
> > + }
>
> Don't we do all of this already a few lines above if ap_filter_input_pending(in->c) == OK?
> Why doing it again?
With whatever tc (client or origin connection), here on tc->rtnevents
& POLLOUT we have out == tc and in == tc->other, so we forward
potential pending input data from tc->other to tc (without waiting
tc->other to become readable).
Below with the same tc (same loop), if tc->rtnevents & POLLIN we
forward from tc to tc->other, thus the opposite direction.
Regards;
Yann.
Re: svn commit: r1892740 - in /httpd/httpd/trunk: changes-entries/ap_proxy_tunnel_run.txt
modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Sep 3, 2021 at 12:58 PM Ruediger Pluem <rp...@apache.org> wrote:
>
>
>
> On 9/3/21 12:27 PM, Yann Ylavic wrote:
> > On Tue, Aug 31, 2021 at 9:29 AM Ruediger Pluem <rp...@apache.org> wrote:
> >>
> >> On 8/30/21 8:04 PM, ylavic@apache.org wrote:
> >>> +
> >>> + /* Flush any pending input data now, we don't know when
> >>> + * the next POLLIN will trigger and retaining data might
> >>> + * deadlock the underlying protocol. We don't check for
> >>> + * pending data first with ap_filter_input_pending() since
> >>> + * the read from proxy_tunnel_forward() is nonblocking
> >>> + * anyway and returning OK if there's no data.
> >>> + */
> >>> + rc = proxy_tunnel_forward(tunnel, in);
> >>> + if (rc != OK) {
> >>> + return rc;
> >>> + }
> >>
> >> Don't we do all of this already a few lines above if ap_filter_input_pending(in->c) == OK?
> >> Why doing it again?
> >
> > With whatever tc (client or origin connection), here on tc->rtnevents
> > & POLLOUT we have out == tc and in == tc->other, so we forward
> > potential pending input data from tc->other to tc (without waiting
> > tc->other to become readable).
> >
> > Below with the same tc (same loop), if tc->rtnevents & POLLIN we
> > forward from tc to tc->other, thus the opposite direction.
>
> I still don't get it. Starting at line 4941 in proxy_util.c we have
>
>
>
> /* Flush any pending input data now, we don't know when
> * the next POLLIN will trigger and retaining data might
> * block the protocol.
> */
> if (ap_filter_input_pending(in->c) == OK) {
> rc = proxy_tunnel_forward(tunnel, in);
> if (rc != OK) {
> return rc;
> }
> }
>
> /* Flush any pending input data now, we don't know when
> * the next POLLIN will trigger and retaining data might
> * deadlock the underlying protocol. We don't check for
> * pending data first with ap_filter_input_pending() since
> * the read from proxy_tunnel_forward() is nonblocking
> * anyway and returning OK if there's no data.
> */
> rc = proxy_tunnel_forward(tunnel, in);
> if (rc != OK) {
> return rc;
> }
>
>
> We execute the same code
>
>
> rc = proxy_tunnel_forward(tunnel, in);
> if (rc != OK) {
> return rc;
> }
>
> twice if ap_filter_input_pending(in->c) == OK. Even the comments above these two executions
> start with the same wording. I still ask me why we need to do it twice.
Argh sorry, I thought you were talking about the
proxy_tunnel_forward() in the POLLIN case, because I didn't see that I
had left the old code in (not visible in the diff..).
I think I got it in r1892851 :)
Thanks!
Re: svn commit: r1892740 - in /httpd/httpd/trunk:
changes-entries/ap_proxy_tunnel_run.txt modules/proxy/proxy_util.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 9/3/21 12:27 PM, Yann Ylavic wrote:
> On Tue, Aug 31, 2021 at 9:29 AM Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 8/30/21 8:04 PM, ylavic@apache.org wrote:
>>> +
>>> + /* Flush any pending input data now, we don't know when
>>> + * the next POLLIN will trigger and retaining data might
>>> + * deadlock the underlying protocol. We don't check for
>>> + * pending data first with ap_filter_input_pending() since
>>> + * the read from proxy_tunnel_forward() is nonblocking
>>> + * anyway and returning OK if there's no data.
>>> + */
>>> + rc = proxy_tunnel_forward(tunnel, in);
>>> + if (rc != OK) {
>>> + return rc;
>>> + }
>>
>> Don't we do all of this already a few lines above if ap_filter_input_pending(in->c) == OK?
>> Why doing it again?
>
> With whatever tc (client or origin connection), here on tc->rtnevents
> & POLLOUT we have out == tc and in == tc->other, so we forward
> potential pending input data from tc->other to tc (without waiting
> tc->other to become readable).
>
> Below with the same tc (same loop), if tc->rtnevents & POLLIN we
> forward from tc to tc->other, thus the opposite direction.
I still don't get it. Starting at line 4941 in proxy_util.c we have
/* Flush any pending input data now, we don't know when
* the next POLLIN will trigger and retaining data might
* block the protocol.
*/
if (ap_filter_input_pending(in->c) == OK) {
rc = proxy_tunnel_forward(tunnel, in);
if (rc != OK) {
return rc;
}
}
/* Flush any pending input data now, we don't know when
* the next POLLIN will trigger and retaining data might
* deadlock the underlying protocol. We don't check for
* pending data first with ap_filter_input_pending() since
* the read from proxy_tunnel_forward() is nonblocking
* anyway and returning OK if there's no data.
*/
rc = proxy_tunnel_forward(tunnel, in);
if (rc != OK) {
return rc;
}
We execute the same code
rc = proxy_tunnel_forward(tunnel, in);
if (rc != OK) {
return rc;
}
twice if ap_filter_input_pending(in->c) == OK. Even the comments above these two executions
start with the same wording. I still ask me why we need to do it twice.
Regards
Rüdiger