>      if (ccfg->in_keep_alive) {
> -        /* For this read, the normal keep-alive timeout must be used */
> +        /* For this read[_request line()], wait for the first byte using the
> +         * normal keep-alive timeout (hence don't take this expected idle 
> time
> +         * into account to setup the connection expiry below).
> +         */
>          ccfg->in_keep_alive = 0;
> -        return ap_get_brigade(f->next, bb, mode, block, readbytes);
> +        rv = ap_get_brigade(f->next, bb, AP_MODE_SPECULATIVE, block, 1);
> +        if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(bb)) {
> +            return rv;
> +        }
> +        apr_brigade_cleanup(bb);
>      }

I think this block was already wrong, but this is where svn blame led me

mod_reqtimeout thinks the first post-keepalive read (of the first byte
of the request line) will be using keepalivetimeout, so it doesn't set
its own header read timeout.  But under event MPM I think this is
wrong.

1.The current APR socket timeout will not be the keepalive timeout but
"Timeout". The keepalive timeout was only ever used for the event MPM
queues.
2. we've already waited (in pollset/keepalive_q) and there is
allegedly a readable socket, so the quick fix of even explicitly
setting the keepalive timeout here is not right.

Should we do something differently here for event/async MPM?  I can't
think of a good value (min of a few different things?)

Reply via email to