On Thu, Apr 28, 2016 at 11:37 AM, Yann Ylavic <[email protected]> wrote:
>
> OK, so the worker scoreboard entry is up to date with the latest
> connection/request handled.
> Now let's consider the case in KEEAPALIVE state where a new request
> arrives (possibly on another connection for mpm_event), or in
> READY/closed state (for any MPM) where a new connection+request is
> handled by the same worker.
>
> When we transition to READ just before calling ap_read_request(), we
> update the client IP in the scoreboard, but keep the request line
> (from the previous request) untouched, mixed data...
> For mpm_event this is a very transient state since the worker was
> scheduled only because the connection is POLLIN ready, so
> ap_read_request() won't block.
> For mpm_worker though, this may last KeepAliveTimeout.
> Moreover for both MPMs, if the connection is closed by the client the
> worker score will stay as is until next (real) request is read on
> another connection, or for mpm worker if keepalive expires the request
> line will be set to the value NULL (unless patch below is applied).
> That's what is observed in PR 59333, and I don't see where your commit
> change this.
> We need to either accept/document mixing, or blank the request line
> (but it will be very noticeable by users as it is already in 2.4.20),
> or restore the latest request line (and vhost) on the connection
> (hence also store it by connection, in conn_rec).
So maybe for 2.4.x we could add the following to you commit:
Index: modules/http/http_core.c
===================================================================
--- modules/http/http_core.c (revision 1741396)
+++ modules/http/http_core.c (working copy)
@@ -141,8 +141,15 @@ static int ap_process_http_async_connection(conn_r
AP_DEBUG_ASSERT(cs->state == CONN_STATE_READ_REQUEST_LINE);
while (cs->state == CONN_STATE_READ_REQUEST_LINE) {
- ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_READ, c);
+ worker_score *ws = ap_get_scoreboard_worker(c->sbh);
+ if (ws->client[0]) {
+ ap_update_child_status(c->sbh, SERVER_BUSY_READ, NULL);
+ }
+ else {
+ ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_READ, c);
+ }
+
if ((r = ap_read_request(c))) {
c->keepalive = AP_CONN_UNKNOWN;
@@ -182,13 +189,20 @@ static int ap_process_http_sync_connection(conn_re
conn_state_t *cs = c->cs;
apr_socket_t *csd = NULL;
int mpm_state = 0;
+ worker_score *ws = ap_get_scoreboard_worker(c->sbh);
+ if (ws->client[0]) {
+ ap_update_child_status(c->sbh, SERVER_BUSY_READ, NULL);
+ }
+ else {
+ ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_READ, c);
+ }
+
/*
* Read and process each request found on our connection
* until no requests are left or we decide to close.
*/
- ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_READ, c);
while ((r = ap_read_request(c)) != NULL) {
apr_interval_time_t keep_alive_timeout = r->server->keep_alive_timeout;
Index: server/protocol.c
===================================================================
--- server/protocol.c (revision 1741396)
+++ server/protocol.c (working copy)
@@ -1093,13 +1093,14 @@ request_rec *ap_read_request(conn_rec *conn)
access_status = r->status;
r->status = HTTP_OK;
ap_die(access_status, r);
- ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
+ if (r->the_request && r->the_request[0]) {
+ ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
+ }
ap_run_log_transaction(r);
r = NULL;
apr_brigade_destroy(tmp_bb);
goto traceout;
case HTTP_REQUEST_TIME_OUT:
- ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
if (!r->connection->keepalives)
ap_run_log_transaction(r);
apr_brigade_destroy(tmp_bb);
_
Such that we don't update the client IP until a request line is read.
So we'd report WRITE + new-request for a request being handled, and
READ + old-request when waiting for a new request.
