On 24 Jan 2022, at 14:13, Yann Ylavic <ylavic....@gmail.com> wrote: >> Maybe it is not needed for the handshake any longer, but it allows fine >> grained timeouts on reading speed for headers and request >> bodies which I am not sure event could deal with. At least it cannot now. > > It's not needed for the handshake *if* we have a HandshakeTimeout or > something (accept() is already done at this point so AcceptTimeout > would be a bit misleading), but currently using s->timeout is not > really the same. > > So far this series is only about handshakes, but indeed if/when all > the request read phases can go async we probably want the fine grained > timeouts/rates from mod_reqtimeout to be available still (so either a > better interaction between the MPM and reqtimeout, or general > HeaderTimeout/BodyTimeout/... directives applicable by the MPM but > we'd still need something to count for bytes/rates in the different > states).
Looks like mod_reqtimeout’s capabilities are overstated at this point - the module neatly steps out the way and does nothing the moment it sees a non blocking connection: https://github.com/apache/httpd/blob/trunk/modules/filters/mod_reqtimeout.c#L226 I imagine what we need in the async case is to have the ability to override the timeout on the connection, and have the MPM react to that timeout. The pattern would then be set the timeout and return APR_EAGAIN, let the mpm handle the timeout. We also need to teach mod_reqtimeout’s code that SSL/TLS libraries sometimes want to write: https://github.com/apache/httpd/blob/trunk/modules/filters/mod_reqtimeout.c#L293 Regards, Graham —
