2017-01-30 15:15 GMT+01:00 Yann Ylavic <ylavic....@gmail.com>: > Hi Luca, > > continuing on dev@... > > On Mon, Jan 30, 2017 at 11:42 AM, <bugzi...@apache.org> wrote: > > https://bz.apache.org/bugzilla/show_bug.cgi?id=56188 > [] > > > > My question was if there was any corner case in which if, after a client > has > > initiated a TCP connection close, mod-proxy-fcgi wrongly assume that the > > connection dropped (because of POLLIN and EOF) taking its own remediation > > (error logs, etc..). > > I think so, think about pipelined requests (or TLS-only data). > > > > > In this case I am wondering if my patch would introduce a performance > > regression for use cases like big requests coming in (like POSTs for > example). > > In the above case, your patch would possibly loop endlessly because > nothing empties the socket, or pulls pfd[1] out from the pollset. > > > We don't really care about reading those bytes (in the code introduced > by my > > patch) but only to check if the connection with the client is still up, > so > > there might be a better way to approach the issue. > > It could be achievable, but not easy, with care taken to pull the > client's socket out of the pollset if anything but an error or a > connection/TLS close is detected. > It probably also shouldn't start before "last_stdin" is true. > > Client side poll() may return with either: > 1. HTTP data (pipelined) => client still alive, non-empty/meta brigade > => don't abort > 2. TCP close or reset => bucket EOS or an APR_E* (though > speculative-non-blocking reads won't return EOS, and may turn EOF into > SUCCESS with empty brigade!) => abort > 3. TLS close notify => EOS/EOF? => abort > 4. TLS renegociation => rejected by httpd (since initiated by the > client) => an APR_E* => abort > n. ... > > Probably not an exhaustive list... >
Thanks a lot for the list! I am not familiar with all the use cases that you brought up but it feels to me that the mod_proxy_fcgi patch could bring more headaches than benefits.. > > > BTW, if the client connection is closed under us, why don't we keep > detecting it on write (pass_brigade) while forwarding the response? > We'd want, for good reasons, to be active between the time the request > is sent to the backend and the response arrives? > 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). 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 :) Thanks! Luca
