Re: [Bug 56188] mod_proxy_fcgi does not send FCGI_ABORT_REQUEST on client disconnect

[Luca Toscano <to...@gmail.com>]

2017-01-30 21:58 GMT+01:00 Yann Ylavic <yl...@gmail.com>:

> On Mon, Jan 30, 2017 at 7:17 PM, Luca Toscano <to...@gmail.com>
> wrote:
> >
> > The use case that I had (the one that caused me to check the original
> > bugzilla task/patch and work on it) was a long running PHP script
> (running
> > on HHVM) that wasn't returning anything until the end of the job
> (minutes),
> > causing mod-proxy-fcgi to keep waiting even if the client disconnected.
> In
> > the beginning I also thought that there was a way to "signal" the FCGI
> > backend to stop whatever was doing (FCGI_ABORT), but it turned out to be
> not
> > widely implemented (at least from what I have checked, more info in
> > bugzilla). Timeout and ProxyTimeout could be a good compromise for this
> > particular issue, but it is a "one size fits all" solution that some
> users
> > didn't like (providing a patch with the pollset solution).
>
> I think that's what CGIDScriptTimeout or ProxyTimeout are for: avoid
> waiting too much for the script or backend (above some reasonably
> configured limit).
>
> >
> > If we remove the above use case (I fixed it in my Production environment
> > with a long Timeout value), would it be fine to rely on something like
> the
> > conn->aborted flag (afaics set by the output filter's chain while
> writing to
> > the client's conn)? It would simplify a lot the problem :)
>
> AFAICT, if forwarding data to the client fails (e.g. socket
> disconnected), ap_pass_brigade() returns an error and the loop exits.
>

Yes confirmed with a test (that flushes data to output periodically), the
connection->aborted flag is set and mod_proxy_fcgi behaves correctly.


> Maybe what is missing is nonblocking reads on the backend side, and on
> EAGAIN flush on the client side to detect potential socket errors?
>

Interesting.. So the idea would be to be non blocking while reading from
the FCGI backend, and in case of EAGAIN flush to the client in order to
force ap_pass_brigade to detect if the client aborted the connection. The
flushing part might be tricky to implement (at least for me, but I'll try
to work on it :).

In any case, it seems like we are in agreement to discard the client
connection polling idea (if anybody has more thoughts please speak up). In
any case, I believe that this use case would need to be documented properly
in the docs to warn users assuming that FCGI_ABORT is implemented.

Thanks for the review!

Luca

Re: [Bug 56188] mod_proxy_fcgi does not send FCGI_ABORT_REQUEST on client disconnect

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

On Thu, Feb 2, 2017 at 5:16 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Thu, Feb 2, 2017 at 4:51 PM, Luca Toscano <to...@gmail.com> wrote:
>>
>> 2017-01-30 21:58 GMT+01:00 Yann Ylavic <yl...@gmail.com>:
>>>
>>> Maybe what is missing is nonblocking reads on the backend side, and on
>>> EAGAIN flush on the client side to detect potential socket errors?
>>
>> Interesting.. So the idea would be to be non blocking while reading from the
>> FCGI backend, and in case of EAGAIN flush to the client in order to force
>> ap_pass_brigade to detect if the client aborted the connection.
>
> Yes, poll() => POLLIN => read() while data available => EAGAIN =>
> flush => poll().

The "read() while data available => EAGAIN" may be replaced by "read()
while read buffer is full" so to avoid a (likely) useless last
non-blocking read.

>
> The flush is necessary because output filters down the road might have
> buffered, so it makes so that everything so far reaches the socket
> (with potential error).
>
>> The flushing
>> part might be tricky to implement (at least for me, but I'll try to work on
>> it :).
>
> It should be as easy as pushing a FLUSH bucket at the end of the
> brigade passed to the client ;)
>
> The hard part may be more the non-blocking read (and handling of
> incomplete headers/data).
>
> Regards,
> Yann.

Re: [Bug 56188] mod_proxy_fcgi does not send FCGI_ABORT_REQUEST on client disconnect

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

On Thu, Feb 2, 2017 at 4:51 PM, Luca Toscano <to...@gmail.com> wrote:
>
> 2017-01-30 21:58 GMT+01:00 Yann Ylavic <yl...@gmail.com>:
>>
>> Maybe what is missing is nonblocking reads on the backend side, and on
>> EAGAIN flush on the client side to detect potential socket errors?
>
> Interesting.. So the idea would be to be non blocking while reading from the
> FCGI backend, and in case of EAGAIN flush to the client in order to force
> ap_pass_brigade to detect if the client aborted the connection.

Yes, poll() => POLLIN => read() while data available => EAGAIN =>
flush => poll().

The flush is necessary because output filters down the road might have
buffered, so it makes so that everything so far reaches the socket
(with potential error).

> The flushing
> part might be tricky to implement (at least for me, but I'll try to work on
> it :).

It should be as easy as pushing a FLUSH bucket at the end of the
brigade passed to the client ;)

The hard part may be more the non-blocking read (and handling of
incomplete headers/data).

Regards,
Yann.