httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: svn commit: r1742794 - /httpd/httpd/branches/2.4.x/STATUS
Date Sun, 08 May 2016 12:29:00 GMT
On Sun, May 8, 2016 at 12:30 PM,  <rjung@apache.org> wrote:
>
> +   * Don't globber scoreboard request info if read_request_line() fails with
> +     a timeout. In that case there's not yet any new useful request info
> +     available.
> +     Noticed via server-status showing request "NULL" after keep-alive
> +     connections timed out.
> +     No CHANGES entry needed, because there's already one for PR 59333.
> +     Sorry for the two patches. The second is better. Both change the same
> +     line, so technically only the second is needed, but merging both keeps
> +     mergeinfo more complete.
> +     trunk patch: http://svn.apache.org/r1742791
> +                  http://svn.apache.org/r1742792

This is to avoid "NULL" instead of blank, right?

ISTM that the request line has already been clobbered (blanked) by the
previous call to ap_update_child_status_from_conn(SERVER_BUSY_READ) in
ap_process_http_[a]sync_connection(), where we already lost the
previous request's line.

That's why I proposed to save the previous request line and host in
the conn_rec, and use them in the scoreboard until a new (real)
request is available (it didn't work as expected because the
scoreboard was clobbered in multiple places before Bill's changes, but
it should work better/easily now).
That would be: when no r is available but c is, use the saved
request-line/host from c; when r is available, use the ones from r and
update them in c.
That way there would be no blank at all (IOW, the scoreboard entry
would be left with the latest request handled on the connection, not a
blank which inevitably ends every connection after keepalive timeout
or close, and shouldn't be taken into account as an empty request
IMHO).

Also, that would be consistent in both MPMs worker and event, whereas
currently event is more likely to show this behaviour, thanks to
queuing (no worker thread used for keepalive handling and hence no
BUSY_READ during that time, no clobbering either).
Thus for mpm_event, we'd need to use
ap_update_child_status_from_conn(SERVER_CLOSING, c) so that the saved
request-line/vhost from the connection get reused when/if a worker
thread handles the lingering close.

How about the attached patch?

Regards,
Yann.

Mime
View raw message